This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH] elf: Support dlvsym within libc.so
- From: Carlos O'Donell <carlos at redhat dot com>
- To: Florian Weimer <fweimer at redhat dot com>, libc-alpha at sourceware dot org
- Date: Thu, 4 Jan 2018 09:36:58 -0800
- Subject: Re: [PATCH] elf: Support dlvsym within libc.so
- Authentication-results: sourceware.org; auth=none
- References: <20171218145711.CF2EB43994576@oldenburg.str.redhat.com> <7c8a04e2-4a8b-da09-e629-7a29c1793959@redhat.com> <306bf0ee-9731-8818-eefc-bd015f9df0ac@redhat.com> <5c6c338a-ddce-ca56-6144-8907546ced3a@redhat.com>
On 01/03/2018 07:16 AM, Florian Weimer wrote:
> New patch attached. This one uses a separate hook variable, so that
> __libc_dlvsym fails in a predictable manner with old statically
> linked binaries.
>
> I would like to backport the libidn2 switch, so this level of
> compatibility is desirable.
This version 2 looks good to me.
Reviewed-by: Carlos O'Donell <carlos@redhat.com>
> Subject: [PATCH] elf: Support dlvsym within libc.so
> To: libc-alpha@sourceware.org
>
> This commit adds a new _dl_open_hook entry for dlvsym and implements the
> function using the existing dl_lookup_symbol_x function supplied by the
> dynamic loader.
>
> A new hook variable, _dl_open_hook2, is introduced, which should make
> this change suitable for backporting: For old statically linked
> binaries, __libc_dlvsym will always return NULL.
>
> 2018-01-03 Florian Weimer <fweimer@redhat.com>
>
> Add support for calling dlvsym from libc.so.
> * include/dlfcn.h (__libc_dlvsym): Declare.
> * elf/Makefile (tests-static-internal): Add
> tst-libc_dlvsym-static.
> (tests-internal): Add tst-libc_dlvsym.
> (modules-names): Add tst-libc_dlvsym-dso.
> (tst-libc_dlvsym, tst-libc_dlvsym-static): Link with libdl.
> (tst-libc_dlvsym-dso.so): Link with libdl, libsupport.
> (tst-libc_dlvsym.out, tst-libc_dlvsym-static.out): The shared
> object tst-libc_dlvsym-dso.so needs to be built before running
> these tests.
> (tst-libc_dlvsym-static-ENV): Set LD_LIBRARY_PATH.
> * elf/Versions: Export __libc_dlvsym.
> * elf/dl-libc.c (struct do_dlvsym_args): New.
> (do_dlvsym, __libc_dlvsym): New functions.
> (struct dl_open_hook, _dl_open_hook): Add dlvsym member.
> (_dl_open_hook2): New variable.
> (__libc_register_dl_open_hook): Set it.
> * elf/tst-libc_dlvsym-dso.c: New file.
> * elf/tst-libc_dlvsym-static.c: Likewise.
> * elf/tst-libc_dlvsym.c: Likewise.
> * elf/tst-libc_dlvsym.h: Likewise.
>
> diff --git a/elf/Makefile b/elf/Makefile
> index 8967ac2685..2a432d8bee 100644
> --- a/elf/Makefile
> +++ b/elf/Makefile
> @@ -151,7 +151,7 @@ tests-static-normal := tst-leaks1-static tst-array1-static tst-array5-static \
> tst-linkall-static tst-env-setuid tst-env-setuid-tunables
> tests-static-internal := tst-tls1-static tst-tls2-static \
> tst-ptrguard1-static tst-stackguard1-static \
> - tst-tls1-static-non-pie
> + tst-tls1-static-non-pie tst-libc_dlvsym-static
OK.
>
> CRT-tst-tls1-static-non-pie := $(csu-objpfx)crt1.o
> tst-tls1-static-non-pie-no-pie = yes
> @@ -192,7 +192,7 @@ tests += restest1 preloadtest loadfail multiload origtest resolvfail \
> tests-internal += loadtest unload unload2 circleload1 \
> neededtest neededtest2 neededtest3 neededtest4 \
> tst-tls3 tst-tls6 tst-tls7 tst-tls8 tst-dlmopen2 \
> - tst-ptrguard1 tst-stackguard1
> + tst-ptrguard1 tst-stackguard1 tst-libc_dlvsym
OK.
> ifeq ($(build-hardcoded-path-in-tests),yes)
> tests += tst-dlopen-aout
> tst-dlopen-aout-no-pie = yes
> @@ -272,7 +272,7 @@ modules-names = testobj1 testobj2 testobj3 testobj4 testobj5 testobj6 \
> tst-audit12mod1 tst-audit12mod2 tst-audit12mod3 tst-auditmod12 \
> tst-latepthreadmod $(tst-tls-many-dynamic-modules) \
> tst-nodelete-dlclose-dso tst-nodelete-dlclose-plugin \
> - tst-main1mod
> + tst-main1mod tst-libc_dlvsym-dso
OK.
> ifeq (yes,$(have-mtls-dialect-gnu2))
> tests += tst-gnu2-tls1
> modules-names += tst-gnu2-tls1mod
> @@ -1436,3 +1436,13 @@ CRT-tst-main1 := $(csu-objpfx)crt1.o
> tst-main1-no-pie = yes
> LDLIBS-tst-main1 = $(libsupport)
> tst-main1mod.so-no-z-defs = yes
> +
> +# Both the main program and the DSO for tst-libc_dlvsym need to link
> +# against libdl.
> +$(objpfx)tst-libc_dlvsym: $(libdl)
> +$(objpfx)tst-libc_dlvsym-dso.so: $(libsupport) $(libdl)
> +$(objpfx)tst-libc_dlvsym.out: $(objpfx)tst-libc_dlvsym-dso.so
> +$(objpfx)tst-libc_dlvsym-static: $(common-objpfx)dlfcn/libdl.a
> +tst-libc_dlvsym-static-ENV = \
> + LD_LIBRARY_PATH=$(objpfx):$(common-objpfx):$(common-objpfx)dlfcn
> +$(objpfx)tst-libc_dlvsym-static.out: $(objpfx)tst-libc_dlvsym-dso.so
OK.
> diff --git a/elf/Versions b/elf/Versions
> index 79ffaf73d2..3b09901f6c 100644
> --- a/elf/Versions
> +++ b/elf/Versions
> @@ -23,9 +23,9 @@ libc {
> GLIBC_PRIVATE {
> # functions used in other libraries
> _dl_addr;
> - _dl_open_hook;
> + _dl_open_hook; _dl_open_hook2;
OK.
> _dl_sym; _dl_vsym;
> - __libc_dlclose; __libc_dlopen_mode; __libc_dlsym;
> + __libc_dlclose; __libc_dlopen_mode; __libc_dlsym; __libc_dlvsym;
OK.
>
> # Internal error handling support. Interposes the functions in ld.so.
> _dl_signal_exception; _dl_catch_exception;
> diff --git a/elf/dl-libc.c b/elf/dl-libc.c
> index c6818e0313..fc01f5514d 100644
> --- a/elf/dl-libc.c
> +++ b/elf/dl-libc.c
> @@ -20,6 +20,7 @@
> #include <dlfcn.h>
> #include <stdlib.h>
> #include <ldsodefs.h>
> +#include <dl-hash.h>
OK.
>
> extern int __libc_argc attribute_hidden;
> extern char **__libc_argv attribute_hidden;
> @@ -78,6 +79,15 @@ struct do_dlsym_args
> const ElfW(Sym) *ref;
> };
>
> +struct do_dlvsym_args
> +{
> + /* dlvsym is like dlsym. */
> + struct do_dlsym_args dlsym;
> +
> + /* But dlvsym needs a version as well. */
> + struct r_found_version version;
> +};
OK. Like the use of closure.
> +
> static void
> do_dlopen (void *ptr)
> {
> @@ -99,6 +109,18 @@ do_dlsym (void *ptr)
> }
>
> static void
> +do_dlvsym (void *ptr)
> +{
> + struct do_dlvsym_args *args = ptr;
> + args->dlsym.ref = NULL;
> + args->dlsym.loadbase
> + = GLRO(dl_lookup_symbol_x) (args->dlsym.name, args->dlsym.map,
> + &args->dlsym.ref,
> + args->dlsym.map->l_local_scope,
> + &args->version, 0, 0, NULL);
> +}
OK.
> +
> +static void
> do_dlclose (void *ptr)
> {
> GLRO(dl_close) ((struct link_map *) ptr);
> @@ -112,6 +134,7 @@ struct dl_open_hook
> void *(*dlopen_mode) (const char *name, int mode);
> void *(*dlsym) (void *map, const char *name);
> int (*dlclose) (void *map);
> + void *(*dlvsym) (void *map, const char *name, const char *version);
OK.
> };
>
> #ifdef SHARED
> @@ -119,6 +142,15 @@ extern struct dl_open_hook *_dl_open_hook;
> libc_hidden_proto (_dl_open_hook);
> struct dl_open_hook *_dl_open_hook __attribute__ ((nocommon));
> libc_hidden_data_def (_dl_open_hook);
> +
> +/* The dlvsym member was added retroactively to struct dl_open_hook.
> + Static applications which have it will set _dl_open_hook2 in
> + addition to _dl_open_hook. */
> +extern struct dl_open_hook *_dl_open_hook2;
> +libc_hidden_proto (_dl_open_hook2);
> +struct dl_open_hook *_dl_open_hook2 __attribute__ ((nocommon));
> +libc_hidden_data_def (_dl_open_hook2);
OK.
> +
> #else
> static void
> do_dlsym_private (void *ptr)
> @@ -142,7 +174,8 @@ static struct dl_open_hook _dl_open_hook =
> {
> .dlopen_mode = __libc_dlopen_mode,
> .dlsym = __libc_dlsym,
> - .dlclose = __libc_dlclose
> + .dlclose = __libc_dlclose,
> + .dlvsym = __libc_dlvsym,
OK.
> };
> #endif
>
> @@ -192,6 +225,11 @@ __libc_register_dl_open_hook (struct link_map *map)
> hook = (struct dl_open_hook **) __libc_dlsym_private (map, "_dl_open_hook");
> if (hook != NULL)
> *hook = &_dl_open_hook;
> +
> + /* For dlvsym support. */
> + hook = (struct dl_open_hook **) __libc_dlsym_private (map, "_dl_open_hook2");
> + if (hook != NULL)
> + *hook = &_dl_open_hook;
OK, if _dl_open_hook2 is available, use that instead.
> }
> #endif
>
> @@ -211,6 +249,39 @@ __libc_dlsym (void *map, const char *name)
> }
> libc_hidden_def (__libc_dlsym)
>
> +/* Replacement for dlvsym. MAP must be a real map. This function
> + returns NULL without setting the dlerror value in case of static
> + dlopen from an old binary. */
> +void *
> +__libc_dlvsym (void *map, const char *name, const char *version)
> +{
> +#ifdef SHARED
> + if (!rtld_active ())
> + {
> + /* The static application is too old and does not provide the
> + dlvsym hook. */
> + if (_dl_open_hook2 == NULL)
> + return NULL;
> + return _dl_open_hook2->dlvsym (map, name, version);
> + }
OK. I like this compat.
> +#endif
> +
> + struct do_dlvsym_args args;
> + args.dlsym.map = map;
> + args.dlsym.name = name;
> +
> + /* See _dl_vsym in dl-sym.c. */
> + args.version.name = version;
> + args.version.hidden = 1;
> + args.version.hash = _dl_elf_hash (version);
> + args.version.filename = NULL;
> +
> + return (dlerror_run (do_dlvsym, &args) ? NULL
> + : (void *) (DL_SYMBOL_ADDRESS (args.dlsym.loadbase,
> + args.dlsym.ref)));
OK.
> +}
> +libc_hidden_def (__libc_dlvsym)
> +
> int
> __libc_dlclose (void *map)
> {
> diff --git a/elf/tst-libc_dlvsym-dso.c b/elf/tst-libc_dlvsym-dso.c
> new file mode 100644
> index 0000000000..d74c1ead61
> --- /dev/null
> +++ b/elf/tst-libc_dlvsym-dso.c
> @@ -0,0 +1,25 @@
> +/* Compare dlvsym and __libc_dlvsym results. Shared object code.
> + Copyright (C) 2017 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
> + <http://www.gnu.org/licenses/>. */
> +
> +#include "tst-libc_dlvsym.h"
> +
> +void
> +compare_vsyms_global (void)
> +{
> + compare_vsyms ();
> +}
OK.
> diff --git a/elf/tst-libc_dlvsym-static.c b/elf/tst-libc_dlvsym-static.c
> new file mode 100644
> index 0000000000..955f28d754
> --- /dev/null
> +++ b/elf/tst-libc_dlvsym-static.c
> @@ -0,0 +1,32 @@
> +/* Compare dlvsym and __libc_dlvsym results. Static version.
> + Copyright (C) 2017 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
> + <http://www.gnu.org/licenses/>. */
> +
> +#include <support/xdlfcn.h>
> +
> +static int
> +do_test (void)
> +{
> + void *handle = xdlopen ("tst-libc_dlvsym-dso.so", RTLD_LAZY);
> + void (*compare) (void) = xdlsym (handle, "compare_vsyms_global");
> + compare ();
> + xdlclose (handle);
> +
> + return 0;
> +}
OK.
> +
> +#include <support/test-driver.c>
> diff --git a/elf/tst-libc_dlvsym.c b/elf/tst-libc_dlvsym.c
> new file mode 100644
> index 0000000000..864db4ee6a
> --- /dev/null
> +++ b/elf/tst-libc_dlvsym.c
> @@ -0,0 +1,34 @@
> +/* Compare dlvsym and __libc_dlvsym results. Dynamic version.
> + Copyright (C) 2017 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
> + <http://www.gnu.org/licenses/>. */
> +
> +#include "tst-libc_dlvsym.h"
> +
> +static int
> +do_test (void)
> +{
> + compare_vsyms ();
> +
> + void *handle = xdlopen ("tst-libc_dlvsym-dso.so", RTLD_LAZY);
> + void (*compare) (void) = xdlsym (handle, "compare_vsyms_global");
> + compare ();
> + xdlclose (handle);
> +
> + return 0;
> +}
OK.
> +
> +#include <support/test-driver.c>
> diff --git a/elf/tst-libc_dlvsym.h b/elf/tst-libc_dlvsym.h
> new file mode 100644
> index 0000000000..6b1d8d5b3a
> --- /dev/null
> +++ b/elf/tst-libc_dlvsym.h
> @@ -0,0 +1,125 @@
> +/* Compare dlvsym and __libc_dlvsym results. Common code.
> + Copyright (C) 2017 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
> + <http://www.gnu.org/licenses/>. */
> +
> +/* compare_vsyms is the main entry point for these tests.
> +
> + Indirectly, It calls __libc_dlvsym (from libc.so; internal
> + interface) and dlvsym (from libdl.so; public interface) to compare
> + the results for a selected set of symbols in libc.so which
> + typically have more than one symbol version. The two functions are
> + implemented by somewhat different code, and this test checks that
> + their results are the same.
> +
> + The versions are generated to range from GLIBC_2.0 to GLIBC_2.Y,
> + with Y being the current __GLIBC_MINOR__ version plus two. In
> + addition, there is a list of special symbol versions of the form
> + GLIBC_2.Y.Z, which were used for some releases.
> +
> + Comparing the two dlvsym results at versions which do not actually
> + exist does not test much, but it will not contribute to false test
> + failures, either. */
OK. Excellent comment.
> +
> +#include <array_length.h>
> +#include <gnu/lib-names.h>
> +#include <stdbool.h>
> +#include <stdio.h>
> +#include <support/check.h>
> +#include <support/xdlfcn.h>
> +
> +/* Run consistency check for versioned symbol NAME@VERSION. NB: We
> + may execute in a shared object, so exit on error for proper error
> + reporting. */
> +static void
> +compare_vsyms_0 (void *libc_handle, const char *name, const char *version,
> + bool *pfound)
> +{
> + void *dlvsym_address = dlvsym (libc_handle, name, version);
> + void *libc_dlvsym_address
> + = __libc_dlvsym (libc_handle, name, version);
> + if (dlvsym_address != libc_dlvsym_address)
> + FAIL_EXIT1 ("%s@%s mismatch: %p != %p",
> + name, version, dlvsym_address, libc_dlvsym_address);
> + if (dlvsym_address != NULL)
> + *pfound = true;
> +}
OK.
> +
> +
> +/* Run consistency check for versioned symbol NAME at multiple symbol
> + version. */
> +static void
> +compare_vsyms_1 (void *libc_handle, const char *name)
> +{
> + bool found = false;
> +
> + /* Historic versions which do not follow the usual GLIBC_2.Y
> + pattern, to increase test coverage. Not all architectures have
> + those, but probing additional versions does not hurt. */
> + static const char special_versions[][12] =
> + {
> + "GLIBC_2.1.1",
> + "GLIBC_2.1.2",
> + "GLIBC_2.1.3",
> + "GLIBC_2.1.4",
> + "GLIBC_2.2.1",
> + "GLIBC_2.2.2",
> + "GLIBC_2.2.3",
> + "GLIBC_2.2.4",
> + "GLIBC_2.2.5",
> + "GLIBC_2.2.6",
> + "GLIBC_2.3.2",
> + "GLIBC_2.3.3",
> + "GLIBC_2.3.4",
> + };
OK.
> + for (int i = 0; i < array_length (special_versions); ++i)
> + compare_vsyms_0 (libc_handle, name, special_versions[i], &found);
> +
> + /* Iterate to an out-of-range version, to cover some unused symbols
> + as well. */
OK.
> + for (int minor_version = 0; minor_version <= __GLIBC_MINOR__ + 2;
> + ++minor_version)
> + {
> + char version[30];
> + snprintf (version, sizeof (version), "GLIBC_%d.%d",
> + __GLIBC__, minor_version);
> + compare_vsyms_0 (libc_handle, name, version, &found);
> + }
> +
> + if (!found)
> + FAIL_EXIT1 ("symbol %s not found at any version", name);
> +}
> +
> +/* Run consistency checks for various symbols which usually have
> + multiple versions. */
> +static void
> +compare_vsyms (void)
> +{
> + /* The minor version loop in compare_vsyms_1 needs updating in case
> + we ever switch to glibc 3.0. */
OK. Good note.
> + if (__GLIBC__ != 2)
> + FAIL_EXIT1 ("unexpected glibc major version: %d", __GLIBC__);
> +
> + /* __libc_dlvsym does not recognize the special RTLD_* handles, so
> + obtain an explicit handle for libc.so. */
> + void *libc_handle = xdlopen (LIBC_SO, RTLD_LAZY | RTLD_NOLOAD);
> +
> + compare_vsyms_1 (libc_handle, "_sys_errlist");
> + compare_vsyms_1 (libc_handle, "_sys_siglist");
> + compare_vsyms_1 (libc_handle, "quick_exit");
> +
> + xdlclose (libc_handle);
> +}
> diff --git a/include/dlfcn.h b/include/dlfcn.h
> index 526086f1a0..12ef913e19 100644
> --- a/include/dlfcn.h
> +++ b/include/dlfcn.h
> @@ -35,9 +35,11 @@ extern char **__libc_argv attribute_hidden;
> __libc_dlopen_mode (name, RTLD_LAZY | __RTLD_DLOPEN)
> extern void *__libc_dlopen_mode (const char *__name, int __mode);
> extern void *__libc_dlsym (void *__map, const char *__name);
> +extern void *__libc_dlvsym (void *map, const char *name, const char *version);
> extern int __libc_dlclose (void *__map);
> libc_hidden_proto (__libc_dlopen_mode)
> libc_hidden_proto (__libc_dlsym)
> +libc_hidden_proto (__libc_dlvsym)
OK.
> libc_hidden_proto (__libc_dlclose)
>
> /* Locate shared object containing the given address. */
--
Cheers,
Carlos.