Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20200131042431.GC1663@brightrain.aerifal.cx>
Date: Thu, 30 Jan 2020 23:24:31 -0500
From: Rich Felker <dalias@...c.org>
To: musl@...ts.openwall.com
Subject: Re: Static linking is broken after creation of DT_TEXTREL
 segment

On Thu, Jan 30, 2020 at 06:02:49PM +0100, Markus Wichmann wrote:
> > > 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.

Right. Not specific to textrels, I actually do want to make ldso track
the hull of writable address range for each dso and validate
relocations against it before attempting to apply them. This would
allow error reporting without significant runtime cost (vs iterating
LOAD segments) and would be a first step towards possibly making ldd
of untrusted binaries safe (though that requires a lot more).

> > 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.

This is true, but that's after the program has started execution. The
dynamic linker does report errors when it's practical to do so, either
to stderr before execution or via dlerror at runtime. So detecting and
reporting this would not be unprecedented.

> 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.

I believe this was fixed in 77846800722914eeba170505c2e7f89e12a6beff.

> 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.

I thought it would produce an error for unsupported relocation type,
but maybe not if the same relocation numbers were reused. This should
probably be improved.

> Anyway, option 1 would leave the relocations unprocessed, typically
> leading to invalid code references down the line, and therefore another
> segfault.

Right. This is really bad and not on the table.

> Option 2 is the same but wordier.

Likewise this makes no sense at all.

> 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).

Assuming this is at startup, that's not a possibility; no application
code has run yet. But ldso already has a fatal error mechanism; it
doesn't need to treat this one differently.

For runtime (dlopen), aborting is not an option. Syntactically valid
files which cannot be loaded for semantic reasons should be an error
reported by dlerror. (musl makes no claim to handle syntactically
invalid files safely; doing so is rather difficult and would require
some intense hardening work, and is of little use except for ldd since
otherwise a malicious file could just have malicious code in its
ctors...)

The right action here in the long term is probably reporting
unsupported files rather than crashing. But it's a fairly minor issue
and depends on some infrastructure that's not yet done for tracking
address ranges.

Rich

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.