From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: by sourceware.org (Postfix, from userid 1791) id A73F2385BF93; Tue, 16 Nov 2021 13:58:05 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org A73F2385BF93 Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit From: Adhemerval Zanella To: glibc-cvs@sourceware.org Subject: [glibc/azanella/ld-audit-fixes] elf: Avoid unnecessary slowdown from profiling with audit (BZ#15533) X-Act-Checkin: glibc X-Git-Author: Adhemerval Zanella X-Git-Refname: refs/heads/azanella/ld-audit-fixes X-Git-Oldrev: d083bace72c2f66d5a89c6c665a82397d8021f99 X-Git-Newrev: 0216888480abe2afb32167a95e2c6aa5ba05e44b Message-Id: <20211116135805.A73F2385BF93@sourceware.org> Date: Tue, 16 Nov 2021 13:58:05 +0000 (GMT) X-BeenThere: glibc-cvs@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Glibc-cvs mailing list List-Unsubscribe: , List-Archive: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 16 Nov 2021 13:58:05 -0000 https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=0216888480abe2afb32167a95e2c6aa5ba05e44b commit 0216888480abe2afb32167a95e2c6aa5ba05e44b Author: Adhemerval Zanella Date: Wed Jun 30 10:24:09 2021 -0300 elf: Avoid unnecessary slowdown from profiling with audit (BZ#15533) The rtld-audit interfaces introduces a slowdown due to enabling profiling instrumentation (as if LD_AUDIT implied LD_PROFILE). However, instrumenting is only necessary if one of audit libraries provides PLT callbacks ( la_pltenter or la_pltexit symbols). Otherwise, the slowdown can be avoided. The following patch adjusts the logic that enables profiling to iterate over all audit modules and check if any of those provides a PLT hook. To keep la_symbind() to work even without PLT callbacks, _dl_fixup now calls the audit callback if the modules implements it. Co-authored-by: Alexander Monakov Checked on x86_64-linux-gnu, i686-linux-gnu, and aarch64-linux-gnu. Diff: --- NEWS | 4 ++ elf/Makefile | 12 ++++++ elf/dl-reloc.c | 20 ++++++++- elf/dl-runtime.c | 110 +++++++++++++++----------------------------------- elf/rtld.c | 8 +--- elf/tst-audit19a.c | 39 ++++++++++++++++++ elf/tst-audit19b.c | 94 ++++++++++++++++++++++++++++++++++++++++++ elf/tst-audit19bmod.c | 23 +++++++++++ elf/tst-auditmod19a.c | 23 +++++++++++ elf/tst-auditmod19b.c | 46 +++++++++++++++++++++ include/link.h | 2 + 11 files changed, 294 insertions(+), 87 deletions(-) diff --git a/NEWS b/NEWS index f10971b180..427cd2a27d 100644 --- a/NEWS +++ b/NEWS @@ -184,6 +184,10 @@ Major new features: than or equal to a given integer. This function is a GNU extension, although Solaris also provides a similar function. +* The audit libraries will avoid unnecessary slowdown if it is not required + PLT tracking (by not implementing the la_pltenter() or la_pltexit() + callbacks). + Deprecated and removed features, and other changes affecting compatibility: * The function pthread_mutex_consistent_np has been deprecated; programs diff --git a/elf/Makefile b/elf/Makefile index 80ed31edbe..d2a71f73f1 100644 --- a/elf/Makefile +++ b/elf/Makefile @@ -231,6 +231,8 @@ tests += restest1 preloadtest loadfail multiload origtest resolvfail \ tst-tls20 tst-tls21 tst-dlmopen-dlerror tst-dlmopen-gethostbyname \ tst-dl-is_dso tst-ro-dynamic \ tst-audit18 \ + tst-audit19a \ + tst-audit19b \ # reldep9 tests-internal += loadtest unload unload2 circleload1 \ neededtest neededtest2 neededtest3 neededtest4 \ @@ -373,6 +375,9 @@ modules-names = testobj1 testobj2 testobj3 testobj4 testobj5 testobj6 \ tst-dlmopen-gethostbyname-mod tst-ro-dynamic-mod \ tst-auditmod18 \ tst-audit18mod \ + tst-auditmod19a \ + tst-auditmod19b \ + tst-audit19bmod \ # Most modules build with _ISOMAC defined, but those filtered out # depend on internal headers. @@ -1544,6 +1549,13 @@ $(objpfx)tst-audit18.out: $(objpfx)tst-auditmod18.so \ $(objpfx)tst-audit18mod.so tst-audit18-ARGS = -- $(host-test-program-cmd) +$(objpfx)tst-audit19a.out: $(objpfx)tst-auditmod19a.so +tst-audit19a-ENV = LD_AUDIT=$(objpfx)tst-auditmod19a.so + +$(objpfx)tst-audit19b.out: $(objpfx)tst-auditmod19b.so +$(objpfx)tst-audit19b: $(objpfx)tst-audit19bmod.so +tst-audit19b-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 \ diff --git a/elf/dl-reloc.c b/elf/dl-reloc.c index 0d5b727c64..64a96c36e8 100644 --- a/elf/dl-reloc.c +++ b/elf/dl-reloc.c @@ -205,12 +205,28 @@ _dl_relocate_object (struct link_map *l, struct r_scope_elem *scope[], int skip_ifunc = reloc_mode & __RTLD_NOIFUNC; #ifdef SHARED + bool consider_symbind = false; /* If we are auditing, install the same handlers we need for profiling. */ if ((reloc_mode & __RTLD_AUDIT) == 0) - consider_profiling |= GLRO(dl_audit) != NULL; + { + struct audit_ifaces *afct = GLRO(dl_audit); + for (unsigned int cnt = 0; cnt < GLRO(dl_naudit); ++cnt) + { + /* Profiling is needed only if PLT hooks are provided. */ + if (afct->ARCH_LA_PLTENTER != NULL + || afct->ARCH_LA_PLTEXIT != NULL) + consider_profiling = 1; + if (afct->symbind != NULL) + consider_symbind = true; + + afct = afct->next; + } + } #elif defined PROF /* Never use dynamic linker profiling for gprof profiling code. */ # define consider_profiling 0 +#else +# define consider_symbind 0 #endif if (l->l_relocated) @@ -272,7 +288,7 @@ _dl_relocate_object (struct link_map *l, struct r_scope_elem *scope[], ELF_DYNAMIC_RELOCATE (l, scope, lazy, consider_profiling, skip_ifunc); #ifndef PROF - if (__glibc_unlikely (consider_profiling) + if (consider_profiling | consider_symbind && l->l_info[DT_PLTRELSZ] != NULL) { /* Allocate the array which will contain the already found diff --git a/elf/dl-runtime.c b/elf/dl-runtime.c index 03da689503..9a38eea7cc 100644 --- a/elf/dl-runtime.c +++ b/elf/dl-runtime.c @@ -131,6 +131,37 @@ _dl_fixup ( && __builtin_expect (ELFW(ST_TYPE) (sym->st_info) == STT_GNU_IFUNC, 0)) value = elf_ifunc_invoke (DL_FIXUP_VALUE_ADDR (value)); +#ifdef SHARED + /* Auditing checkpoint: we have a new binding. Provide the auditing + libraries the possibility to change the value and tell us whether further + auditing is wanted. + The l_reloc_result is only allocated if there is an audit module which + provides a la_symbind(). */ + if (l->l_reloc_result != NULL) + { + /* This is the address in the array where we store the result of previous + relocations. */ + struct reloc_result *reloc_result + = &l->l_reloc_result[reloc_index (pltgot, reloc_arg, sizeof (PLTREL))]; + unsigned int init = atomic_load_acquire (&reloc_result->init); + if (init == 0) + { + _dl_audit_symbind (l, reloc_result, sym, &value, result); + + /* Store the result for later runs. */ + if (__glibc_likely (! GLRO(dl_bind_not))) + { + reloc_result->addr = value; + /* Guarantee all previous writes complete before init is + updated. See CONCURRENCY NOTES below. */ + atomic_store_release (&reloc_result->init, 1); + } + } + else + value = reloc_result->addr; + } +#endif + /* Finally, fix up the plt itself. */ if (__glibc_unlikely (GLRO(dl_bind_not))) return value; @@ -290,84 +321,7 @@ _dl_profile_fixup ( auditing libraries the possibility to change the value and tell us whether further auditing is wanted. */ if (defsym != NULL && GLRO(dl_naudit) > 0) - { - reloc_result->bound = result; - /* Compute index of the symbol entry in the symbol table of - the DSO with the definition. */ - reloc_result->boundndx = (defsym - - (ElfW(Sym) *) D_PTR (result, - l_info[DT_SYMTAB])); - - /* Determine whether any of the two participating DSOs is - interested in auditing. */ - if ((l->l_audit_any_plt | result->l_audit_any_plt) != 0) - { - unsigned int flags = 0; - struct audit_ifaces *afct = GLRO(dl_audit); - /* Synthesize a symbol record where the st_value field is - the result. */ - ElfW(Sym) sym = *defsym; - sym.st_value = DL_FIXUP_VALUE_ADDR (value); - - /* Keep track whether there is any interest in tracing - the call in the lower two bits. */ - assert (DL_NNS * 2 <= sizeof (reloc_result->flags) * 8); - assert ((LA_SYMB_NOPLTENTER | LA_SYMB_NOPLTEXIT) == 3); - reloc_result->enterexit = LA_SYMB_NOPLTENTER | LA_SYMB_NOPLTEXIT; - - const char *strtab2 = (const void *) D_PTR (result, - l_info[DT_STRTAB]); - - for (unsigned int cnt = 0; cnt < GLRO(dl_naudit); ++cnt) - { - /* XXX Check whether both DSOs must request action or - only one */ - struct auditstate *l_state = link_map_audit_state (l, cnt); - struct auditstate *result_state - = link_map_audit_state (result, cnt); - if ((l_state->bindflags & LA_FLG_BINDFROM) != 0 - && (result_state->bindflags & LA_FLG_BINDTO) != 0) - { - if (afct->symbind != NULL) - { - uintptr_t new_value - = afct->symbind (&sym, reloc_result->boundndx, - &l_state->cookie, - &result_state->cookie, - &flags, - strtab2 + defsym->st_name); - if (new_value != (uintptr_t) sym.st_value) - { - flags |= LA_SYMB_ALTVALUE; - sym.st_value = new_value; - } - } - - /* Remember the results for every audit library and - store a summary in the first two bits. */ - reloc_result->enterexit - &= flags & (LA_SYMB_NOPLTENTER | LA_SYMB_NOPLTEXIT); - reloc_result->enterexit - |= ((flags & (LA_SYMB_NOPLTENTER | LA_SYMB_NOPLTEXIT)) - << ((cnt + 1) * 2)); - } - else - /* If the bind flags say this auditor is not interested, - set the bits manually. */ - reloc_result->enterexit - |= ((LA_SYMB_NOPLTENTER | LA_SYMB_NOPLTEXIT) - << ((cnt + 1) * 2)); - - afct = afct->next; - } - - reloc_result->flags = flags; - value = DL_FIXUP_ADDR_VALUE (sym.st_value); - } - else - /* Set all bits since this symbol binding is not interesting. */ - reloc_result->enterexit = (1u << DL_NNS) - 1; - } + _dl_audit_symbind (l, reloc_result, defsym, &value, result); #endif /* Store the result for later runs. */ diff --git a/elf/rtld.c b/elf/rtld.c index dc20643ee9..73a852ad42 100644 --- a/elf/rtld.c +++ b/elf/rtld.c @@ -1015,13 +1015,7 @@ ERROR: audit interface '%s' requires version %d (maximum supported version %d); "la_objsearch\0" "la_objopen\0" "la_preinit\0" -#if __ELF_NATIVE_CLASS == 32 - "la_symbind32\0" -#elif __ELF_NATIVE_CLASS == 64 - "la_symbind64\0" -#else -# error "__ELF_NATIVE_CLASS must be defined" -#endif + LA_SYMBIND "\0" #define STRING(s) __STRING (s) "la_" STRING (ARCH_LA_PLTENTER) "\0" "la_" STRING (ARCH_LA_PLTEXIT) "\0" diff --git a/elf/tst-audit19a.c b/elf/tst-audit19a.c new file mode 100644 index 0000000000..36b781f9be --- /dev/null +++ b/elf/tst-audit19a.c @@ -0,0 +1,39 @@ +/* Check if DT_AUDIT a module without la_plt{enter,exit} symbols does not incur + in profiling (BZ#15533). + 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 + . */ + +#include +#include + +/* We interpose the profile resolver and if it is called it means profiling is + enabled. */ +void +_dl_runtime_profile (ElfW(Word) addr) +{ + volatile int *p = NULL; + *p = 0; +} + +static int +do_test (void) +{ + printf ("..."); + return 0; +} + +#include diff --git a/elf/tst-audit19b.c b/elf/tst-audit19b.c new file mode 100644 index 0000000000..300197ca2e --- /dev/null +++ b/elf/tst-audit19b.c @@ -0,0 +1,94 @@ +/* Check if DT_AUDIT a module with la_plt{enter,exit} call la_symbind() + for lazy resolution. + 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 + . */ + +#include +#include +#include +#include +#include +#include +#include + +static int restart; +#define CMDLINE_OPTIONS \ + { "restart", no_argument, &restart, 1 }, + +int tst_audit18bmod1_func (void); + +static int +handle_restart (void) +{ + TEST_COMPARE (tst_audit18bmod1_func (), 10); + return 0; +} + +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[]) +{ + /* 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-auditmod18b.so", 0); + struct support_capture_subprocess result + = support_capture_subprogram (spargv[0], spargv); + support_capture_subprocess_check (&result, "tst-audit18b", 0, sc_allow_stderr); + + bool find_symbind = false; + + 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, "la_symbind: tst_audit18bmod1_func") == 0) + find_symbind = true; + + TEST_COMPARE (find_symbind, true); + + free (buffer); + xfclose (out); + + return 0; +} + +#define TEST_FUNCTION_ARGV do_test +#include diff --git a/elf/tst-audit19bmod.c b/elf/tst-audit19bmod.c new file mode 100644 index 0000000000..9ffdcd8f3f --- /dev/null +++ b/elf/tst-audit19bmod.c @@ -0,0 +1,23 @@ +/* Extra module for tst-audit18b. + 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 + . */ + +int +tst_audit18bmod1_func (void) +{ + return 10; +} diff --git a/elf/tst-auditmod19a.c b/elf/tst-auditmod19a.c new file mode 100644 index 0000000000..2296382a1c --- /dev/null +++ b/elf/tst-auditmod19a.c @@ -0,0 +1,23 @@ +/* Audit module for tst-audit18a. + 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 + . */ + +unsigned int +la_version (unsigned int version) +{ + return version; +} diff --git a/elf/tst-auditmod19b.c b/elf/tst-auditmod19b.c new file mode 100644 index 0000000000..52bb88c7d7 --- /dev/null +++ b/elf/tst-auditmod19b.c @@ -0,0 +1,46 @@ +/* Audit module for tst-audit18b. + 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 + . */ + +#include +#include +#include + +unsigned int +la_version (unsigned int version) +{ + return version; +} + +unsigned int +la_objopen (struct link_map *map, Lmid_t lmid, uintptr_t *cookie) +{ + return LA_FLG_BINDTO | LA_FLG_BINDFROM; +} + +uintptr_t +#if __ELF_NATIVE_CLASS == 32 +la_symbind32 (Elf32_Sym *sym, unsigned int ndx, uintptr_t *refcook, + uintptr_t *defcook, unsigned int *flags, const char *symname) +#else +la_symbind64 (Elf64_Sym *sym, unsigned int ndx, uintptr_t *refcook, + uintptr_t *defcook, unsigned int *flags, const char *symname) +#endif +{ + fprintf (stderr, "la_symbind: %s\n", symname); + return sym->st_value; +} diff --git a/include/link.h b/include/link.h index c1c382ccfa..6a9f788d2b 100644 --- a/include/link.h +++ b/include/link.h @@ -367,8 +367,10 @@ extern struct r_debug_extended _r_debug_extended attribute_hidden; #if __ELF_NATIVE_CLASS == 32 # define symbind symbind32 +# define LA_SYMBIND "la_symbind32" #elif __ELF_NATIVE_CLASS == 64 # define symbind symbind64 +# define LA_SYMBIND "la_symbind64" #else # error "__ELF_NATIVE_CLASS must be defined" #endif