[glibc] elf: Replace alloca/VLA with dl_scratch_buffer in dl-load.c
Adhemerval Zanella via Glibc-cvs <[email protected]> Wed, 20 May 2026 18:17:58 +0000 (GMT)
| Newsgroups | gmane.comp.lib.glibc.cvs |
|---|---|
| Message-ID | <[email protected]> |
https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=26ed1cb8ca8aa676f3eea6d617ddab0275f4da5d commit 26ed1cb8ca8aa676f3eea6d617ddab0275f4da5d Author: Adhemerval Zanella <[email protected]> Date: Tue May 19 10:23:53 2026 -0300 elf: Replace alloca/VLA with dl_scratch_buffer in dl-load.c is_trusted_path_normalize, print_search_path, and open_path used alloca or a VLA to hold a path scratch buffer sized by user-controlled inputs (an RPATH directory length, or max_dirnamelen + max_capstrlen + namelen). On the worst case that consumes up to PATH_MAX bytes of stack per call, which can overflow a PTHREAD_STACK_MIN-sized stack mid-dlopen when combined with the loader's other on-stack scratch (struct filebuf, etc.). Replace those allocations with dl_scratch_buffer. As a small cleanup, print_search_path now takes the scratch buffer from its caller (open_path's buffer is already large enough -- max_dirnamelen + max_capstrlen + namelen with namelen >= 1 covers the max_dirnamelen + max_capstrlen + 1 print_search_path requires), so LD_DEBUG=libs no longer pays for an extra allocation per open_path invocation. A new test elf/tst-dl-path-buf exercises the relevant paths -- dlopen via DT_RPATH, open_path failure cleanup, dlopen with an over-long name, dlopen from a PTHREAD_STACK_MIN thread. Checked on aarch64-linux-gnu, x86_64-linux-gnu, and i686-linux-gnu. Reviewed-by: H.J. Lu <[email protected]> Diff: --- elf/Makefile | 13 +++ elf/dl-load.c | 51 ++++++++--- elf/tst-dl-path-buf-mod.c | 23 +++++ elf/tst-dl-path-buf.c | 220 ++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 296 insertions(+), 11 deletions(-) diff --git a/elf/Makefile b/elf/Makefile index a946d5806f..f668dec368 100644 --- a/elf/Makefile +++ b/elf/Makefile @@ -410,6 +410,7 @@ tests += \ tst-debug1 \ tst-deep1 \ tst-dl-is_dso \ + tst-dl-path-buf \ tst-dlclose-lazy \ tst-dlmodcount \ tst-dlmopen-dlerror \ @@ -703,6 +704,7 @@ ifeq (yes,$(build-shared)) ifneq ($(PERL),no) tests-special += \ $(objpfx)noload-mem.out \ + $(objpfx)tst-dl-path-buf-mem.out \ $(objpfx)tst-leaks1-mem.out \ # tests-special endif @@ -912,6 +914,7 @@ modules-names += \ tst-deep1mod1 \ tst-deep1mod2 \ tst-deep1mod3 \ + tst-dl-path-buf-mod \ tst-dl_find_object-mod1 \ tst-dl_find_object-mod2 \ tst-dl_find_object-mod3 \ @@ -2278,6 +2281,16 @@ CFLAGS-tst-dlopenrpath.c += -DPFX=\"$(objpfx)\" LDFLAGS-tst-dlopenrpathmod.so += -Wl,-rpath,\$$ORIGIN/test-subdir $(objpfx)tst-dlopenrpath.out: $(objpfx)firstobj.so +$(objpfx)tst-dl-path-buf: $(objpfx)tst-dl-path-buf-mod.so $(shared-thread-library) +LDFLAGS-tst-dl-path-buf += -Wl,-rpath,\$$ORIGIN/tst-dl-path-buf-subdir +tst-dl-path-buf-TUNABLES = glibc.mem.decorate_maps=1 +tst-dl-path-buf-ENV = MALLOC_TRACE=$(objpfx)tst-dl-path-buf.mtrace \ + LD_PRELOAD=$(common-objpfx)/malloc/libc_malloc_debug.so + +$(objpfx)tst-dl-path-buf-mem.out: $(objpfx)tst-dl-path-buf.out + $(common-objpfx)malloc/mtrace $(objpfx)tst-dl-path-buf.mtrace > $@; \ + $(evaluate-test) + $(objpfx)tst-deep1mod2.so: $(objpfx)tst-deep1mod3.so $(objpfx)tst-deep1: $(objpfx)tst-deep1mod1.so $(objpfx)tst-deep1.out: $(objpfx)tst-deep1mod2.so diff --git a/elf/dl-load.c b/elf/dl-load.c index d20e49d526..204faffaa0 100644 --- a/elf/dl-load.c +++ b/elf/dl-load.c @@ -33,6 +33,7 @@ #include <sys/types.h> #include <gnu/lib-names.h> #include <dl-tunables.h> +#include <dl-scratch-buffer.h> #include "dynamic-link.h" #include "get-dynamic-info.h" @@ -96,7 +97,9 @@ is_trusted_path_normalize (const char *path, size_t len) if (len == 0) return false; - char *npath = (char *) alloca (len + 2); + struct dl_scratch_buffer scratch = dl_scratch_buffer_init (); + dl_scratch_buffer_allocate (&scratch, len + 2, 0); + char *npath = scratch.data; char *wnp = npath; while (*path != '\0') { @@ -131,19 +134,24 @@ is_trusted_path_normalize (const char *path, size_t len) if (wnp == npath || wnp[-1] != '/') *wnp++ = '/'; + bool result = false; const char *trun = system_dirs; for (size_t idx = 0; idx < nsystem_dirs_len; ++idx) { if (wnp - npath >= system_dirs_len[idx] && memcmp (trun, npath, system_dirs_len[idx]) == 0) - /* Found it. */ - return true; + { + /* Found it. */ + result = true; + break; + } trun += system_dirs_len[idx] + 1; } - return false; + dl_scratch_buffer_free (&scratch); + return result; } /* Given a substring starting at INPUT, just after the DST '$' start @@ -1470,12 +1478,17 @@ cannot enable executable stack as shared object requires"); return l; } -/* Print search path. */ +/* Print search path. BUF is a scratch buffer provided by the caller; + it must be large enough to hold the longest "<dirname><capstr>" plus + a trailing NUL byte -- i.e. at least + max_dirnamelen + max_capstrlen + 1 bytes. open_path's path buffer + (max_dirnamelen + max_capstrlen + namelen, namelen >= 1) is reused + here so that enabling LD_DEBUG=libs does not require an extra mmap + per call. */ static void print_search_path (struct r_search_path_elem **list, - const char *what, const char *name) + const char *what, const char *name, char *buf) { - char buf[max_dirnamelen + max_capstrlen]; int first = 1; _dl_debug_printf (" search path="); @@ -1725,7 +1738,6 @@ open_path (const char *name, size_t namelen, int mode, bool *found_other_class) { struct r_search_path_elem **dirs = sps->dirs; - char *buf; int fd = -1; const char *current_what = NULL; int any = 0; @@ -1735,7 +1747,18 @@ open_path (const char *name, size_t namelen, int mode, given on the command line when rtld is run directly. */ return -1; - buf = alloca (max_dirnamelen + max_capstrlen + namelen); + /* The scratch buffer below is sized to satisfy both this function's + candidate-path construction (max_dirnamelen + max_capstrlen + namelen) + and print_search_path's buffer precondition + (max_dirnamelen + max_capstrlen + 1). An empty NAME would under-size the + buffer for the latter and would also produce a meaningless lookup (the + loader rejects empty names well before reaching here). */ + assert (namelen >= 1); + + size_t bufsize = max_dirnamelen + max_capstrlen + namelen; + struct dl_scratch_buffer scratch = dl_scratch_buffer_init (); + dl_scratch_buffer_allocate (&scratch, bufsize, 0); + char *buf = scratch.data; do { struct r_search_path_elem *this_dir = *dirs; @@ -1750,7 +1773,7 @@ open_path (const char *name, size_t namelen, int mode, && current_what != this_dir->what) { current_what = this_dir->what; - print_search_path (dirs, current_what, this_dir->where); + print_search_path (dirs, current_what, this_dir->where, buf); } edp = (char *) __mempcpy (buf, this_dir->dirname, this_dir->dirnamelen); @@ -1838,6 +1861,7 @@ open_path (const char *name, size_t namelen, int mode, if (*realname != NULL) { memcpy (*realname, buf, buflen); + dl_scratch_buffer_free (&scratch); return fd; } else @@ -1845,6 +1869,7 @@ open_path (const char *name, size_t namelen, int mode, /* No memory for the name, we certainly won't be able to load and link it. */ __close_nocancel (fd); + dl_scratch_buffer_free (&scratch); return -1; } } @@ -1854,7 +1879,10 @@ open_path (const char *name, size_t namelen, int mode, directory (for instance, if the component is a existing file meaning essentially that the pathname is invalid - ENOTDIR). */ if (here_any && errno != ENOENT && errno != EACCES && errno != ENOTDIR) - return -1; + { + dl_scratch_buffer_free (&scratch); + return -1; + } /* Remember whether we found anything. */ any |= here_any; @@ -1875,6 +1903,7 @@ open_path (const char *name, size_t namelen, int mode, sps->dirs = (void *) -1; } + dl_scratch_buffer_free (&scratch); return -1; } diff --git a/elf/tst-dl-path-buf-mod.c b/elf/tst-dl-path-buf-mod.c new file mode 100644 index 0000000000..5642ea985a --- /dev/null +++ b/elf/tst-dl-path-buf-mod.c @@ -0,0 +1,23 @@ +/* Trivial DSO used by tst-dl-path-buf. + Copyright (C) 2026 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/>. */ + +unsigned int +tst_dl_path_buf_mod_value (void) +{ + return 0xaabbccddu; +} diff --git a/elf/tst-dl-path-buf.c b/elf/tst-dl-path-buf.c new file mode 100644 index 0000000000..487de1fa0b --- /dev/null +++ b/elf/tst-dl-path-buf.c @@ -0,0 +1,220 @@ +/* Exercise the mmap-backed path scratch buffer in elf/dl-load.c. + Copyright (C) 2026 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/>. */ + +/* open_path()'s scratch buffer comes from dl_scratch_buffer, which uses + anonymous mmap while __minimal_malloc is active (during loader startup) + and libc malloc afterwards. Mappings from the mmap backend are tagged + with the VMA name " glibc: loader scratch". This test exercises the + relevant code paths -- search via DT_RPATH, open_path failure cleanup, + dlopen with an over-long name, dlopen from a minimal-stack thread, + and per-backend leak checks -- to verify each path properly + alloc/frees the scratch buffer. */ + +#include <dlfcn.h> +#include <errno.h> +#include <limits.h> +#include <mcheck.h> +#include <pthread.h> +#include <stdio.h> +#include <stdlib.h> +#include <string.h> +#include <unistd.h> +#include <sys/stat.h> + +#include <support/check.h> +#include <support/support.h> +#include <support/temp_file.h> +#include <support/xdlfcn.h> +#include <support/xstdio.h> +#include <support/xthread.h> +#include <support/xunistd.h> + +#ifndef PATH_MAX +# define PATH_MAX 1024 +#endif + +/* Must match LDFLAGS-tst-dl-path-buf in elf/Makefile. The test binary's + DT_RPATH resolves to $ORIGIN of the binary plus this subdirectory. */ +#define MOD_SUBDIR "tst-dl-path-buf-subdir" +#define MOD_NAME "tst-dl-path-buf-mod.so" + +/* Tag installed by _dl_scratch_buffer_allocate via __set_vma_name. Mappings + in /proc/self/maps annotated with this string belong to a live scratch + buffer; after a successful dlopen/dlclose cycle there must be zero of + them. */ +#define SCRATCH_VMA_TAG "[anon: glibc: loader scratch]" + +/* Open MOD_NAME via DT_RPATH. Returns the handle; the caller closes it. */ +static void +dlopen_module (void) +{ + void *h = xdlopen (MOD_NAME, RTLD_NOW | RTLD_LOCAL); + unsigned int (*fn) (void) = xdlsym (h, "tst_dl_path_buf_mod_value"); + TEST_COMPARE (fn (), 0xaabbccddu); + xdlclose (h); +} + +/* Subtest 1: basic dlopen/dlclose via DT_RPATH search. */ +static void +test_basic (void) +{ + dlopen_module (); +} + +/* Subtest 2: non-existent name: open_path is exercised on every search list + (DT_RPATH then cache then __rtld_search_dirs), failing each time. Each + failure path must release its scratch buffer. */ +static void +test_nonexistent (void) +{ + void *h = dlopen ("tst-dl-path-buf-does-not-exist.so", + RTLD_NOW | RTLD_LOCAL); + TEST_VERIFY (h == NULL); +} + +/* Subtest 3: a name whose resolved length far exceeds PATH_MAX cannot refer + to a real file: open_path will allocate a large scratch buffer, build + candidate paths, and have every open() return ENAMETOOLONG. dlopen must + therefore fail cleanly (and without leaking the scratch buffer on the + failure paths). */ +static void +test_overlong_name (void) +{ + char *huge = xmalloc (PATH_MAX + 64); + memset (huge, 'a', PATH_MAX + 32); + memcpy (huge + PATH_MAX, ".so", 4); + + void *h = dlopen (huge, RTLD_NOW | RTLD_LOCAL); + TEST_VERIFY (h == NULL); + + free (huge); +} + +/* Count anonymous mappings in /proc/self/maps annotated with SCRATCH_VMA_TAG. + Used by the mmap-backend leak subtest below. */ +static unsigned int +count_scratch_mappings (void) +{ + FILE *f = xfopen ("/proc/self/maps", "r"); + unsigned int n = 0; + char *line = NULL; + size_t line_len = 0; + while (xgetline (&line, &line_len, f)) + if (strstr (line, SCRATCH_VMA_TAG) != NULL) + ++n; + free (line); + xfclose (f); + return n; +} + +/* Subtest 4a (mmap backend). dl_scratch_buffer's mmap backend is used while + __rtld_malloc_is_complete returns false -- that window covers the entire + loader-startup phase, during which the loader resolves the test binary's + DT_NEEDED dependencies via open_path() (and so allocates and frees scratch + buffers). */ +static void +test_no_leak_mmap (void) +{ + if (!support_set_vma_name_supported ()) + { + printf ("info: skipping mmap-backend leak subtest:" + " kernel does not support PR_SET_VMA_ANON_NAME\n"); + return; + } + + unsigned int residual = count_scratch_mappings (); + if (residual != 0) + FAIL_EXIT1 ("%u leaked loader scratch mapping(s) survived loader" + " startup -- _dl_scratch_buffer_free's mmap backend" + " is broken", residual); +} + +/* Subtest 4b (malloc backend). Once libc malloc is active, + dl_scratch_buffer_allocate routes through malloc and the mapping VMA tag is + no longer used. Drive enough dlopen success+failure cycles to exercise + every path in dl_scratch_buffer_free. */ +static void +test_no_leak_malloc (void) +{ + mtrace (); + + enum { iterations = 10 }; + for (unsigned int i = 0; i < iterations; ++i) + { + dlopen_module (); + void *nh = dlopen ("tst-dl-path-buf-does-not-exist.so", + RTLD_NOW | RTLD_LOCAL); + TEST_VERIFY (nh == NULL); + } +} + +/* Subtest 5. Run the success path from a PTHREAD_STACK_MIN thread. */ +static void * +minstack_thread (void *closure) +{ + dlopen_module (); + void *nh = dlopen ("tst-dl-path-buf-does-not-exist.so", + RTLD_NOW | RTLD_LOCAL); + TEST_VERIFY (nh == NULL); + return NULL; +} + +static void +test_minstack (void) +{ + size_t stacksize = support_small_thread_stack_size (true); + + pthread_attr_t attr; + xpthread_attr_init (&attr); + xpthread_attr_setstacksize (&attr, stacksize); + pthread_t thr = xpthread_create (&attr, minstack_thread, NULL); + xpthread_join (thr); + xpthread_attr_destroy (&attr); +} + +static int +do_test (void) +{ + support_need_proc ("/proc/self/maps is read for the leak subtest."); + + char *subdir = xasprintf ("%s/elf/" MOD_SUBDIR, support_objdir_root); + xmkdirp (subdir, 0777); + add_temp_file (subdir); + + char *src = xasprintf ("%s/elf/" MOD_NAME, support_objdir_root); + char *dst = xasprintf ("%s/" MOD_NAME, subdir); + support_copy_file (src, dst); + add_temp_file (dst); + + /* Check the mmap backend's startup behavior first, before any + subtest can perturb /proc/self/maps with its own allocations. */ + test_no_leak_mmap (); + + test_basic (); + test_nonexistent (); + test_overlong_name (); + test_no_leak_malloc (); + test_minstack (); + + free (src); + free (dst); + free (subdir); + return 0; +} + +#include <support/test-driver.c>