Re: [musl] SIGSEGV with TEXTREL

[Szabolcs Nagy <nsz@port70.net>]

* Dominic Chen <d.c.ddcc@gmail.com> [2020-09-24 23:50:19 -0400]:
> Please CC me on replies.
> 
> I recently discovered that musl doesn't support DT/DF_TEXTREL in the
> main executable, which can result in the dynamic loader crashing with
> SIGSEGV and SEGV_ACCERR while processing relocations. I spent a few days
> trying to fix this in the toolchain, but because it is a prototype based
> on Clang/LLVM 4.0.0 that adds runtime instrumentation built using the
> x64 large code model, so it's not easy to fix. Also, glibc does support
> this behavior.

there are no existing libcs that fully support textrels
(since for that not just dynamic relocs but static relocs
need to be supported too).

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.

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

> 
> I ended up implementing support for this in musl itself (patch
> attached), but given the discussion in the previous thread, "Static
> linking is broken after creation of DT_TEXTREL," it seems like this
> isn't acceptable due to overhead? I don't quite understand the concern,
> because the loader needs to iterate again over the program headers only

dynamic linking overhead is not the issue.

> if the program contains TEXTRELs, which is strictly an improvement, even
> if the iteration itself is suboptimal. Alternatively, I'd suggest that
> musl at least warns about unsupported TEXTRELs if present, because
> asking application developers to debug a crashing ELF loader is quite a
> high bar.

dynamic linker failure diagnostic is something musl could
improve i think.

> 
> Thanks,
> 
> Dominic
> 

> 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)
>  {
> +	unsigned char textrel = 0;
>  	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)) {
> +			size_t cnt = p->phnum;
> +			Phdr *ph = p->phdr;
> +			for (; cnt--; ph = (void *)((char *)ph + p->phentsize)) {
> +				if (ph->p_type == PT_LOAD && !(ph->p_flags & PF_W)) {
> +					unsigned prot = (((ph->p_flags&PF_R) ? PROT_READ : 0) |
> +									((ph->p_flags&PF_X) ? PROT_EXEC : 0));
> +					size_t start = ph->p_vaddr & -PAGE_SIZE,
> +					       end = (ph->p_vaddr + ph->p_memsz + PAGE_SIZE-1) & -PAGE_SIZE;
> +					if (mprotect(laddr(p, start), end - start, prot|PROT_WRITE)
> +						&& errno != ENOSYS) {
> +						error("Error relocating %s: TEXTREL unprotect failed: %m",
> +						p->name);
> +						if (runtime) longjmp(*rtld_fail, 1);
> +					}
> +					textrel = 1;
> +				}
> +			}
> +		}
> +
>  		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],
> @@ -1345,6 +1367,25 @@ static void reloc_all(struct dso *p)
>  			if (runtime) longjmp(*rtld_fail, 1);
>  		}
>  
> +		if (textrel) {
> +			size_t cnt = p->phnum;
> +			Phdr *ph = p->phdr;
> +			for (; cnt--; ph = (void *)((char *)ph + p->phentsize)) {
> +				if (ph->p_type == PT_LOAD && !(ph->p_flags & PF_W)) {
> +					unsigned prot = (((ph->p_flags&PF_R) ? PROT_READ : 0) |
> +									((ph->p_flags&PF_X) ? PROT_EXEC : 0));
> +					size_t start = ph->p_vaddr & -PAGE_SIZE,
> +					       end = (ph->p_vaddr + ph->p_memsz + PAGE_SIZE-1) & -PAGE_SIZE;
> +					if (mprotect(laddr(p, start), end - start, prot)
> +						&& errno != ENOSYS) {
> +						error("Error relocating %s: TEXTREL protect failed: %m",
> +						p->name);
> +						if (runtime) longjmp(*rtld_fail, 1);
> +					}
> +				}
> +			}
> +		}
> +
>  		p->relocated = 1;
>  	}
>  }