diff mbox series

[v4] elf: Only allow execute libc.so.6 directly [BZ #28453]

Message ID 20211210145220.3750010-1-hjl.tools@gmail.com
State Superseded
Headers show
Series [v4] elf: Only allow execute libc.so.6 directly [BZ #28453] | expand

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:52 p.m. UTC
Changes in the v4 patch:

1. Only allow executing executables and libc.s.6 directly.

Changes in the v3 patch:

1. Delay zero entry point value check.
2. Build testobj1.so with -Wl,--entry=0

Changes in the v2 patch:

1. Use rtld_progname in the error message.

A shared library can have an invalid non-zero entry point value generated
by the old linkers, which leads to ld.so crash when it executes such
shared library directly.  Since we can't rely on entry point values to
decide if it is a valid entry point, we should only allow executing
executables and libc.s.6 directly.  Now we get

$ ./elf/ld.so ./libc.so.6
GNU C Library (GNU libc) development release version 2.34.9000.
Copyright (C) 2021 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.
There is NO warranty; not even for MERCHANTABILITY or FITNESS FOR A
PARTICULAR PURPOSE.
Compiled by GNU CC version 11.2.1 20211203 (Red Hat 11.2.1-7).
libc ABIs: UNIQUE IFUNC ABSOLUTE
For bug reporting instructions, please see:
<https://www.gnu.org/software/libc/bugs.html>.
$ ./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              | 37 ++++++++++++++++++++++++++-----------
 elf/tst-rtld-run-dso.sh | 33 +++++++++++++++++++++++++++++++++
 3 files changed, 69 insertions(+), 11 deletions(-)
 create mode 100755 elf/tst-rtld-run-dso.sh

Comments

Florian Weimer Dec. 10, 2021, 2:58 p.m. UTC | #1
* H. J. Lu:

> +  /* With DT_NEEDED dependencies, it is a shared library.  Only allow
> +     execute libc.so directly.  */
> +  if (__glibc_unlikely (main_map->l_info[DT_NEEDED] != NULL))
> +    return (main_map->l_info[DT_SONAME] != NULL
> +	    && strncmp ("libc.so.6",
> +			((const char *) D_PTR (main_map, l_info[DT_STRTAB])
> +			 + main_map->l_info[DT_SONAME]->d_un.d_val),
> +			sizeof ("libc.so.6") - 1) == 0);

I don't think this works.  See /usr/bin/npc in Fedora, it has a soname.

Thanks,
Florian
H.J. Lu Dec. 10, 2021, 3:26 p.m. UTC | #2
On Fri, Dec 10, 2021 at 6:59 AM Florian Weimer <fweimer@redhat.com> wrote:
>
> * H. J. Lu:
>
> > +  /* With DT_NEEDED dependencies, it is a shared library.  Only allow
> > +     execute libc.so directly.  */
> > +  if (__glibc_unlikely (main_map->l_info[DT_NEEDED] != NULL))
> > +    return (main_map->l_info[DT_SONAME] != NULL
> > +         && strncmp ("libc.so.6",
> > +                     ((const char *) D_PTR (main_map, l_info[DT_STRTAB])
> > +                      + main_map->l_info[DT_SONAME]->d_un.d_val),
> > +                     sizeof ("libc.so.6") - 1) == 0);
>
> I don't think this works.  See /usr/bin/npc in Fedora, it has a soname.

It should work:

$ readelf -l /usr/bin/npc

Elf file type is DYN (Position-Independent Executable file)
Entry point 0x2a5c0
There are 14 program headers, starting at offset 64

Program Headers:
  Type           Offset             VirtAddr           PhysAddr
                 FileSiz            MemSiz              Flags  Align
  PHDR           0x0000000000000040 0x0000000000000040 0x0000000000000040
                 0x0000000000000310 0x0000000000000310  R      0x8
  INTERP         0x0000000000000350 0x0000000000000350 0x0000000000000350
                 0x000000000000001c 0x000000000000001c  R      0x1
      [Requesting program interpreter: /lib64/ld-linux-x86-64.so.2]

My patch checks PT_INTERP first.
Florian Weimer Dec. 10, 2021, 4:06 p.m. UTC | #3
* H. J. Lu:

> On Fri, Dec 10, 2021 at 6:59 AM Florian Weimer <fweimer@redhat.com> wrote:
>>
>> * H. J. Lu:
>>
>> > +  /* With DT_NEEDED dependencies, it is a shared library.  Only allow
>> > +     execute libc.so directly.  */
>> > +  if (__glibc_unlikely (main_map->l_info[DT_NEEDED] != NULL))
>> > +    return (main_map->l_info[DT_SONAME] != NULL
>> > +         && strncmp ("libc.so.6",
>> > +                     ((const char *) D_PTR (main_map, l_info[DT_STRTAB])
>> > +                      + main_map->l_info[DT_SONAME]->d_un.d_val),
>> > +                     sizeof ("libc.so.6") - 1) == 0);
>>
>> I don't think this works.  See /usr/bin/npc in Fedora, it has a soname.
>
> It should work:
>
> $ readelf -l /usr/bin/npc
>
> Elf file type is DYN (Position-Independent Executable file)
> Entry point 0x2a5c0
> There are 14 program headers, starting at offset 64
>
> Program Headers:
>   Type           Offset             VirtAddr           PhysAddr
>                  FileSiz            MemSiz              Flags  Align
>   PHDR           0x0000000000000040 0x0000000000000040 0x0000000000000040
>                  0x0000000000000310 0x0000000000000310  R      0x8
>   INTERP         0x0000000000000350 0x0000000000000350 0x0000000000000350
>                  0x000000000000001c 0x000000000000001c  R      0x1
>       [Requesting program interpreter: /lib64/ld-linux-x86-64.so.2]
>
> My patch checks PT_INTERP first.

But libc.so.6 has a program interpreter?  Why do we need to flag it
separately?

I'm really confused now. 8-(

Thanks,
Florian
H.J. Lu Dec. 10, 2021, 6:37 p.m. UTC | #4
On Fri, Dec 10, 2021 at 8:06 AM Florian Weimer <fweimer@redhat.com> wrote:
>
> * H. J. Lu:
>
> > On Fri, Dec 10, 2021 at 6:59 AM Florian Weimer <fweimer@redhat.com> wrote:
> >>
> >> * H. J. Lu:
> >>
> >> > +  /* With DT_NEEDED dependencies, it is a shared library.  Only allow
> >> > +     execute libc.so directly.  */
> >> > +  if (__glibc_unlikely (main_map->l_info[DT_NEEDED] != NULL))
> >> > +    return (main_map->l_info[DT_SONAME] != NULL
> >> > +         && strncmp ("libc.so.6",
> >> > +                     ((const char *) D_PTR (main_map, l_info[DT_STRTAB])
> >> > +                      + main_map->l_info[DT_SONAME]->d_un.d_val),
> >> > +                     sizeof ("libc.so.6") - 1) == 0);
> >>
> >> I don't think this works.  See /usr/bin/npc in Fedora, it has a soname.
> >
> > It should work:
> >
> > $ readelf -l /usr/bin/npc
> >
> > Elf file type is DYN (Position-Independent Executable file)
> > Entry point 0x2a5c0
> > There are 14 program headers, starting at offset 64
> >
> > Program Headers:
> >   Type           Offset             VirtAddr           PhysAddr
> >                  FileSiz            MemSiz              Flags  Align
> >   PHDR           0x0000000000000040 0x0000000000000040 0x0000000000000040
> >                  0x0000000000000310 0x0000000000000310  R      0x8
> >   INTERP         0x0000000000000350 0x0000000000000350 0x0000000000000350
> >                  0x000000000000001c 0x000000000000001c  R      0x1
> >       [Requesting program interpreter: /lib64/ld-linux-x86-64.so.2]
> >
> > My patch checks PT_INTERP first.
>
> But libc.so.6 has a program interpreter?  Why do we need to flag it
> separately?
>
> I'm really confused now. 8-(
>
>

Fixed in the v5 patch.
diff mbox series

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..f6b80444fa 100644
--- a/elf/rtld.c
+++ b/elf/rtld.c
@@ -1108,8 +1108,9 @@  load_audit_modules (struct link_map *main_map, struct audit_list *audit_list)
 }
 
 /* Check if the executable is not actualy dynamically linked, and
-   invoke it directly in that case.  */
-static void
+   invoke it directly in that case.  Return true if it can be executed
+   directly by ld.so.  */
+static bool
 rtld_chain_load (struct link_map *main_map, char *argv0)
 {
   /* The dynamic loader run against itself.  */
@@ -1122,17 +1123,22 @@  rtld_chain_load (struct link_map *main_map, char *argv0)
 		  + main_map->l_info[DT_SONAME]->d_un.d_val)) == 0)
     _dl_fatal_printf ("%s: loader cannot load itself\n", rtld_soname);
 
-  /* With DT_NEEDED dependencies, the executable is dynamically
-     linked.  */
-  if (__glibc_unlikely (main_map->l_info[DT_NEEDED] != NULL))
-    return;
-
-  /* If the executable has program interpreter, it is dynamically
-     linked.  */
+  /* If it has program interpreter, it is dynamically linked
+     executable.  */
   for (size_t i = 0; i < main_map->l_phnum; ++i)
     if (main_map->l_phdr[i].p_type == PT_INTERP)
-      return;
+      return true;
+
+  /* With DT_NEEDED dependencies, it is a shared library.  Only allow
+     execute libc.so directly.  */
+  if (__glibc_unlikely (main_map->l_info[DT_NEEDED] != NULL))
+    return (main_map->l_info[DT_SONAME] != NULL
+	    && strncmp ("libc.so.6",
+			((const char *) D_PTR (main_map, l_info[DT_STRTAB])
+			 + main_map->l_info[DT_SONAME]->d_un.d_val),
+			sizeof ("libc.so.6") - 1) == 0);
 
+  /* This is a static executable.  */
   const char *pathname = _dl_argv[0];
   if (argv0 != NULL)
     _dl_argv[0] = argv0;
@@ -1144,6 +1150,7 @@  rtld_chain_load (struct link_map *main_map, char *argv0)
   else
     _dl_fatal_printf("%s: cannot execute %s: %d\n",
 		     rtld_soname, pathname, errno);
+  return true;
 }
 
 static void
@@ -1181,6 +1188,8 @@  dl_main (const ElfW(Phdr) *phdr,
   _dl_starting_up = 1;
 #endif
 
+  bool can_execute = true;
+
   const char *ld_so_name = _dl_argv[0];
   if (*user_entry == (ElfW(Addr)) ENTRY_POINT)
     {
@@ -1415,7 +1424,7 @@  dl_main (const ElfW(Phdr) *phdr,
       main_map = GL(dl_ns)[LM_ID_BASE]._ns_loaded;
 
       if (__glibc_likely (state.mode == rtld_mode_normal))
-	rtld_chain_load (main_map, argv0);
+	can_execute = rtld_chain_load (main_map, argv0);
 
       phdr = main_map->l_phdr;
       phnum = main_map->l_phnum;
@@ -2491,6 +2500,12 @@  dl_main (const ElfW(Phdr) *phdr,
       rtld_timer_accum (&relocate_time, start);
     }
 
+  /* Stop if it can't be executed.  */
+  if (!can_execute)
+    _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.  */
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"