[glibc/azanella/ld-audit-fixes] elf: Avoid unnecessary slowdown from profiling with audit (BZ#15533)

public inbox for glibc-cvs@sourceware.org
help / color / mirror / Atom feed

* [glibc/azanella/ld-audit-fixes] elf: Avoid unnecessary slowdown from profiling with audit (BZ#15533)
@ 2021-07-30 19:24 Adhemerval Zanella
  0 siblings, 0 replies; 6+ messages in thread
From: Adhemerval Zanella @ 2021-07-30 19:24 UTC (permalink / raw)
  To: glibc-cvs

https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=2e687ebe7a86233a6ea9b51e0d998da1ea24fb9f

commit 2e687ebe7a86233a6ea9b51e0d998da1ea24fb9f
Author: Adhemerval Zanella <adhemerval.zanella@linaro.org>
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 <amonakov@ispras.ru>
    
    Checked on x86_64-linux-gnu and i686-linux-gnu.

Diff:
---
 NEWS                  |   3 +
 elf/Makefile          |   9 +++
 elf/dl-reloc.c        |  18 ++++-
 elf/dl-runtime.c      | 188 +++++++++++++++++++++++++++++---------------------
 elf/rtld.c            |   8 +--
 elf/tst-audit18a.c    |  39 +++++++++++
 elf/tst-audit18b.c    |  94 +++++++++++++++++++++++++
 elf/tst-audit18bmod.c |  23 ++++++
 elf/tst-audit18mod.c  |  17 +++++
 elf/tst-auditmod18a.c |  23 ++++++
 elf/tst-auditmod18b.c |  46 ++++++++++++
 include/link.h        |   2 +
 12 files changed, 383 insertions(+), 87 deletions(-)

diff --git a/NEWS b/NEWS
index ea54518142..ca6e058b19 100644
--- a/NEWS
+++ b/NEWS
@@ -76,6 +76,9 @@ Major new features:
   equal to a giver integer.  This function is a GNU extension, although
   Solaris also provides a similar function.
 
+* The audit libraries will avoid unnecessary slowdown if PLT tracking is not
+  required (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 d05f410592..eab664acaf 100644
--- a/elf/Makefile
+++ b/elf/Makefile
@@ -220,6 +220,7 @@ tests += restest1 preloadtest loadfail multiload origtest resolvfail \
 	 tst-dlopenfail-2 \
 	 tst-filterobj tst-filterobj-dlopen tst-auxobj tst-auxobj-dlopen \
 	 tst-audit14 tst-audit15 tst-audit16 tst-audit17 \
+	 tst-audit18a tst-audit18b \
 	 tst-single_threaded tst-single_threaded-pthread \
 	 tst-tls-ie tst-tls-ie-dlmopen argv0test \
 	 tst-glibc-hwcaps tst-glibc-hwcaps-prepend tst-glibc-hwcaps-mask \
@@ -301,6 +302,7 @@ modules-names = testobj1 testobj2 testobj3 testobj4 testobj5 testobj6 \
 		tst-unique1mod1 tst-unique1mod2 \
 		tst-unique2mod1 tst-unique2mod2 \
 		tst-auditmod9a tst-auditmod9b \
+		tst-auditmod18a tst-auditmod18b tst-audit18bmod \
 		$(if $(CXX),tst-unique3lib tst-unique3lib2 tst-unique4lib \
 		  tst-nodelete-uniquemod tst-nodelete-rtldmod \
 		  tst-nodelete-zmod \
@@ -1486,6 +1488,13 @@ $(objpfx)tst-auditmod17.so: $(objpfx)tst-auditmod17.os
 CFLAGS-.os += $(call elide-stack-protector,.os,tst-auditmod17)
 tst-audit17-ENV = LD_AUDIT=$(objpfx)tst-auditmod17.so
 
+$(objpfx)tst-audit18a.out: $(objpfx)tst-auditmod18a.so
+tst-audit18a-ENV = LD_AUDIT=$(objpfx)tst-auditmod18a.so
+
+$(objpfx)tst-audit18b.out: $(objpfx)tst-auditmod18b.so
+$(objpfx)tst-audit18b: $(objpfx)tst-audit18bmod.so
+tst-audit18b-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 e13a672ade..c6bc1e6b4e 100644
--- a/elf/dl-reloc.c
+++ b/elf/dl-reloc.c
@@ -177,11 +177,25 @@ _dl_relocate_object (struct link_map *l, struct r_scope_elem *scope[],
   const char *errstring = NULL;
   int lazy = reloc_mode & RTLD_LAZY;
   int skip_ifunc = reloc_mode & __RTLD_NOIFUNC;
+  bool consider_symbind = false;
 
 #ifdef SHARED
   /* 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
@@ -275,7 +289,7 @@ _dl_relocate_object (struct link_map *l, struct r_scope_elem *scope[],
     ELF_DYNAMIC_RELOCATE (l, 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 9d0d941000..29031099f5 100644
--- a/elf/dl-runtime.c
+++ b/elf/dl-runtime.c
@@ -43,6 +43,84 @@
 # define ARCH_FIXUP_ATTRIBUTE
 #endif
 
+#ifdef SHARED
+static void
+_dl_audit_symbind (struct link_map *l, struct reloc_result *reloc_result,
+		   const ElfW(Sym) *defsym, DL_FIXUP_VALUE_TYPE *value,
+		   lookup_t result)
+{
+  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]));
+
+  if ((l->l_audit_any_plt | result->l_audit_any_plt) == 0)
+    {
+      /* Set all bits since this symbol binding is not interesting.  */
+      reloc_result->enterexit = (1u << DL_NNS) - 1;
+      return;
+    }
+
+  /* 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]);
+
+  unsigned int flags = 0;
+  struct audit_ifaces *afct = GLRO(dl_audit);
+  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);
+}
+#endif
+
 /* This function is called through a special trampoline from the PLT the
    first time each PLT entry is called.  We must perform the relocation
    specified in the PLT of the given shared object, and return the resolved
@@ -138,6 +216,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;
@@ -296,84 +405,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 d733359eaf..374bf86a69 100644
--- a/elf/rtld.c
+++ b/elf/rtld.c
@@ -1013,13 +1013,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-audit18a.c b/elf/tst-audit18a.c
new file mode 100644
index 0000000000..36b781f9be
--- /dev/null
+++ b/elf/tst-audit18a.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
+   <https://www.gnu.org/licenses/>.  */
+
+#include <link.h>
+#include <stdio.h>
+
+/* 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 <support/test-driver.c>
diff --git a/elf/tst-audit18b.c b/elf/tst-audit18b.c
new file mode 100644
index 0000000000..300197ca2e
--- /dev/null
+++ b/elf/tst-audit18b.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
+   <https://www.gnu.org/licenses/>.  */
+
+#include <getopt.h>
+#include <support/capture_subprocess.h>
+#include <support/check.h>
+#include <support/xstdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <stdbool.h>
+
+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 <support/test-driver.c>
diff --git a/elf/tst-audit18bmod.c b/elf/tst-audit18bmod.c
new file mode 100644
index 0000000000..9ffdcd8f3f
--- /dev/null
+++ b/elf/tst-audit18bmod.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
+   <https://www.gnu.org/licenses/>.  */
+
+int
+tst_audit18bmod1_func (void)
+{
+  return 10;
+}
diff --git a/elf/tst-audit18mod.c b/elf/tst-audit18mod.c
new file mode 100644
index 0000000000..b6eb3731ee
--- /dev/null
+++ b/elf/tst-audit18mod.c
@@ -0,0 +1,17 @@
+/* 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
+   <https://www.gnu.org/licenses/>.  */
diff --git a/elf/tst-auditmod18a.c b/elf/tst-auditmod18a.c
new file mode 100644
index 0000000000..2296382a1c
--- /dev/null
+++ b/elf/tst-auditmod18a.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
+   <https://www.gnu.org/licenses/>.  */
+
+unsigned int
+la_version (unsigned int version)
+{
+  return version;
+}
diff --git a/elf/tst-auditmod18b.c b/elf/tst-auditmod18b.c
new file mode 100644
index 0000000000..52bb88c7d7
--- /dev/null
+++ b/elf/tst-auditmod18b.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
+   <https://www.gnu.org/licenses/>.  */
+
+#include <link.h>
+#include <string.h>
+#include <stdio.h>
+
+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 4af16cb596..ebd0f511e2 100644
--- a/include/link.h
+++ b/include/link.h
@@ -355,8 +355,10 @@ struct auditstate
 
 #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


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [glibc/azanella/ld-audit-fixes] elf: Avoid unnecessary slowdown from profiling with audit (BZ#15533)
@ 2021-11-16 13:58 Adhemerval Zanella
  0 siblings, 0 replies; 6+ messages in thread
From: Adhemerval Zanella @ 2021-11-16 13:58 UTC (permalink / raw)
  To: glibc-cvs

https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=0216888480abe2afb32167a95e2c6aa5ba05e44b

commit 0216888480abe2afb32167a95e2c6aa5ba05e44b
Author: Adhemerval Zanella <adhemerval.zanella@linaro.org>
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 <amonakov@ispras.ru>
    
    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
+   <https://www.gnu.org/licenses/>.  */
+
+#include <link.h>
+#include <stdio.h>
+
+/* 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 <support/test-driver.c>
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
+   <https://www.gnu.org/licenses/>.  */
+
+#include <getopt.h>
+#include <support/capture_subprocess.h>
+#include <support/check.h>
+#include <support/xstdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <stdbool.h>
+
+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 <support/test-driver.c>
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
+   <https://www.gnu.org/licenses/>.  */
+
+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
+   <https://www.gnu.org/licenses/>.  */
+
+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
+   <https://www.gnu.org/licenses/>.  */
+
+#include <link.h>
+#include <string.h>
+#include <stdio.h>
+
+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


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [glibc/azanella/ld-audit-fixes] elf: Avoid unnecessary slowdown from profiling with audit (BZ#15533)
@ 2021-11-09 18:18 Adhemerval Zanella
  0 siblings, 0 replies; 6+ messages in thread
From: Adhemerval Zanella @ 2021-11-09 18:18 UTC (permalink / raw)
  To: glibc-cvs

https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=061ca04de0972fab55c6b8f2c92b3b217ac7c45f

commit 061ca04de0972fab55c6b8f2c92b3b217ac7c45f
Author: Adhemerval Zanella <adhemerval.zanella@linaro.org>
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 <amonakov@ispras.ru>
    
    Checked on x86_64-linux-gnu, i686-linux-gnu, and aarch64-linux-gnu.

Diff:
---
 NEWS                  |   4 ++
 elf/Makefile          |  10 +++
 elf/dl-reloc.c        |  18 ++++-
 elf/dl-runtime.c      | 188 +++++++++++++++++++++++++++++---------------------
 elf/rtld.c            |   8 +--
 elf/tst-audit18a.c    |  39 +++++++++++
 elf/tst-audit18b.c    |  94 +++++++++++++++++++++++++
 elf/tst-audit18bmod.c |  23 ++++++
 elf/tst-audit18mod.c  |  17 +++++
 elf/tst-auditmod18a.c |  23 ++++++
 elf/tst-auditmod18b.c |  46 ++++++++++++
 include/link.h        |   2 +
 12 files changed, 385 insertions(+), 87 deletions(-)

diff --git a/NEWS b/NEWS
index 82b7016aef..18a9d7fb5e 100644
--- a/NEWS
+++ b/NEWS
@@ -180,6 +180,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 72004484db..38e2da03a4 100644
--- a/elf/Makefile
+++ b/elf/Makefile
@@ -225,6 +225,8 @@ tests += restest1 preloadtest loadfail multiload origtest resolvfail \
 	 tst-dlopen-self tst-auditmany tst-initfinilazyfail tst-dlopenfail \
 	 tst-dlopenfail-2 \
 	 tst-filterobj tst-filterobj-dlopen tst-auxobj tst-auxobj-dlopen \
+	 tst-audit14 tst-audit15 tst-audit16 tst-audit17 \
+	 tst-audit18a tst-audit18b \
 	 tst-single_threaded tst-single_threaded-pthread \
 	 tst-tls-ie tst-tls-ie-dlmopen argv0test \
 	 tst-glibc-hwcaps tst-glibc-hwcaps-prepend tst-glibc-hwcaps-mask \
@@ -313,6 +315,7 @@ modules-names = testobj1 testobj2 testobj3 testobj4 testobj5 testobj6 \
 		tst-unique1mod1 tst-unique1mod2 \
 		tst-unique2mod1 tst-unique2mod2 \
 		tst-auditmod9a tst-auditmod9b \
+		tst-auditmod18a tst-auditmod18b tst-audit18bmod \
 		$(if $(CXX),tst-unique3lib tst-unique3lib2 tst-unique4lib \
 		  tst-nodelete-uniquemod tst-nodelete-rtldmod \
 		  tst-nodelete-zmod \
@@ -1537,6 +1540,13 @@ $(objpfx)tst-audit16-cmp.out: tst-audit16.exp $(objpfx)tst-audit16.out
 	cmp $^ > $@; \
 	$(evaluate-test)
 
+$(objpfx)tst-audit18a.out: $(objpfx)tst-auditmod18a.so
+tst-audit18a-ENV = LD_AUDIT=$(objpfx)tst-auditmod18a.so
+
+$(objpfx)tst-audit18b.out: $(objpfx)tst-auditmod18b.so
+$(objpfx)tst-audit18b: $(objpfx)tst-audit18bmod.so
+tst-audit18b-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..27f792b7b2 100644
--- a/elf/dl-reloc.c
+++ b/elf/dl-reloc.c
@@ -203,11 +203,25 @@ _dl_relocate_object (struct link_map *l, struct r_scope_elem *scope[],
   const char *errstring = NULL;
   int lazy = reloc_mode & RTLD_LAZY;
   int skip_ifunc = reloc_mode & __RTLD_NOIFUNC;
+  bool consider_symbind = false;
 
 #ifdef SHARED
   /* 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
@@ -272,7 +286,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 61c260ddb8..b0c788eb22 100644
--- a/elf/dl-runtime.c
+++ b/elf/dl-runtime.c
@@ -44,6 +44,84 @@
 # define ARCH_FIXUP_ATTRIBUTE
 #endif
 
+#ifdef SHARED
+static void
+_dl_audit_symbind (struct link_map *l, struct reloc_result *reloc_result,
+		   const ElfW(Sym) *defsym, DL_FIXUP_VALUE_TYPE *value,
+		   lookup_t result)
+{
+  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]));
+
+  if ((l->l_audit_any_plt | result->l_audit_any_plt) == 0)
+    {
+      /* Set all bits since this symbol binding is not interesting.  */
+      reloc_result->enterexit = (1u << DL_NNS) - 1;
+      return;
+    }
+
+  /* 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]);
+
+  unsigned int flags = 0;
+  struct audit_ifaces *afct = GLRO(dl_audit);
+  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);
+}
+#endif
+
 /* This function is called through a special trampoline from the PLT the
    first time each PLT entry is called.  We must perform the relocation
    specified in the PLT of the given shared object, and return the resolved
@@ -139,6 +217,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;
@@ -297,84 +406,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 be2d5d8e74..8953347b00 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-audit18a.c b/elf/tst-audit18a.c
new file mode 100644
index 0000000000..36b781f9be
--- /dev/null
+++ b/elf/tst-audit18a.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
+   <https://www.gnu.org/licenses/>.  */
+
+#include <link.h>
+#include <stdio.h>
+
+/* 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 <support/test-driver.c>
diff --git a/elf/tst-audit18b.c b/elf/tst-audit18b.c
new file mode 100644
index 0000000000..300197ca2e
--- /dev/null
+++ b/elf/tst-audit18b.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
+   <https://www.gnu.org/licenses/>.  */
+
+#include <getopt.h>
+#include <support/capture_subprocess.h>
+#include <support/check.h>
+#include <support/xstdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <stdbool.h>
+
+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 <support/test-driver.c>
diff --git a/elf/tst-audit18bmod.c b/elf/tst-audit18bmod.c
new file mode 100644
index 0000000000..9ffdcd8f3f
--- /dev/null
+++ b/elf/tst-audit18bmod.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
+   <https://www.gnu.org/licenses/>.  */
+
+int
+tst_audit18bmod1_func (void)
+{
+  return 10;
+}
diff --git a/elf/tst-audit18mod.c b/elf/tst-audit18mod.c
new file mode 100644
index 0000000000..b6eb3731ee
--- /dev/null
+++ b/elf/tst-audit18mod.c
@@ -0,0 +1,17 @@
+/* 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
+   <https://www.gnu.org/licenses/>.  */
diff --git a/elf/tst-auditmod18a.c b/elf/tst-auditmod18a.c
new file mode 100644
index 0000000000..2296382a1c
--- /dev/null
+++ b/elf/tst-auditmod18a.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
+   <https://www.gnu.org/licenses/>.  */
+
+unsigned int
+la_version (unsigned int version)
+{
+  return version;
+}
diff --git a/elf/tst-auditmod18b.c b/elf/tst-auditmod18b.c
new file mode 100644
index 0000000000..52bb88c7d7
--- /dev/null
+++ b/elf/tst-auditmod18b.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
+   <https://www.gnu.org/licenses/>.  */
+
+#include <link.h>
+#include <string.h>
+#include <stdio.h>
+
+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


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [glibc/azanella/ld-audit-fixes] elf: Avoid unnecessary slowdown from profiling with audit (BZ#15533)
@ 2021-09-10 18:25 Adhemerval Zanella
  0 siblings, 0 replies; 6+ messages in thread
From: Adhemerval Zanella @ 2021-09-10 18:25 UTC (permalink / raw)
  To: glibc-cvs

https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=79e43559e820e54a9d020aaff739400f95920fc6

commit 79e43559e820e54a9d020aaff739400f95920fc6
Author: Adhemerval Zanella <adhemerval.zanella@linaro.org>
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 <amonakov@ispras.ru>

Diff:
---
 NEWS                  |   4 ++
 elf/Makefile          |  10 +++
 elf/dl-reloc.c        |  18 ++++-
 elf/dl-runtime.c      | 188 +++++++++++++++++++++++++++++---------------------
 elf/rtld.c            |   8 +--
 elf/tst-audit18a.c    |  39 +++++++++++
 elf/tst-audit18b.c    |  94 +++++++++++++++++++++++++
 elf/tst-audit18bmod.c |  23 ++++++
 elf/tst-audit18mod.c  |  17 +++++
 elf/tst-auditmod18a.c |  23 ++++++
 elf/tst-auditmod18b.c |  46 ++++++++++++
 include/link.h        |   2 +
 12 files changed, 385 insertions(+), 87 deletions(-)

diff --git a/NEWS b/NEWS
index 5b014fabbf..c0d5ec0f2f 100644
--- a/NEWS
+++ b/NEWS
@@ -127,6 +127,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 9f3fadc37e..c482d26185 100644
--- a/elf/Makefile
+++ b/elf/Makefile
@@ -219,6 +219,8 @@ tests += restest1 preloadtest loadfail multiload origtest resolvfail \
 	 tst-dlopen-self tst-auditmany tst-initfinilazyfail tst-dlopenfail \
 	 tst-dlopenfail-2 \
 	 tst-filterobj tst-filterobj-dlopen tst-auxobj tst-auxobj-dlopen \
+	 tst-audit14 tst-audit15 tst-audit16 tst-audit17 \
+	 tst-audit18a tst-audit18b \
 	 tst-single_threaded tst-single_threaded-pthread \
 	 tst-tls-ie tst-tls-ie-dlmopen argv0test \
 	 tst-glibc-hwcaps tst-glibc-hwcaps-prepend tst-glibc-hwcaps-mask \
@@ -303,6 +305,7 @@ modules-names = testobj1 testobj2 testobj3 testobj4 testobj5 testobj6 \
 		tst-unique1mod1 tst-unique1mod2 \
 		tst-unique2mod1 tst-unique2mod2 \
 		tst-auditmod9a tst-auditmod9b \
+		tst-auditmod18a tst-auditmod18b tst-audit18bmod \
 		$(if $(CXX),tst-unique3lib tst-unique3lib2 tst-unique4lib \
 		  tst-nodelete-uniquemod tst-nodelete-rtldmod \
 		  tst-nodelete-zmod \
@@ -1488,6 +1491,13 @@ $(objpfx)tst-auditmod17.so: $(objpfx)tst-auditmod17.os
 CFLAGS-.os += $(call elide-stack-protector,.os,tst-auditmod17)
 tst-audit17-ENV = LD_AUDIT=$(objpfx)tst-auditmod17.so
 
+$(objpfx)tst-audit18a.out: $(objpfx)tst-auditmod18a.so
+tst-audit18a-ENV = LD_AUDIT=$(objpfx)tst-auditmod18a.so
+
+$(objpfx)tst-audit18b.out: $(objpfx)tst-auditmod18b.so
+$(objpfx)tst-audit18b: $(objpfx)tst-audit18bmod.so
+tst-audit18b-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 e13a672ade..c6bc1e6b4e 100644
--- a/elf/dl-reloc.c
+++ b/elf/dl-reloc.c
@@ -177,11 +177,25 @@ _dl_relocate_object (struct link_map *l, struct r_scope_elem *scope[],
   const char *errstring = NULL;
   int lazy = reloc_mode & RTLD_LAZY;
   int skip_ifunc = reloc_mode & __RTLD_NOIFUNC;
+  bool consider_symbind = false;
 
 #ifdef SHARED
   /* 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
@@ -275,7 +289,7 @@ _dl_relocate_object (struct link_map *l, struct r_scope_elem *scope[],
     ELF_DYNAMIC_RELOCATE (l, 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 9d0d941000..29031099f5 100644
--- a/elf/dl-runtime.c
+++ b/elf/dl-runtime.c
@@ -43,6 +43,84 @@
 # define ARCH_FIXUP_ATTRIBUTE
 #endif
 
+#ifdef SHARED
+static void
+_dl_audit_symbind (struct link_map *l, struct reloc_result *reloc_result,
+		   const ElfW(Sym) *defsym, DL_FIXUP_VALUE_TYPE *value,
+		   lookup_t result)
+{
+  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]));
+
+  if ((l->l_audit_any_plt | result->l_audit_any_plt) == 0)
+    {
+      /* Set all bits since this symbol binding is not interesting.  */
+      reloc_result->enterexit = (1u << DL_NNS) - 1;
+      return;
+    }
+
+  /* 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]);
+
+  unsigned int flags = 0;
+  struct audit_ifaces *afct = GLRO(dl_audit);
+  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);
+}
+#endif
+
 /* This function is called through a special trampoline from the PLT the
    first time each PLT entry is called.  We must perform the relocation
    specified in the PLT of the given shared object, and return the resolved
@@ -138,6 +216,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;
@@ -296,84 +405,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 878e6480f4..8a2b9903f8 100644
--- a/elf/rtld.c
+++ b/elf/rtld.c
@@ -1013,13 +1013,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-audit18a.c b/elf/tst-audit18a.c
new file mode 100644
index 0000000000..36b781f9be
--- /dev/null
+++ b/elf/tst-audit18a.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
+   <https://www.gnu.org/licenses/>.  */
+
+#include <link.h>
+#include <stdio.h>
+
+/* 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 <support/test-driver.c>
diff --git a/elf/tst-audit18b.c b/elf/tst-audit18b.c
new file mode 100644
index 0000000000..300197ca2e
--- /dev/null
+++ b/elf/tst-audit18b.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
+   <https://www.gnu.org/licenses/>.  */
+
+#include <getopt.h>
+#include <support/capture_subprocess.h>
+#include <support/check.h>
+#include <support/xstdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <stdbool.h>
+
+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 <support/test-driver.c>
diff --git a/elf/tst-audit18bmod.c b/elf/tst-audit18bmod.c
new file mode 100644
index 0000000000..9ffdcd8f3f
--- /dev/null
+++ b/elf/tst-audit18bmod.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
+   <https://www.gnu.org/licenses/>.  */
+
+int
+tst_audit18bmod1_func (void)
+{
+  return 10;
+}
diff --git a/elf/tst-audit18mod.c b/elf/tst-audit18mod.c
new file mode 100644
index 0000000000..b6eb3731ee
--- /dev/null
+++ b/elf/tst-audit18mod.c
@@ -0,0 +1,17 @@
+/* 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
+   <https://www.gnu.org/licenses/>.  */
diff --git a/elf/tst-auditmod18a.c b/elf/tst-auditmod18a.c
new file mode 100644
index 0000000000..2296382a1c
--- /dev/null
+++ b/elf/tst-auditmod18a.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
+   <https://www.gnu.org/licenses/>.  */
+
+unsigned int
+la_version (unsigned int version)
+{
+  return version;
+}
diff --git a/elf/tst-auditmod18b.c b/elf/tst-auditmod18b.c
new file mode 100644
index 0000000000..52bb88c7d7
--- /dev/null
+++ b/elf/tst-auditmod18b.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
+   <https://www.gnu.org/licenses/>.  */
+
+#include <link.h>
+#include <string.h>
+#include <stdio.h>
+
+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 4af16cb596..ebd0f511e2 100644
--- a/include/link.h
+++ b/include/link.h
@@ -355,8 +355,10 @@ struct auditstate
 
 #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


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [glibc/azanella/ld-audit-fixes] elf: Avoid unnecessary slowdown from profiling with audit (BZ#15533)
@ 2021-08-04 20:52 Adhemerval Zanella
  0 siblings, 0 replies; 6+ messages in thread
From: Adhemerval Zanella @ 2021-08-04 20:52 UTC (permalink / raw)
  To: glibc-cvs

https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=6e058b261224463114cc02c252fba99ec4e5dbbd

commit 6e058b261224463114cc02c252fba99ec4e5dbbd
Author: Adhemerval Zanella <adhemerval.zanella@linaro.org>
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 <amonakov@ispras.ru>

Diff:
---
 NEWS                  |   4 ++
 elf/Makefile          |   9 +++
 elf/dl-reloc.c        |  18 ++++-
 elf/dl-runtime.c      | 188 +++++++++++++++++++++++++++++---------------------
 elf/rtld.c            |   8 +--
 elf/tst-audit18a.c    |  39 +++++++++++
 elf/tst-audit18b.c    |  94 +++++++++++++++++++++++++
 elf/tst-audit18bmod.c |  23 ++++++
 elf/tst-audit18mod.c  |  17 +++++
 elf/tst-auditmod18a.c |  23 ++++++
 elf/tst-auditmod18b.c |  46 ++++++++++++
 include/link.h        |   2 +
 12 files changed, 384 insertions(+), 87 deletions(-)

diff --git a/NEWS b/NEWS
index 79c895e382..703a3ba3bf 100644
--- a/NEWS
+++ b/NEWS
@@ -119,6 +119,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 d05f410592..eab664acaf 100644
--- a/elf/Makefile
+++ b/elf/Makefile
@@ -220,6 +220,7 @@ tests += restest1 preloadtest loadfail multiload origtest resolvfail \
 	 tst-dlopenfail-2 \
 	 tst-filterobj tst-filterobj-dlopen tst-auxobj tst-auxobj-dlopen \
 	 tst-audit14 tst-audit15 tst-audit16 tst-audit17 \
+	 tst-audit18a tst-audit18b \
 	 tst-single_threaded tst-single_threaded-pthread \
 	 tst-tls-ie tst-tls-ie-dlmopen argv0test \
 	 tst-glibc-hwcaps tst-glibc-hwcaps-prepend tst-glibc-hwcaps-mask \
@@ -301,6 +302,7 @@ modules-names = testobj1 testobj2 testobj3 testobj4 testobj5 testobj6 \
 		tst-unique1mod1 tst-unique1mod2 \
 		tst-unique2mod1 tst-unique2mod2 \
 		tst-auditmod9a tst-auditmod9b \
+		tst-auditmod18a tst-auditmod18b tst-audit18bmod \
 		$(if $(CXX),tst-unique3lib tst-unique3lib2 tst-unique4lib \
 		  tst-nodelete-uniquemod tst-nodelete-rtldmod \
 		  tst-nodelete-zmod \
@@ -1486,6 +1488,13 @@ $(objpfx)tst-auditmod17.so: $(objpfx)tst-auditmod17.os
 CFLAGS-.os += $(call elide-stack-protector,.os,tst-auditmod17)
 tst-audit17-ENV = LD_AUDIT=$(objpfx)tst-auditmod17.so
 
+$(objpfx)tst-audit18a.out: $(objpfx)tst-auditmod18a.so
+tst-audit18a-ENV = LD_AUDIT=$(objpfx)tst-auditmod18a.so
+
+$(objpfx)tst-audit18b.out: $(objpfx)tst-auditmod18b.so
+$(objpfx)tst-audit18b: $(objpfx)tst-audit18bmod.so
+tst-audit18b-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 e13a672ade..c6bc1e6b4e 100644
--- a/elf/dl-reloc.c
+++ b/elf/dl-reloc.c
@@ -177,11 +177,25 @@ _dl_relocate_object (struct link_map *l, struct r_scope_elem *scope[],
   const char *errstring = NULL;
   int lazy = reloc_mode & RTLD_LAZY;
   int skip_ifunc = reloc_mode & __RTLD_NOIFUNC;
+  bool consider_symbind = false;
 
 #ifdef SHARED
   /* 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
@@ -275,7 +289,7 @@ _dl_relocate_object (struct link_map *l, struct r_scope_elem *scope[],
     ELF_DYNAMIC_RELOCATE (l, 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 9d0d941000..29031099f5 100644
--- a/elf/dl-runtime.c
+++ b/elf/dl-runtime.c
@@ -43,6 +43,84 @@
 # define ARCH_FIXUP_ATTRIBUTE
 #endif
 
+#ifdef SHARED
+static void
+_dl_audit_symbind (struct link_map *l, struct reloc_result *reloc_result,
+		   const ElfW(Sym) *defsym, DL_FIXUP_VALUE_TYPE *value,
+		   lookup_t result)
+{
+  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]));
+
+  if ((l->l_audit_any_plt | result->l_audit_any_plt) == 0)
+    {
+      /* Set all bits since this symbol binding is not interesting.  */
+      reloc_result->enterexit = (1u << DL_NNS) - 1;
+      return;
+    }
+
+  /* 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]);
+
+  unsigned int flags = 0;
+  struct audit_ifaces *afct = GLRO(dl_audit);
+  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);
+}
+#endif
+
 /* This function is called through a special trampoline from the PLT the
    first time each PLT entry is called.  We must perform the relocation
    specified in the PLT of the given shared object, and return the resolved
@@ -138,6 +216,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;
@@ -296,84 +405,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 d733359eaf..374bf86a69 100644
--- a/elf/rtld.c
+++ b/elf/rtld.c
@@ -1013,13 +1013,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-audit18a.c b/elf/tst-audit18a.c
new file mode 100644
index 0000000000..36b781f9be
--- /dev/null
+++ b/elf/tst-audit18a.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
+   <https://www.gnu.org/licenses/>.  */
+
+#include <link.h>
+#include <stdio.h>
+
+/* 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 <support/test-driver.c>
diff --git a/elf/tst-audit18b.c b/elf/tst-audit18b.c
new file mode 100644
index 0000000000..300197ca2e
--- /dev/null
+++ b/elf/tst-audit18b.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
+   <https://www.gnu.org/licenses/>.  */
+
+#include <getopt.h>
+#include <support/capture_subprocess.h>
+#include <support/check.h>
+#include <support/xstdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <stdbool.h>
+
+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 <support/test-driver.c>
diff --git a/elf/tst-audit18bmod.c b/elf/tst-audit18bmod.c
new file mode 100644
index 0000000000..9ffdcd8f3f
--- /dev/null
+++ b/elf/tst-audit18bmod.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
+   <https://www.gnu.org/licenses/>.  */
+
+int
+tst_audit18bmod1_func (void)
+{
+  return 10;
+}
diff --git a/elf/tst-audit18mod.c b/elf/tst-audit18mod.c
new file mode 100644
index 0000000000..b6eb3731ee
--- /dev/null
+++ b/elf/tst-audit18mod.c
@@ -0,0 +1,17 @@
+/* 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
+   <https://www.gnu.org/licenses/>.  */
diff --git a/elf/tst-auditmod18a.c b/elf/tst-auditmod18a.c
new file mode 100644
index 0000000000..2296382a1c
--- /dev/null
+++ b/elf/tst-auditmod18a.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
+   <https://www.gnu.org/licenses/>.  */
+
+unsigned int
+la_version (unsigned int version)
+{
+  return version;
+}
diff --git a/elf/tst-auditmod18b.c b/elf/tst-auditmod18b.c
new file mode 100644
index 0000000000..52bb88c7d7
--- /dev/null
+++ b/elf/tst-auditmod18b.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
+   <https://www.gnu.org/licenses/>.  */
+
+#include <link.h>
+#include <string.h>
+#include <stdio.h>
+
+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 4af16cb596..ebd0f511e2 100644
--- a/include/link.h
+++ b/include/link.h
@@ -355,8 +355,10 @@ struct auditstate
 
 #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


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [glibc/azanella/ld-audit-fixes] elf: Avoid unnecessary slowdown from profiling with audit (BZ#15533)
@ 2021-08-04 17:46 Adhemerval Zanella
  0 siblings, 0 replies; 6+ messages in thread
From: Adhemerval Zanella @ 2021-08-04 17:46 UTC (permalink / raw)
  To: glibc-cvs

https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=fa333e24d92283183d8d5f5294739c6240eb28f6

commit fa333e24d92283183d8d5f5294739c6240eb28f6
Author: Adhemerval Zanella <adhemerval.zanella@linaro.org>
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 <amonakov@ispras.ru>

Diff:
---
 NEWS                  |   4 ++
 elf/Makefile          |   9 +++
 elf/dl-reloc.c        |  18 ++++-
 elf/dl-runtime.c      | 188 +++++++++++++++++++++++++++++---------------------
 elf/rtld.c            |   8 +--
 elf/tst-audit18a.c    |  39 +++++++++++
 elf/tst-audit18b.c    |  94 +++++++++++++++++++++++++
 elf/tst-audit18bmod.c |  23 ++++++
 elf/tst-audit18mod.c  |  17 +++++
 elf/tst-auditmod18a.c |  23 ++++++
 elf/tst-auditmod18b.c |  46 ++++++++++++
 include/link.h        |   2 +
 12 files changed, 384 insertions(+), 87 deletions(-)

diff --git a/NEWS b/NEWS
index 79c895e382..703a3ba3bf 100644
--- a/NEWS
+++ b/NEWS
@@ -119,6 +119,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 d05f410592..eab664acaf 100644
--- a/elf/Makefile
+++ b/elf/Makefile
@@ -220,6 +220,7 @@ tests += restest1 preloadtest loadfail multiload origtest resolvfail \
 	 tst-dlopenfail-2 \
 	 tst-filterobj tst-filterobj-dlopen tst-auxobj tst-auxobj-dlopen \
 	 tst-audit14 tst-audit15 tst-audit16 tst-audit17 \
+	 tst-audit18a tst-audit18b \
 	 tst-single_threaded tst-single_threaded-pthread \
 	 tst-tls-ie tst-tls-ie-dlmopen argv0test \
 	 tst-glibc-hwcaps tst-glibc-hwcaps-prepend tst-glibc-hwcaps-mask \
@@ -301,6 +302,7 @@ modules-names = testobj1 testobj2 testobj3 testobj4 testobj5 testobj6 \
 		tst-unique1mod1 tst-unique1mod2 \
 		tst-unique2mod1 tst-unique2mod2 \
 		tst-auditmod9a tst-auditmod9b \
+		tst-auditmod18a tst-auditmod18b tst-audit18bmod \
 		$(if $(CXX),tst-unique3lib tst-unique3lib2 tst-unique4lib \
 		  tst-nodelete-uniquemod tst-nodelete-rtldmod \
 		  tst-nodelete-zmod \
@@ -1486,6 +1488,13 @@ $(objpfx)tst-auditmod17.so: $(objpfx)tst-auditmod17.os
 CFLAGS-.os += $(call elide-stack-protector,.os,tst-auditmod17)
 tst-audit17-ENV = LD_AUDIT=$(objpfx)tst-auditmod17.so
 
+$(objpfx)tst-audit18a.out: $(objpfx)tst-auditmod18a.so
+tst-audit18a-ENV = LD_AUDIT=$(objpfx)tst-auditmod18a.so
+
+$(objpfx)tst-audit18b.out: $(objpfx)tst-auditmod18b.so
+$(objpfx)tst-audit18b: $(objpfx)tst-audit18bmod.so
+tst-audit18b-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 e13a672ade..c6bc1e6b4e 100644
--- a/elf/dl-reloc.c
+++ b/elf/dl-reloc.c
@@ -177,11 +177,25 @@ _dl_relocate_object (struct link_map *l, struct r_scope_elem *scope[],
   const char *errstring = NULL;
   int lazy = reloc_mode & RTLD_LAZY;
   int skip_ifunc = reloc_mode & __RTLD_NOIFUNC;
+  bool consider_symbind = false;
 
 #ifdef SHARED
   /* 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
@@ -275,7 +289,7 @@ _dl_relocate_object (struct link_map *l, struct r_scope_elem *scope[],
     ELF_DYNAMIC_RELOCATE (l, 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 9d0d941000..29031099f5 100644
--- a/elf/dl-runtime.c
+++ b/elf/dl-runtime.c
@@ -43,6 +43,84 @@
 # define ARCH_FIXUP_ATTRIBUTE
 #endif
 
+#ifdef SHARED
+static void
+_dl_audit_symbind (struct link_map *l, struct reloc_result *reloc_result,
+		   const ElfW(Sym) *defsym, DL_FIXUP_VALUE_TYPE *value,
+		   lookup_t result)
+{
+  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]));
+
+  if ((l->l_audit_any_plt | result->l_audit_any_plt) == 0)
+    {
+      /* Set all bits since this symbol binding is not interesting.  */
+      reloc_result->enterexit = (1u << DL_NNS) - 1;
+      return;
+    }
+
+  /* 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]);
+
+  unsigned int flags = 0;
+  struct audit_ifaces *afct = GLRO(dl_audit);
+  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);
+}
+#endif
+
 /* This function is called through a special trampoline from the PLT the
    first time each PLT entry is called.  We must perform the relocation
    specified in the PLT of the given shared object, and return the resolved
@@ -138,6 +216,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;
@@ -296,84 +405,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 d733359eaf..374bf86a69 100644
--- a/elf/rtld.c
+++ b/elf/rtld.c
@@ -1013,13 +1013,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-audit18a.c b/elf/tst-audit18a.c
new file mode 100644
index 0000000000..36b781f9be
--- /dev/null
+++ b/elf/tst-audit18a.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
+   <https://www.gnu.org/licenses/>.  */
+
+#include <link.h>
+#include <stdio.h>
+
+/* 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 <support/test-driver.c>
diff --git a/elf/tst-audit18b.c b/elf/tst-audit18b.c
new file mode 100644
index 0000000000..300197ca2e
--- /dev/null
+++ b/elf/tst-audit18b.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
+   <https://www.gnu.org/licenses/>.  */
+
+#include <getopt.h>
+#include <support/capture_subprocess.h>
+#include <support/check.h>
+#include <support/xstdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <stdbool.h>
+
+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 <support/test-driver.c>
diff --git a/elf/tst-audit18bmod.c b/elf/tst-audit18bmod.c
new file mode 100644
index 0000000000..9ffdcd8f3f
--- /dev/null
+++ b/elf/tst-audit18bmod.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
+   <https://www.gnu.org/licenses/>.  */
+
+int
+tst_audit18bmod1_func (void)
+{
+  return 10;
+}
diff --git a/elf/tst-audit18mod.c b/elf/tst-audit18mod.c
new file mode 100644
index 0000000000..b6eb3731ee
--- /dev/null
+++ b/elf/tst-audit18mod.c
@@ -0,0 +1,17 @@
+/* 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
+   <https://www.gnu.org/licenses/>.  */
diff --git a/elf/tst-auditmod18a.c b/elf/tst-auditmod18a.c
new file mode 100644
index 0000000000..2296382a1c
--- /dev/null
+++ b/elf/tst-auditmod18a.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
+   <https://www.gnu.org/licenses/>.  */
+
+unsigned int
+la_version (unsigned int version)
+{
+  return version;
+}
diff --git a/elf/tst-auditmod18b.c b/elf/tst-auditmod18b.c
new file mode 100644
index 0000000000..52bb88c7d7
--- /dev/null
+++ b/elf/tst-auditmod18b.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
+   <https://www.gnu.org/licenses/>.  */
+
+#include <link.h>
+#include <string.h>
+#include <stdio.h>
+
+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 4af16cb596..ebd0f511e2 100644
--- a/include/link.h
+++ b/include/link.h
@@ -355,8 +355,10 @@ struct auditstate
 
 #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


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2021-11-16 13:58 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-30 19:24 [glibc/azanella/ld-audit-fixes] elf: Avoid unnecessary slowdown from profiling with audit (BZ#15533) Adhemerval Zanella
2021-08-04 17:46 Adhemerval Zanella
2021-08-04 20:52 Adhemerval Zanella
2021-09-10 18:25 Adhemerval Zanella
2021-11-09 18:18 Adhemerval Zanella
2021-11-16 13:58 Adhemerval Zanella

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).