Message ID | 20211210145220.3750010-1-hjl.tools@gmail.com |
---|---|
State | New |
Headers | show |
Series | [v4] elf: Only allow execute libc.so.6 directly [BZ #28453] | expand |
* 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.
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"