Re: [musl] SIGSEGV with TEXTREL

[Rich Felker <dalias@libc.org>]

On Fri, Sep 25, 2020 at 04:13:19PM -0400, Dominic Chen wrote:
> On 9/25/2020 2:58 PM, Rich Felker wrote:
> 
> > On the other hand, there's no compelling reason to support textrels in
> > the main program since the main program can just be linked as non-PIE
> > if you have object files (e.g. due to asm source files or static
> > libraries you don't have source to) that are not PIC-compatible.
> 
> I'm actually comparing the overheads of various security mechanisms, so
> I need to build with PIC and RELRO/BIND_NOW for ASLR.

If you build with PIC then you don't have textrels.

> On 9/25/2020 5:37 AM, Szabolcs Nagy wrote:
> 
> > there are no existing libcs that fully support textrels
> > (since for that not just dynamic relocs but static relocs
> > need to be supported too).
> I only need TEXTREL support for dynamic relocations, so static
> relocations aren't an issue.
> > glibc only supports a small set of textrels and of course
> > it has to mark the code executable+writable which means
> > (1) the code cannot be shared across processes, it will
> > actually use physical memory where the modified code is
> > stored per process which is not ideal when you work with
> > large code model, (2) all security policies have to be
> > turned off that prevent exec+write mappings for this to
> > work at all which is not acceptable in many environments.
> 
> I don't see how (2) applies. Both glibc and the previous patch only
> remap text segments writable during relocation processing, and then
> remap them back read-only immediately afterwards. If you're referring to
> W^X, text segments don't need to be executable during relocation
> processing either, so that can be avoided.

Technically that's true for a main executable loaded by the dynamic
linker, but it's not true for static-PIE or for (future plan; PoC
patch exists but needs polishing before it goes upstream) musl's new
dynamic linking mode with no PT_INTERP where the program itself does
the work of finding and loading the dynamic linker from its startup
code (allows shipping dynamic linker alongside executable with an
executable-relative rather than absolute pathname). It's also not true
for the dynamic linker itself.

> > for these reasons it is considered to be a bug to create
> > binaries with textrels. i think large code model should
> > not need textrel on x86_64: there should be a way to
> > create >4G pc relative offset in code that does not need
> > any relocs. (or do you have some example where that fails?)
> Before D47211 (Clang/LLVM 7.0.0), PIC with the medium or large code
> models is unsupported, and the compiler will silently ignore the PIC flag.

Then this is a configuration that's not supported. You could still
make non-PIE (pass -no-pie at link time) executables just fine.

> > dynamic linker failure diagnostic is something musl could
> > improve i think.
> How about something along the lines of the following?
> >
> >> diff --git a/ldso/dynlink.c b/ldso/dynlink.c
> >> index d7726118..c7449df2 100644
> >> --- a/ldso/dynlink.c
> >> +++ b/ldso/dynlink.c
> >> @@ -1326,10 +1326,32 @@ static void do_mips_relocs(struct dso *p, size_t *got)
> >>  
> >>  static void reloc_all(struct dso *p)
> >>  {
> >>  	size_t dyn[DYN_CNT];
> >>  	for (; p; p=p->next) {
> >>  		if (p->relocated) continue;
> >>  		decode_vec(p->dynv, dyn, DYN_CNT);
> >> +
> >> +		if ((dyn[0] & 1<<DT_TEXTREL) || (dyn[DT_FLAGS] & DF_TEXTREL)) {
> >> +			error("Warning: TEXTREL not supported!",
> >> +
> >>  		if (NEED_MIPS_GOT_RELOCS)
> >>  			do_mips_relocs(p, laddr(p, dyn[DT_PLTGOT]));
> >>  		do_relocs(p, laddr(p, dyn[DT_JMPREL]), dyn[DT_PLTRELSZ],
> >>

This breaks existing support for textrels in shared libraries, which
isn't a feature most of us really want anymore, but it would be a
regression to remove it.

The right way to make this more friendly, I think, would be tracking
the writable mapping range for each DSO (technically this is
incomplete since it could be multiple ranges, but in that case we'd
just take the convex hull of them and accept false negatives because
anything else is almost surely too big a performance hit), and
erroring out before processing a relocation at an address that's not
writable for its DSO. This would also go part of the way towards
making it possible for ldd to process untrusted files.

I'm not 100% opposed to allowing textrel for the main application with
the existing textrel restrictions that exist for shared libraries (on
the types of relocations supported and the archs it works on), but I
do lean against it, and somewhat worry that it would lead to requests
for further textrel support when it's something we're trying to phase
out rather than embrace.

Rich