On Tue, May 25, 2021 at 12:00:21AM -0900, Philip Guenther wrote:
> On Mon, May 24, 2021 at 4:59 AM Klemens Nanni <[email protected]> wrote:
>
> > When tinkering with ld.so crashes due to file corruption the other day
> > I tested a few changes but did not want to replace /usr/libexec/ld.so
> > and since recompiling executable to change their interpreter is not
> > always an option, I went for https://github.com/NixOS/patchelf which
> > allows me to manipulate executables in place.
> >
> > The tool works just fine and as a byproduct rearanges program headers;
> > that in itself is fine except our run-time link-editor is not happy with
> > such valid executables:
> >
> >
> > ELF mandates nothing but the file header be at a fixed location, hence
> > ld.so(1) must not assume any specific order for headers, segments, etc.
> >
>
> (Not quite: it does place ordering requirements in some specific cases,
> such as that PT_INTERP and PT_PHDR, if present, must appear before any
> PT_LOAD segments in the program header, and that PT_LOAD segments must
> appear in p_vaddr order.)
Thanks for clarifying on that. I've rechecked and this goes in line
with ELF Version 1.1 Section 2-2 "Program Loading and Dynamic Linking",
"Segment Types":
PT_LOAD [...] Loadable segment entries in the program header table
appear in ascending order, sorted on the p_vaddr member.
PT_INTERP [...] If it is present, it must precede any loadable segment
entry. [...]
> Looping over the program header table to parse segment headers,
> > _dl_boot() creates the executable object upon DYNAMIC and expects it to
> > be set upon GNU_RELRO, resulting in a NULL dereference iff that order is
> > reversed.
> >
> > Store relocation bits in temporary variables and update the executable
> > object once all segment headers are parsed to lift this dependency.
> >
> > Under __mips__ _dl_boot() later on uses the same temporary variable, so
> > move nothing but the declaration out of MI code so as to not alter the
> > MD code's logic/behaviour.
> >
> >
> > This fix is needed for the following work on OpenBSD:
> >
> > $ patchelf --set-interpreter $PWD/obj/ld.so ./my-ldso-test
> > $ readelf -l ./my-ldso-test | grep interpreter
> > [Requesting program interpreter: /usr/src/libexec/
> > ld.so/obj/ld.so]
> > $ ./my-ldso-test
> > it works!
> >
> > amd64 and arm64 regress is happy and all my patched executables work
> > with this.
> >
> > Feedback? Objections? OK?
> >
> > diff d7231fb4fb547dd287a884c56ae7c8b10f9145fe
> > f023dbe355bef379d55eb93eddbb2702559d5bdb
> > blob - 18bd30af759bffbc4e3fbfee9ffc29906f0d1bb0
> > blob + b66dbb169aad9afffa1283d480ad9276aff9072a
> > --- libexec/ld.so/loader.c
> > +++ libexec/ld.so/loader.c
> > @@ -464,6 +464,7 @@ _dl_boot(const char **argv, char **envp, const long dy
> > int failed;
> > struct dep_node *n;
> > Elf_Addr minva, maxva, exe_loff, exec_end, cur_exec_end;
> > + Elf_Addr relro_addr = 0, relro_size = 0;
> > Elf_Phdr *ptls = NULL;
> > int align;
> >
> > @@ -552,8 +553,8 @@ _dl_boot(const char **argv, char **envp, const long dy
> > ptls = phdp;
> > break;
> > case PT_GNU_RELRO:
> > - exe_obj->relro_addr = phdp->p_vaddr + exe_loff;
> > - exe_obj->relro_size = phdp->p_memsz;
> > + relro_addr = phdp->p_vaddr + exe_loff;
> > + relro_size = phdp->p_memsz;
> > break;
> > }
> > phdp++;
> > @@ -561,6 +562,8 @@ _dl_boot(const char **argv, char **envp, const long dy
> > exe_obj->load_list = load_list;
> > exe_obj->obj_flags |= DF_1_GLOBAL;
> > exe_obj->load_size = maxva - minva;
> > + exe_obj->relro_addr = relro_addr;
> > + exe_obj->relro_size = relro_size;
> > _dl_set_sod(exe_obj->load_name, &exe_obj->sod);
> >
> > #ifdef __i386__
> > @@ -638,7 +641,7 @@ _dl_boot(const char **argv, char **envp, const long dy
> > debug_map->r_ldbase = dyn_loff;
> > _dl_debug_map = debug_map;
> > #ifdef __mips__
> > - Elf_Addr relro_addr = exe_obj->relro_addr;
> > + relro_addr = exe_obj->relro_addr;
> > if (dynp->d_tag == DT_DEBUG &&
> > ((Elf_Addr)map_link + sizeof(*map_link) <= relro_addr
> > ||
> > (Elf_Addr)map_link >= relro_addr +
> > exe_obj->relro_size)) {
> >
> >
> ok guenther@
>
> (This still assumes PT_GNU_RELRO comes after PT_PHDR, for exe_loff, but I
> suspect even patchelf wouldn't screw with that.)
Yes, not just PT_GNU_RELRO but PT_DYNAMIC and PT_LOAD as well due to
`exe_loff' in our code.
PT_LOAD *must* come after PT_PHDR according to the above mentioned, on
PT_DYNAMIC however I couldn't find any such requirement in the spec.
