Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Fri, 31 Jan 2020 18:16:19 +0300
From: Андрей Аладьев <aladjev.andrew@...il.com>
To: musl@...ts.openwall.com
Subject: Re: Static linking is broken after creation of DT_TEXTREL segment

Hello Markus, I want to ask a question about this one:

> The issue is more complicated, because the app can have an unbounded
number of PT_LOAD segments with the PF_W flag absent. So checking the
relocations would require the dynlinker to first iterate over all PHDRs to
check for the unlikely case that textrels are present. Only because they
might be.

I made a light code overview and found that there is already a mapping for
segments: "loadmap":

dso->loadmap->segs[i].p_vaddr = ph->p_vaddr;
dso->loadmap->segs[i].p_memsz = ph->p_memsz;

We can add here the following line:
dso->loadmap->segs[i].readonly = ph->p_flags & PF_W;

Than add "readonly" into "fdpic_loadseg":
struct fdpic_loadseg {
  uintptr_t addr, p_vaddr, p_memsz;
  bool readonly;
};

Than we can refactor "do_relocs" function, it will use new function like
"reloc_addr = request_writable_address(dso, rel[0])" instead of "laddr". It
will return "NULL" if address is out of segments or readonly and throw
beautiful error.

But I found that "loadmap" is used only when "DL_FDPIC" is "1". But this is
true only for "sh" arch. Why? This loadmap is very good abstraction the can
live in separate file and be used everywhere.

чт, 30 янв. 2020 г. в 20:03, Markus Wichmann <nullplan@....net>:

> On Wed, Jan 29, 2020 at 11:08:46PM +0300, Андрей Аладьев wrote:
> > > Ooh boy, why would you do this? When there's a perfectly good -lgmp
> just
> > waiting for you.
> >
> > Usage of "/usr/lib/libgmp.a" directly is not forbidden by any toolchain
> or
> > build system. Moreover it is recommended by cmake. You can google for
> > "target_link_libraries( site:github.com" and found millions of software
> > that uses something like:
> >
> > find_library(EXTERNAL_LIB)
> > target_link_libraries(something ${EXTERNAL_LIB_FOUND})
> >
>
> Doesn't that search for the library in the correct mode? I thought cmake
> was that smart...
>
> > > The warning is justified, you usually do not want to do this. With a
> > TEXTREL, the code has to be mapped as writable, so now programming errors
> > and exploits can change the executable code.
> >
> > This is warning from ld, not from musl. Segfault is not an acceptable
> > message from libc. Libc should not try to write into readonly pointer.
> >
> > > Well, the remedy is obvious: Get rid of the TEXTREL.
> >
> > Yes, I've found a workaround: "USE='-asm' emerge -v1 gmp", assembly is
> > broken, will report it to gmp upstream. But this is not a fix for the
> issue.
> >
>
> There are several ways to achieve this. Getting rid of the assembly is
> one such way. In another answer you also advocated for --with-pic. My
> solution would probably have been to patch the code to never emit text
> relocations in the first place, which is achieved the same way in the
> end.
>
> The issue of musl not supporting textrels in the application itself
> remains, though.
>
> > > Iterate over the apps PHDRs and remove write protection from all RO
> > segments?
> >
> > So libc knows that file is mapped as readonly and should not try to write
> > into readonly pointers.
>
> Almost all relocations point to writable memory. For obvious reasons. So
> musl doesn't check this.
>
> The issue is more complicated, because the app can have an unbounded
> number of PT_LOAD segments with the PF_W flag absent. So checking the
> relocations would require the dynlinker to first iterate over all PHDRs
> to check for the unlikely case that textrels are present. Only because
> they might be.
>
> > Libc can do the following:
> >
> > 1. Ignore impossible relocations.
> > 2. Add a warning to stderr and still ignore impossible relocations.
> > 3. do abort, user will receive SIGABRT and understand that he uses libc
> in
> > a wrong way.
> >
> > Segfault is not an acceptable answer.
> >
>
> Have I got news for you. Unlike glibc, musl does not indicate
> irrecoverable state with a litany into stderr, but usually by calling
> a_crash(), which will terminate the process by executing an illegal
> instruction. This typically results in SIGILL being delivered, but on
> some archs it is still a segfault.
>
> Also, there is at least one place in the dynlinker where, as I recall,
> mmap() is being called directly, but rather than check for errors in the
> return value, the value is just used, because all error returns cause
> segfaults.
>
> And then there was the case of PowerPC's original ABI, now called the
> BSS-PLT ABI, which expects the dynlinker to fill out the PLT at runtime,
> which musl doesn't do. Trying to run a BSS-PLT binary with musl will
> therefore also very quickly segfault. musl doesn't fill out the PLT,
> because the ABI is old, a replacement has been in place since at least
> 2003, and PowerPC would be the only arch to need something like this.
>
> Anyway, option 1 would leave the relocations unprocessed, typically
> leading to invalid code references down the line, and therefore another
> segfault. Option 2 is the same but wordier. Option 3 has a chance to be
> subverted (user could block or ignore SIGABRT before executing the main
> binary. With SIGSEGV or SIGILL, the user can block or ignore those, but
> then the kernel will just kill the process outright if those conditions
> arrise).
>
> Options 1 and 2 also have the undesirable effect of possibly only
> crashing sometimes, not all the time. See the recent cuserid() thread
> about why that is a problem.
>
> And the issue you have with musl, not gmp, still remains: TEXTREL in the
> application remains unsupported.
>
> Ciao,
> Markus
>

Content of type "text/html" skipped

Powered by blists - more mailing lists

Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.