Re: ld.so: program headers: do not rely on DYNAMIC coming before GNU_RELRO
On Tue, May 25, 2021 at 12:00:21AM -0900, Philip Guenther wrote:
> On Mon, May 24, 2021 at 4:59 AM Klemens Nanni 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.
Re: ld.so: program headers: do not rely on DYNAMIC coming before GNU_RELRO
On Mon, May 24, 2021 at 4:59 AM Klemens Nanni 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.)
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.)
ld.so: program headers: do not rely on DYNAMIC coming before GNU_RELRO
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.
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)) {
