[v4] elf: Only allow execute libc.so.6 directly [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
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
* 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
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.
* 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
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.
@@ -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
@@ -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. */
new file mode 100755
@@ -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"