elf: Stop with zero entry point value [BZ #28453]

Message ID 20211210023106.3564447-1-hjl.tools@gmail.com
State Superseded
Headers
Series elf: Stop with zero entry point value [BZ #28453] |

Checks

Context Check Description
dj/TryBot-apply_patch success Patch applied to master at the time it was sent
dj/TryBot-32bit success Build for i686

Commit Message

H.J. Lu Dec. 10, 2021, 2:31 a.m. UTC
  Stop with zero entry point value unless we are tracing shared objects
since a zero entry point value in the ELF header indicates there is no
associated entry point.  Now we get

$ ./elf/ld.so /lib64/libstdc++.so.6.0.29
./elf/ld.so: cannot execute '/lib64/libstdc++.so.6.0.29' without entry point
$

instead of

$ /lib64/ld-linux-x86-64.so.2 /lib64/libstdc++.so.6.0.29
Segmentation fault (core dumped)
$

This fixes [BZ #28453].
---
 elf/Makefile            | 10 ++++++++++
 elf/rtld.c              |  8 ++++++++
 elf/tst-rtld-run-dso.sh | 33 +++++++++++++++++++++++++++++++++
 3 files changed, 51 insertions(+)
 create mode 100755 elf/tst-rtld-run-dso.sh
  

Comments

Florian Weimer Dec. 10, 2021, 4:23 a.m. UTC | #1
* H. J. Lu via Libc-alpha:

> Stop with zero entry point value unless we are tracing shared objects
> since a zero entry point value in the ELF header indicates there is no
> associated entry point.  Now we get
>
> $ ./elf/ld.so /lib64/libstdc++.so.6.0.29
> ./elf/ld.so: cannot execute '/lib64/libstdc++.so.6.0.29' without entry point
> $
>
> instead of
>
> $ /lib64/ld-linux-x86-64.so.2 /lib64/libstdc++.so.6.0.29
> Segmentation fault (core dumped)
> $
>
> This fixes [BZ #28453].

Hah.  We recently had a downstream request to fix this.

> +$(objpfx)tst-rtld-run-dso.out: tst-rtld-run-dso.sh $(objpfx)ld.so \
> +			    $(objpfx)testobj1.so
> +	$(SHELL) tst-rtld-run-dso.sh $(objpfx)ld.so $(objpfx)testobj1.so \
> +	    '$(test-wrapper-env)' '$(run_program_env)' > $@
> +	$(evaluate-test)
> +



> diff --git a/elf/rtld.c b/elf/rtld.c
> index 6ce1e07dc0..77bcdf8e29 100644
> --- a/elf/rtld.c
> +++ b/elf/rtld.c
> @@ -1424,6 +1424,14 @@ dl_main (const ElfW(Phdr) *phdr,
>  	 implementations which has no real free() function it does not
>  	 makes sense to free the old string first.  */
>        main_map->l_name = (char *) "";
> +
> +      /* Stop if there is no associated entry point and we are not
> +         tracing shared objects.  */
> +      if (main_map->l_entry == main_map->l_addr
> +	  && state.mode != rtld_mode_trace)
> +	_dl_fatal_printf("%s: cannot execute '%s' without entry point\n",
> +			 ld_so_name, _dl_argv[_dl_argc -1]);

Missing space before 1.

Should we say “cannot execute shared object” or “cannot exe[cute a]
shared library directly”?  execve should fail with ELIBEXEC, and the
error messages should match.

Should this check come later, after we have run ELF constructors, to
maximize backwards compatibility?  ELF constructors might never return.

Thanks,
Florian
  
Florian Weimer Dec. 10, 2021, 4:26 a.m. UTC | #2
* Florian Weimer:

> * H. J. Lu via Libc-alpha:
>
>> Stop with zero entry point value unless we are tracing shared objects
>> since a zero entry point value in the ELF header indicates there is no
>> associated entry point.  Now we get
>>
>> $ ./elf/ld.so /lib64/libstdc++.so.6.0.29
>> ./elf/ld.so: cannot execute '/lib64/libstdc++.so.6.0.29' without entry point
>> $
>>
>> instead of
>>
>> $ /lib64/ld-linux-x86-64.so.2 /lib64/libstdc++.so.6.0.29
>> Segmentation fault (core dumped)
>> $
>>
>> This fixes [BZ #28453].
>
> Hah.  We recently had a downstream request to fix this.
>
>> +$(objpfx)tst-rtld-run-dso.out: tst-rtld-run-dso.sh $(objpfx)ld.so \
>> +			    $(objpfx)testobj1.so
>> +	$(SHELL) tst-rtld-run-dso.sh $(objpfx)ld.so $(objpfx)testobj1.so \
>> +	    '$(test-wrapper-env)' '$(run_program_env)' > $@
>> +	$(evaluate-test)
>> +

And I forgot to fill in the blanks:

Nick changed binutils very recently to default a zero entry point if
_start is not defined.  Before ld just used the lowest text address as
the entry point (apparently a leftover from the a.out days).

This means we need to link testobj1 with -Wl,--entry=0 (or use a new
test object).

>> diff --git a/elf/rtld.c b/elf/rtld.c
>> index 6ce1e07dc0..77bcdf8e29 100644
>> --- a/elf/rtld.c
>> +++ b/elf/rtld.c
>> @@ -1424,6 +1424,14 @@ dl_main (const ElfW(Phdr) *phdr,
>>  	 implementations which has no real free() function it does not
>>  	 makes sense to free the old string first.  */
>>        main_map->l_name = (char *) "";
>> +
>> +      /* Stop if there is no associated entry point and we are not
>> +         tracing shared objects.  */
>> +      if (main_map->l_entry == main_map->l_addr
>> +	  && state.mode != rtld_mode_trace)
>> +	_dl_fatal_printf("%s: cannot execute '%s' without entry point\n",
>> +			 ld_so_name, _dl_argv[_dl_argc -1]);
>
> Missing space before 1.
>
> Should we say “cannot execute shared object” or “cannot exe[cute a]
> shared library directly”?  execve should fail with ELIBEXEC, and the
> error messages should match.
>
> Should this check come later, after we have run ELF constructors, to
> maximize backwards compatibility?  ELF constructors might never return.
>
> Thanks,
> Florian
  
H.J. Lu Dec. 10, 2021, 5:02 a.m. UTC | #3
On Thu, Dec 9, 2021 at 8:23 PM Florian Weimer <fweimer@redhat.com> wrote:
>
> * H. J. Lu via Libc-alpha:
>
> > Stop with zero entry point value unless we are tracing shared objects
> > since a zero entry point value in the ELF header indicates there is no
> > associated entry point.  Now we get
> >
> > $ ./elf/ld.so /lib64/libstdc++.so.6.0.29
> > ./elf/ld.so: cannot execute '/lib64/libstdc++.so.6.0.29' without entry point
> > $
> >
> > instead of
> >
> > $ /lib64/ld-linux-x86-64.so.2 /lib64/libstdc++.so.6.0.29
> > Segmentation fault (core dumped)
> > $
> >
> > This fixes [BZ #28453].
>
> Hah.  We recently had a downstream request to fix this.
>
> > +$(objpfx)tst-rtld-run-dso.out: tst-rtld-run-dso.sh $(objpfx)ld.so \
> > +                         $(objpfx)testobj1.so
> > +     $(SHELL) tst-rtld-run-dso.sh $(objpfx)ld.so $(objpfx)testobj1.so \
> > +         '$(test-wrapper-env)' '$(run_program_env)' > $@
> > +     $(evaluate-test)
> > +
>
>
>
> > diff --git a/elf/rtld.c b/elf/rtld.c
> > index 6ce1e07dc0..77bcdf8e29 100644
> > --- a/elf/rtld.c
> > +++ b/elf/rtld.c
> > @@ -1424,6 +1424,14 @@ dl_main (const ElfW(Phdr) *phdr,
> >        implementations which has no real free() function it does not
> >        makes sense to free the old string first.  */
> >        main_map->l_name = (char *) "";
> > +
> > +      /* Stop if there is no associated entry point and we are not
> > +         tracing shared objects.  */
> > +      if (main_map->l_entry == main_map->l_addr
> > +       && state.mode != rtld_mode_trace)
> > +     _dl_fatal_printf("%s: cannot execute '%s' without entry point\n",
> > +                      ld_so_name, _dl_argv[_dl_argc -1]);
>
> Missing space before 1.
>
> Should we say “cannot execute shared object” or “cannot exe[cute a]
> shared library directly”?  execve should fail with ELIBEXEC, and the
> error messages should match.

Fixed.

> Should this check come later, after we have run ELF constructors, to
> maximize backwards compatibility?  ELF constructors might never return.

Fixed.

> Thanks,
> Florian
>

Here is the v3 patch.
  
Florian Weimer Dec. 10, 2021, 6:07 a.m. UTC | #4
* H. J. Lu:

> diff --git a/elf/rtld.c b/elf/rtld.c
> index 6ce1e07dc0..7d1801c51c 100644
> --- a/elf/rtld.c
> +++ b/elf/rtld.c
> @@ -2491,6 +2491,12 @@ dl_main (const ElfW(Phdr) *phdr,
>        rtld_timer_accum (&relocate_time, start);
>      }
>  
> +  /* Stop if there is no associated entry point.  */
> +  if (rtld_is_main && main_map->l_entry == main_map->l_addr)
> +    _dl_fatal_printf("%s: cannot execute shared object '%s' directly "
> +		     "without entry point\n",
> +		     ld_so_name, rtld_progname);
> +
>    /* Relocation is complete.  Perform early libc initialization.  This
>       is the initial libc, even if audit modules have been loaded with
>       other libcs.  */

I think this is still too early for full backwards compatibility.

However, the bug you are actually trying to fix occurs during
relocation, so it looks to me as if using entry point addresses as an
indicator will not actually work.  And entry point zero is generated by
recent binutils only anyway.

Thanks,
Florian
  
H.J. Lu Dec. 10, 2021, 1:15 p.m. UTC | #5
On Thu, Dec 9, 2021 at 10:07 PM Florian Weimer <fweimer@redhat.com> wrote:
>
> * H. J. Lu:
>
> > diff --git a/elf/rtld.c b/elf/rtld.c
> > index 6ce1e07dc0..7d1801c51c 100644
> > --- a/elf/rtld.c
> > +++ b/elf/rtld.c
> > @@ -2491,6 +2491,12 @@ dl_main (const ElfW(Phdr) *phdr,
> >        rtld_timer_accum (&relocate_time, start);
> >      }
> >
> > +  /* Stop if there is no associated entry point.  */
> > +  if (rtld_is_main && main_map->l_entry == main_map->l_addr)
> > +    _dl_fatal_printf("%s: cannot execute shared object '%s' directly "
> > +                  "without entry point\n",
> > +                  ld_so_name, rtld_progname);
> > +
> >    /* Relocation is complete.  Perform early libc initialization.  This
> >       is the initial libc, even if audit modules have been loaded with
> >       other libcs.  */
>
> I think this is still too early for full backwards compatibility.
>
> However, the bug you are actually trying to fix occurs during
> relocation, so it looks to me as if using entry point addresses as an
> indicator will not actually work.  And entry point zero is generated by
> recent binutils only anyway.

True.  There is no easy way to identify a wrong entry point generated
by the old linkers.

> Thanks,
> Florian
>
  

Patch

diff --git a/elf/Makefile b/elf/Makefile
index ef36008673..1832dfa537 100644
--- a/elf/Makefile
+++ b/elf/Makefile
@@ -50,6 +50,10 @@  ifeq (yesyes,$(build-shared)$(run-built-tests))
 tests-special += $(objpfx)list-tunables.out
 endif
 
+ifeq (yes,$(build-shared))
+tests-special += $(objpfx)tst-rtld-run-dso.out
+endif
+
 # Make sure that the compiler does not insert any library calls in tunables
 # code paths.
 ifeq (yes,$(have-loop-to-function))
@@ -1877,6 +1881,12 @@  $(objpfx)list-tunables.out: tst-rtld-list-tunables.sh $(objpfx)ld.so
 	    $(objpfx)/tst-rtld-list-tunables.out > $@; \
 	$(evaluate-test)
 
+$(objpfx)tst-rtld-run-dso.out: tst-rtld-run-dso.sh $(objpfx)ld.so \
+			    $(objpfx)testobj1.so
+	$(SHELL) tst-rtld-run-dso.sh $(objpfx)ld.so $(objpfx)testobj1.so \
+	    '$(test-wrapper-env)' '$(run_program_env)' > $@
+	$(evaluate-test)
+
 tst-dst-static-ENV = LD_LIBRARY_PATH='$$ORIGIN'
 
 $(objpfx)tst-rtld-help.out: $(objpfx)ld.so
diff --git a/elf/rtld.c b/elf/rtld.c
index 6ce1e07dc0..77bcdf8e29 100644
--- a/elf/rtld.c
+++ b/elf/rtld.c
@@ -1424,6 +1424,14 @@  dl_main (const ElfW(Phdr) *phdr,
 	 implementations which has no real free() function it does not
 	 makes sense to free the old string first.  */
       main_map->l_name = (char *) "";
+
+      /* Stop if there is no associated entry point and we are not
+         tracing shared objects.  */
+      if (main_map->l_entry == main_map->l_addr
+	  && state.mode != rtld_mode_trace)
+	_dl_fatal_printf("%s: cannot execute '%s' without entry point\n",
+			 ld_so_name, _dl_argv[_dl_argc -1]);
+
       *user_entry = main_map->l_entry;
 
       /* Set bit indicating this is the main program map.  */
diff --git a/elf/tst-rtld-run-dso.sh b/elf/tst-rtld-run-dso.sh
new file mode 100755
index 0000000000..5192f64210
--- /dev/null
+++ b/elf/tst-rtld-run-dso.sh
@@ -0,0 +1,33 @@ 
+#!/bin/sh
+# Test for ld.so on a shared library with no associated entry point.
+# Copyright (C) 2021 Free Software Foundation, Inc.
+# This file is part of the GNU C Library.
+#
+# The GNU C Library is free software; you can redistribute it and/or
+# modify it under the terms of the GNU Lesser General Public
+# License as published by the Free Software Foundation; either
+# version 2.1 of the License, or (at your option) any later version.
+#
+# The GNU C Library is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+# Lesser General Public License for more details.
+#
+# You should have received a copy of the GNU Lesser General Public
+# License along with the GNU C Library; if not, see
+# <https://www.gnu.org/licenses/>.
+
+set -e
+
+rtld=$1
+dso=$2
+test_wrapper_env=$3
+run_program_env=$4
+
+LC_ALL=C
+export LC_ALL
+
+${test_wrapper_env} \
+${run_program_env} \
+$rtld $dso 2>&1 \
+| grep "cannot execute"