[v6,14/20] elf: Issue audit la_objopen() for vDSO
Checks
| Context |
Check |
Description |
| dj/TryBot-apply_patch |
success
|
Patch applied to master at the time it was sent
|
Commit Message
The vDSO is is listed in the link_map chain, but is never the subject of
an la_objopen call. A new internal flag __RTLD_VDSO is added that
acts as __RTLD_OPENEXEC to allocate the required 'struct auditstate'
extra space for the 'struct link_map'.
The return value from the callback() is currently ignored, since there
is no PLT call involved by glibc when using the vDSO, neither the vDSO
are exported directly.
Checked on x86_64-linux-gnu, i686-linux-gnu, and aarch64-linux-gnu.
---
elf/Makefile | 5 ++
elf/dl-object.c | 17 +++---
elf/rtld.c | 7 +++
elf/setup-vdso.h | 2 +-
elf/tst-audit22.c | 124 +++++++++++++++++++++++++++++++++++++++++++
elf/tst-auditmod22.c | 51 ++++++++++++++++++
include/dlfcn.h | 1 +
7 files changed, 199 insertions(+), 8 deletions(-)
create mode 100644 elf/tst-audit22.c
create mode 100644 elf/tst-auditmod22.c
Comments
* Adhemerval Zanella:
> The vDSO is is listed in the link_map chain, but is never the subject of
> an la_objopen call. A new internal flag __RTLD_VDSO is added that
> acts as __RTLD_OPENEXEC to allocate the required 'struct auditstate'
> extra space for the 'struct link_map'.
>
> The return value from the callback() is currently ignored, since there
> is no PLT call involved by glibc when using the vDSO, neither the vDSO
> are exported directly.
The usual comment about (). 8-)
> diff --git a/elf/dl-object.c b/elf/dl-object.c
> index eb2158a84b..6f26da4310 100644
> --- a/elf/dl-object.c
> +++ b/elf/dl-object.c
> @@ -59,16 +59,19 @@ _dl_new_object (char *realname, const char *libname, int type,
> {
> #ifdef SHARED
> unsigned int naudit;
> - if (__glibc_unlikely ((mode & __RTLD_OPENEXEC) != 0))
> + if (__glibc_unlikely ((mode & (__RTLD_OPENEXEC | __RTLD_VDSO)) != 0))
> {
> - assert (type == lt_executable);
> - assert (nsid == LM_ID_BASE);
> + if (mode & __RTLD_OPENEXEC)
> + {
> + assert (type == lt_executable);
> + assert (nsid == LM_ID_BASE);
>
> - /* Ignore the specified libname for the main executable. It is
> - only known with an explicit loader invocation. */
> - libname = "";
> + /* Ignore the specified libname for the main executable. It is
> + only known with an explicit loader invocation. */
> + libname = "";
> + }
>
> - /* We create the map for the executable before we know whether
> + /* We create the map for the executable and vDSO before we know whether
> we have auditing libraries and if yes, how many. Assume the
> worst. */
> naudit = DL_NNS;
Okay, we call _dl_new_object early for the vDSO, too, so the comment is
appropriate.
> diff --git a/elf/tst-audit22.c b/elf/tst-audit22.c
> new file mode 100644
> index 0000000000..f136f25a32
> --- /dev/null
> +++ b/elf/tst-audit22.c
> + /* The respawned process should always print the vDSO address (otherwise it
> + will fails as unsupported). However, on some architectures the audit
> + module might see the vDSO with l_addr being 0, meaning a fixed mapping
> + (linux-gate.so). In this case we don't its value against AT_SYSINFO_EHDR
> + one. */
“we don't [check] its value”
> + uintptr_t vdso_process = 0;
> + bool vdso_audit_found = false;
> + uintptr_t vdso_audit = 0;
> +
> + FILE *out = fmemopen (result.err.buffer, result.err.length, "r");
> + TEST_VERIFY (out != NULL);
> + char *buffer = NULL;
> + size_t buffer_length = 0;
> + while (xgetline (&buffer, &buffer_length, out))
> + {
> + if (startswith (buffer, "vdso: "))
> + vdso_process = parse_address (buffer + strlen ("vdso "));
"vdso: " (with colon), but sscanf already skips the leading space.
> +#define TEST_FUNCTION_ARGV do_test
> +#include <support/test-driver.c>
> diff --git a/elf/tst-auditmod22.c b/elf/tst-auditmod22.c
> new file mode 100644
> index 0000000000..8e05ce8cbb
> +static inline bool
> +startswith (const char *str, const char *pre)
> +{
> + size_t lenpre = strlen (pre);
> + size_t lenstr = strlen (str);
> + return lenstr < lenpre ? false : memcmp (pre, str, lenpre) == 0;
> +}
lenstr >= lenpre && memcmp (pre, str, lenpre) == 0
> +
> +unsigned int
> +la_version (unsigned int version)
> +{
> + return LAV_CURRENT;
> +}
> +
> +unsigned int
> +la_objopen (struct link_map *map, Lmid_t lmid, uintptr_t *cookie)
> +{
> + /* The linux-gate.so is placed at a fixed address, thus l_addr being 0,
> + and it might be the value reported as the AT_SYSINFO_EHDR. */
> + if (map->l_addr == 0 && startswith (map->l_name, "linux-gate.so"))
> + fprintf (stderr, "vdso found: %p\n", NULL);
> + else if (map->l_addr == getauxval (AT_SYSINFO_EHDR))
> + fprintf (stderr, "vdso found: %p\n", (void*) map->l_addr);
> +
> + return 0;
> +}
Would it be possible to look at the program headers to get the minimum
mapped address for the linux-gate.so object?
> diff --git a/include/dlfcn.h b/include/dlfcn.h
> index a4c283728f..66bcf2dff9 100644
> --- a/include/dlfcn.h
> +++ b/include/dlfcn.h
> @@ -12,6 +12,7 @@
> #define __RTLD_AUDIT 0x08000000
> #define __RTLD_SECURE 0x04000000 /* Apply additional security checks. */
> #define __RTLD_NOIFUNC 0x02000000 /* Suppress calling ifunc functions. */
> +#define __RTLD_VDSO 0x01000000
>
> #define __LM_ID_CALLER -2
The __RTLD_VDSO definition is unique, okay. But maybe add a comment,
like “Tell _dl_new_object the object is system-loaded.”
Thanks,
Florian
On 18/12/2021 17:00, Florian Weimer wrote:
> * Adhemerval Zanella:
>
>> The vDSO is is listed in the link_map chain, but is never the subject of
>> an la_objopen call. A new internal flag __RTLD_VDSO is added that
>> acts as __RTLD_OPENEXEC to allocate the required 'struct auditstate'
>> extra space for the 'struct link_map'.
>>
>> The return value from the callback() is currently ignored, since there
>> is no PLT call involved by glibc when using the vDSO, neither the vDSO
>> are exported directly.
>
> The usual comment about (). 8-)
Ack.
>
>> diff --git a/elf/dl-object.c b/elf/dl-object.c
>> index eb2158a84b..6f26da4310 100644
>> --- a/elf/dl-object.c
>> +++ b/elf/dl-object.c
>> @@ -59,16 +59,19 @@ _dl_new_object (char *realname, const char *libname, int type,
>> {
>> #ifdef SHARED
>> unsigned int naudit;
>> - if (__glibc_unlikely ((mode & __RTLD_OPENEXEC) != 0))
>> + if (__glibc_unlikely ((mode & (__RTLD_OPENEXEC | __RTLD_VDSO)) != 0))
>> {
>> - assert (type == lt_executable);
>> - assert (nsid == LM_ID_BASE);
>> + if (mode & __RTLD_OPENEXEC)
>> + {
>> + assert (type == lt_executable);
>> + assert (nsid == LM_ID_BASE);
>>
>> - /* Ignore the specified libname for the main executable. It is
>> - only known with an explicit loader invocation. */
>> - libname = "";
>> + /* Ignore the specified libname for the main executable. It is
>> + only known with an explicit loader invocation. */
>> + libname = "";
>> + }
>>
>> - /* We create the map for the executable before we know whether
>> + /* We create the map for the executable and vDSO before we know whether
>> we have auditing libraries and if yes, how many. Assume the
>> worst. */
>> naudit = DL_NNS;
>
> Okay, we call _dl_new_object early for the vDSO, too, so the comment is
> appropriate.
>
>> diff --git a/elf/tst-audit22.c b/elf/tst-audit22.c
>> new file mode 100644
>> index 0000000000..f136f25a32
>> --- /dev/null
>> +++ b/elf/tst-audit22.c
>
>> + /* The respawned process should always print the vDSO address (otherwise it
>> + will fails as unsupported). However, on some architectures the audit
>> + module might see the vDSO with l_addr being 0, meaning a fixed mapping
>> + (linux-gate.so). In this case we don't its value against AT_SYSINFO_EHDR
>> + one. */
>
> “we don't [check] its value”
Ack.
>
>> + uintptr_t vdso_process = 0;
>> + bool vdso_audit_found = false;
>> + uintptr_t vdso_audit = 0;
>> +
>> + FILE *out = fmemopen (result.err.buffer, result.err.length, "r");
>> + TEST_VERIFY (out != NULL);
>> + char *buffer = NULL;
>> + size_t buffer_length = 0;
>> + while (xgetline (&buffer, &buffer_length, out))
>> + {
>> + if (startswith (buffer, "vdso: "))
>> + vdso_process = parse_address (buffer + strlen ("vdso "));
>
> "vdso: " (with colon), but sscanf already skips the leading space.
Ack.
>
>> +#define TEST_FUNCTION_ARGV do_test
>> +#include <support/test-driver.c>
>> diff --git a/elf/tst-auditmod22.c b/elf/tst-auditmod22.c
>> new file mode 100644
>> index 0000000000..8e05ce8cbb
>
>> +static inline bool
>> +startswith (const char *str, const char *pre)
>> +{
>> + size_t lenpre = strlen (pre);
>> + size_t lenstr = strlen (str);
>> + return lenstr < lenpre ? false : memcmp (pre, str, lenpre) == 0;
>> +}
>
> lenstr >= lenpre && memcmp (pre, str, lenpre) == 0
Ack.
>
>> +
>> +unsigned int
>> +la_version (unsigned int version)
>> +{
>> + return LAV_CURRENT;
>> +}
>> +
>> +unsigned int
>> +la_objopen (struct link_map *map, Lmid_t lmid, uintptr_t *cookie)
>> +{
>> + /* The linux-gate.so is placed at a fixed address, thus l_addr being 0,
>> + and it might be the value reported as the AT_SYSINFO_EHDR. */
>> + if (map->l_addr == 0 && startswith (map->l_name, "linux-gate.so"))
>> + fprintf (stderr, "vdso found: %p\n", NULL);
>> + else if (map->l_addr == getauxval (AT_SYSINFO_EHDR))
>> + fprintf (stderr, "vdso found: %p\n", (void*) map->l_addr);
>> +
>> + return 0;
>> +}
>
> Would it be possible to look at the program headers to get the minimum
> mapped address for the linux-gate.so object?
It should be possible, but I only saw it on a ia64 machine (which I don't
have access anymore) and reading the kernel source linux-gate.so is
provided also for x86, sh, and sparc 32-bits kernels. I am not sure
if it worth the trouble.
>
>> diff --git a/include/dlfcn.h b/include/dlfcn.h
>> index a4c283728f..66bcf2dff9 100644
>> --- a/include/dlfcn.h
>> +++ b/include/dlfcn.h
>> @@ -12,6 +12,7 @@
>> #define __RTLD_AUDIT 0x08000000
>> #define __RTLD_SECURE 0x04000000 /* Apply additional security checks. */
>> #define __RTLD_NOIFUNC 0x02000000 /* Suppress calling ifunc functions. */
>> +#define __RTLD_VDSO 0x01000000
>>
>> #define __LM_ID_CALLER -2
>
> The __RTLD_VDSO definition is unique, okay. But maybe add a comment,
> like “Tell _dl_new_object the object is system-loaded.”
Ack.
>
> Thanks,
> Florian
>
* Adhemerval Zanella:
>>> +unsigned int
>>> +la_version (unsigned int version)
>>> +{
>>> + return LAV_CURRENT;
>>> +}
>>> +
>>> +unsigned int
>>> +la_objopen (struct link_map *map, Lmid_t lmid, uintptr_t *cookie)
>>> +{
>>> + /* The linux-gate.so is placed at a fixed address, thus l_addr being 0,
>>> + and it might be the value reported as the AT_SYSINFO_EHDR. */
>>> + if (map->l_addr == 0 && startswith (map->l_name, "linux-gate.so"))
>>> + fprintf (stderr, "vdso found: %p\n", NULL);
>>> + else if (map->l_addr == getauxval (AT_SYSINFO_EHDR))
>>> + fprintf (stderr, "vdso found: %p\n", (void*) map->l_addr);
>>> +
>>> + return 0;
>>> +}
>>
>> Would it be possible to look at the program headers to get the minimum
>> mapped address for the linux-gate.so object?
>
> It should be possible, but I only saw it on a ia64 machine (which I don't
> have access anymore) and reading the kernel source linux-gate.so is
> provided also for x86, sh, and sparc 32-bits kernels. I am not sure
> if it worth the trouble.
Fair enough, keep the current version then.
Thanks,
Florian
@@ -235,6 +235,7 @@ tests += restest1 preloadtest loadfail multiload origtest resolvfail \
tst-audit19b \
tst-audit20 \
tst-audit21 \
+ tst-audit22 \
# reldep9
tests-internal += loadtest unload unload2 circleload1 \
neededtest neededtest2 neededtest3 neededtest4 \
@@ -383,6 +384,7 @@ modules-names = testobj1 testobj2 testobj3 testobj4 testobj5 testobj6 \
tst-auditmod20 \
tst-auditmod21a \
tst-auditmod21b \
+ tst-auditmod22 \
# Most modules build with _ISOMAC defined, but those filtered out
# depend on internal headers.
@@ -1583,6 +1585,9 @@ $(objpfx)tst-audit21.out: $(objpfx)tst-auditmod21a.so
$(objpfx)tst-auditmod21a.so: $(objpfx)tst-auditmod21b.so
tst-audit21-ENV = LD_AUDIT=$(objpfx)tst-auditmod21a.so
+$(objpfx)tst-audit22.out: $(objpfx)tst-auditmod22.so
+tst-audit22-ARGS = -- $(host-test-program-cmd)
+
# tst-sonamemove links against an older implementation of the library.
LDFLAGS-tst-sonamemove-linkmod1.so = \
-Wl,--version-script=tst-sonamemove-linkmod1.map \
@@ -59,16 +59,19 @@ _dl_new_object (char *realname, const char *libname, int type,
{
#ifdef SHARED
unsigned int naudit;
- if (__glibc_unlikely ((mode & __RTLD_OPENEXEC) != 0))
+ if (__glibc_unlikely ((mode & (__RTLD_OPENEXEC | __RTLD_VDSO)) != 0))
{
- assert (type == lt_executable);
- assert (nsid == LM_ID_BASE);
+ if (mode & __RTLD_OPENEXEC)
+ {
+ assert (type == lt_executable);
+ assert (nsid == LM_ID_BASE);
- /* Ignore the specified libname for the main executable. It is
- only known with an explicit loader invocation. */
- libname = "";
+ /* Ignore the specified libname for the main executable. It is
+ only known with an explicit loader invocation. */
+ libname = "";
+ }
- /* We create the map for the executable before we know whether
+ /* We create the map for the executable and vDSO before we know whether
we have auditing libraries and if yes, how many. Assume the
worst. */
naudit = DL_NNS;
@@ -1879,6 +1879,13 @@ dl_main (const ElfW(Phdr) *phdr,
assert (i == npreloads);
}
+#ifdef NEED_DL_SYSINFO_DSO
+ /* Now that the audit modules are opened, call la_objopen() for the
+ vDSO. */
+ if (GLRO(dl_sysinfo_map) != NULL)
+ _dl_audit_objopen (GLRO(dl_sysinfo_map), LM_ID_BASE, true);
+#endif
+
/* Load all the libraries specified by DT_NEEDED entries. If LD_PRELOAD
specified some libraries to load, these are inserted before the actual
dependencies in the executable's searchlist for symbol resolution. */
@@ -30,7 +30,7 @@ setup_vdso (struct link_map *main_map __attribute__ ((unused)),
We just want our data structures to describe it as if we had just
mapped and relocated it normally. */
struct link_map *l = _dl_new_object ((char *) "", "", lt_library, NULL,
- 0, LM_ID_BASE);
+ __RTLD_VDSO, LM_ID_BASE);
if (__glibc_likely (l != NULL))
{
l->l_phdr = ((const void *) GLRO(dl_sysinfo_dso)
new file mode 100644
@@ -0,0 +1,124 @@
+/* Check DTAUDIT and vDSO interaction.
+ 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/>. */
+
+#include <errno.h>
+#include <getopt.h>
+#include <limits.h>
+#include <inttypes.h>
+#include <string.h>
+#include <stdlib.h>
+#include <support/capture_subprocess.h>
+#include <support/check.h>
+#include <support/xstdio.h>
+#include <support/support.h>
+#include <sys/auxv.h>
+
+static int restart;
+#define CMDLINE_OPTIONS \
+ { "restart", no_argument, &restart, 1 },
+
+static uintptr_t vdso_addr;
+
+static int
+handle_restart (void)
+{
+ fprintf (stderr, "vdso: %p\n", (void*) vdso_addr);
+ return 0;
+}
+
+static uintptr_t
+parse_address (const char *str)
+{
+ void *r;
+ TEST_COMPARE (sscanf (str, "%p\n", &r), 1);
+ return (uintptr_t) r;
+}
+
+static inline bool
+startswith (const char *str, const char *pre)
+{
+ size_t lenpre = strlen (pre);
+ size_t lenstr = strlen (str);
+ return lenstr < lenpre ? false : memcmp (pre, str, lenpre) == 0;
+}
+
+static int
+do_test (int argc, char *argv[])
+{
+ vdso_addr = getauxval (AT_SYSINFO_EHDR);
+ if (vdso_addr == 0)
+ FAIL_UNSUPPORTED ("getauxval (AT_SYSINFO_EHDR) returned 0");
+
+ /* We must have either:
+ - One our fource parameters left if called initially:
+ + path to ld.so optional
+ + "--library-path" optional
+ + the library path optional
+ + the application name */
+ if (restart)
+ return handle_restart ();
+
+ char *spargv[9];
+ int i = 0;
+ for (; i < argc - 1; i++)
+ spargv[i] = argv[i + 1];
+ spargv[i++] = (char *) "--direct";
+ spargv[i++] = (char *) "--restart";
+ spargv[i] = NULL;
+
+ setenv ("LD_AUDIT", "tst-auditmod22.so", 0);
+ struct support_capture_subprocess result
+ = support_capture_subprogram (spargv[0], spargv);
+ support_capture_subprocess_check (&result, "tst-audit22", 0, sc_allow_stderr);
+
+ /* The respawned process should always print the vDSO address (otherwise it
+ will fails as unsupported). However, on some architectures the audit
+ module might see the vDSO with l_addr being 0, meaning a fixed mapping
+ (linux-gate.so). In this case we don't its value against AT_SYSINFO_EHDR
+ one. */
+ uintptr_t vdso_process = 0;
+ bool vdso_audit_found = false;
+ uintptr_t vdso_audit = 0;
+
+ FILE *out = fmemopen (result.err.buffer, result.err.length, "r");
+ TEST_VERIFY (out != NULL);
+ char *buffer = NULL;
+ size_t buffer_length = 0;
+ while (xgetline (&buffer, &buffer_length, out))
+ {
+ if (startswith (buffer, "vdso: "))
+ vdso_process = parse_address (buffer + strlen ("vdso "));
+ else if (startswith (buffer, "vdso found: "))
+ {
+ vdso_audit = parse_address (buffer + strlen ("vdso found: "));
+ vdso_audit_found = true;
+ }
+ }
+
+ TEST_COMPARE (vdso_audit_found, true);
+ if (vdso_audit != 0)
+ TEST_COMPARE (vdso_process, vdso_audit);
+
+ free (buffer);
+ xfclose (out);
+
+ return 0;
+}
+
+#define TEST_FUNCTION_ARGV do_test
+#include <support/test-driver.c>
new file mode 100644
@@ -0,0 +1,51 @@
+/* Check DTAUDIT and vDSO interaction.
+ 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/>. */
+
+#include <link.h>
+#include <inttypes.h>
+#include <stdbool.h>
+#include <string.h>
+#include <stdio.h>
+#include <sys/auxv.h>
+
+static inline bool
+startswith (const char *str, const char *pre)
+{
+ size_t lenpre = strlen (pre);
+ size_t lenstr = strlen (str);
+ return lenstr < lenpre ? false : memcmp (pre, str, lenpre) == 0;
+}
+
+unsigned int
+la_version (unsigned int version)
+{
+ return LAV_CURRENT;
+}
+
+unsigned int
+la_objopen (struct link_map *map, Lmid_t lmid, uintptr_t *cookie)
+{
+ /* The linux-gate.so is placed at a fixed address, thus l_addr being 0,
+ and it might be the value reported as the AT_SYSINFO_EHDR. */
+ if (map->l_addr == 0 && startswith (map->l_name, "linux-gate.so"))
+ fprintf (stderr, "vdso found: %p\n", NULL);
+ else if (map->l_addr == getauxval (AT_SYSINFO_EHDR))
+ fprintf (stderr, "vdso found: %p\n", (void*) map->l_addr);
+
+ return 0;
+}
@@ -12,6 +12,7 @@
#define __RTLD_AUDIT 0x08000000
#define __RTLD_SECURE 0x04000000 /* Apply additional security checks. */
#define __RTLD_NOIFUNC 0x02000000 /* Suppress calling ifunc functions. */
+#define __RTLD_VDSO 0x01000000
#define __LM_ID_CALLER -2