public inbox for libc-stable@sourceware.org
 help / color / mirror / Atom feed
From: Carlos O'Donell <carlos@redhat.com>
To: libc-stable@sourceware.org
Cc: Adhemerval Zanella <adhemerval.zanella@linaro.org>,
	Alexander Monakov <amonakov@ispras.ru>,
	Florian Weimer <fweimer@redhat.com>
Subject: [PATCH 16/27] elf: Avoid unnecessary slowdown from profiling with audit (BZ#15533)
Date: Tue, 12 Apr 2022 14:41:54 -0400	[thread overview]
Message-ID: <20220412184205.3343677-17-carlos@redhat.com> (raw)
In-Reply-To: <20220412184205.3343677-1-carlos@redhat.com>

From: Adhemerval Zanella <adhemerval.zanella@linaro.org>

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.

Reviewed-by: Florian Weimer <fweimer@redhat.com>
(cherry picked from commit 063f9ba220f434c7f30dd65c4cff17c0c458a7cf)

Resolved conflicts:
	NEWS
	elf/Makefile
---
 NEWS                  |  5 +++
 elf/Makefile          | 12 ++++++
 elf/dl-reloc.c        | 20 ++++++++-
 elf/dl-runtime.c      | 31 ++++++++++++++
 elf/rtld.c            |  8 +---
 elf/tst-audit19a.c    | 38 +++++++++++++++++
 elf/tst-audit19b.c    | 94 +++++++++++++++++++++++++++++++++++++++++++
 elf/tst-audit19bmod.c | 23 +++++++++++
 elf/tst-auditmod19a.c | 25 ++++++++++++
 elf/tst-auditmod19b.c | 46 +++++++++++++++++++++
 include/link.h        |  2 +
 11 files changed, 295 insertions(+), 9 deletions(-)
 create mode 100644 elf/tst-audit19a.c
 create mode 100644 elf/tst-audit19b.c
 create mode 100644 elf/tst-audit19bmod.c
 create mode 100644 elf/tst-auditmod19a.c
 create mode 100644 elf/tst-auditmod19b.c

diff --git a/NEWS b/NEWS
index aa442a5e4b..16decdb3b3 100644
--- a/NEWS
+++ b/NEWS
@@ -7,6 +7,11 @@ using `glibc' in the "product" field.
 \f
 Version 2.34.1
 
+Major new features:
+
+* The audit libraries will avoid unnecessary slowdown if it is not required
+  PLT tracking (by not implementing the la_pltenter or la_pltexit callbacks).
+
 Security related changes:
 
   CVE-2022-23219: Passing an overlong file name to the clnt_create
diff --git a/elf/Makefile b/elf/Makefile
index ceac1a9db4..ff20a15457 100644
--- a/elf/Makefile
+++ b/elf/Makefile
@@ -354,6 +354,7 @@ tests += \
   tst-audit16 \
   tst-audit17 \
   tst-audit18 \
+  tst-audit19b \
   tst-auditmany \
   tst-auxobj \
   tst-auxobj-dlopen \
@@ -441,6 +442,7 @@ tests-internal += \
   neededtest2 \
   neededtest3 \
   neededtest4 \
+  tst-audit19a \
   tst-create_format1 \
   tst-dl-hwcaps_split \
   tst-dlmopen2 \
@@ -610,6 +612,7 @@ modules-names = \
   tst-audit12mod3 \
   tst-audit13mod1 \
   tst-audit18mod \
+  tst-audit19bmod \
   tst-auditlogmod-1 \
   tst-auditlogmod-2 \
   tst-auditlogmod-3 \
@@ -628,6 +631,8 @@ modules-names = \
   tst-auditmod11 \
   tst-auditmod12 \
   tst-auditmod18 \
+  tst-auditmod19a \
+  tst-auditmod19b \
   tst-auxvalmod \
   tst-big-note-lib \
   tst-deep1mod1 \
@@ -1963,6 +1968,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 3447de7f35..5b69321bda 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 e42f6e8b8d..77a5cccdcb 100644
--- a/elf/dl-runtime.c
+++ b/elf/dl-runtime.c
@@ -124,6 +124,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;
diff --git a/elf/rtld.c b/elf/rtld.c
index fe7f8d27c5..5e5d3001a4 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..035cde9351
--- /dev/null
+++ b/elf/tst-audit19a.c
@@ -0,0 +1,38 @@
+/* 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 <support/xdlfcn.h>
+#include <support/check.h>
+
+static int
+do_test (void)
+{
+  void *h = xdlopen ("tst-auditmod19a.so", RTLD_NOW);
+
+  struct link_map *lmap;
+  TEST_VERIFY_EXIT (dlinfo (h, RTLD_DI_LINKMAP, &lmap) == 0);
+
+  /* The internal array is only allocated if profiling is enabled.  */
+  TEST_VERIFY (lmap->l_reloc_result == NULL);
+
+  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..da015734f2
--- /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..f582040994
--- /dev/null
+++ b/elf/tst-auditmod19a.c
@@ -0,0 +1,25 @@
+/* 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/>.  */
+
+#include <link.h>
+
+unsigned int
+la_version (unsigned int version)
+{
+  return LAV_CURRENT;
+}
diff --git a/elf/tst-auditmod19b.c b/elf/tst-auditmod19b.c
new file mode 100644
index 0000000000..e2248b2a75
--- /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 LAV_CURRENT;
+}
+
+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 c46aced9f7..12a740bed1 100644
--- a/include/link.h
+++ b/include/link.h
@@ -358,8 +358,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
-- 
2.35.1


  parent reply	other threads:[~2022-04-12 18:42 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-04-12 18:41 [PATCH 00/27] Improve LD_AUDIT support in glibc 2.34 Carlos O'Donell
2022-04-12 18:41 ` [PATCH 01/27] powerpc: Delete unneeded ELF_MACHINE_BEFORE_RTLD_RELOC Carlos O'Donell
2022-04-12 18:41 ` [PATCH 02/27] elf: Avoid nested functions in the loader [BZ #27220] Carlos O'Donell
2022-04-12 18:41 ` [PATCH 03/27] elf: Fix elf_get_dynamic_info definition Carlos O'Donell
2022-04-12 18:41 ` [PATCH 04/27] elf: Fix dynamic-link.h usage on rtld.c Carlos O'Donell
2022-04-12 18:41 ` [PATCH 05/27] elf: Fix elf_get_dynamic_info() for bootstrap Carlos O'Donell
2022-04-12 18:41 ` [PATCH 06/27] elf: Move LAV_CURRENT to link_lavcurrent.h Carlos O'Donell
2022-04-12 18:41 ` [PATCH 07/27] elf: Move la_activity (LA_ACT_ADD) after _dl_add_to_namespace_list() (BZ #28062) Carlos O'Donell
2022-04-12 18:41 ` [PATCH 08/27] elf: Add _dl_audit_objopen Carlos O'Donell
2022-04-12 18:41 ` [PATCH 09/27] elf: Add _dl_audit_activity_map and _dl_audit_activity_nsid Carlos O'Donell
2022-04-12 18:41 ` [PATCH 10/27] elf: Add _dl_audit_objsearch Carlos O'Donell
2022-04-12 18:41 ` [PATCH 11/27] elf: Add _dl_audit_objclose Carlos O'Donell
2022-04-12 18:41 ` [PATCH 12/27] elf: Add _dl_audit_symbind_alt and _dl_audit_symbind Carlos O'Donell
2022-04-12 18:41 ` [PATCH 13/27] elf: Add _dl_audit_preinit Carlos O'Donell
2022-04-12 18:41 ` [PATCH 14/27] elf: Add _dl_audit_pltenter Carlos O'Donell
2022-04-12 18:41 ` [PATCH 15/27] elf: Add _dl_audit_pltexit Carlos O'Donell
2022-04-12 18:41 ` Carlos O'Donell [this message]
2022-04-12 18:41 ` [PATCH 17/27] elf: Add audit tests for modules with TLSDESC Carlos O'Donell
2022-04-12 18:41 ` [PATCH 18/27] elf: Issue audit la_objopen for vDSO Carlos O'Donell
2022-04-12 18:41 ` [PATCH 19/27] elf: Do not fail for failed dlmopen on audit modules (BZ #28061) Carlos O'Donell
2022-04-12 18:41 ` [PATCH 20/27] elf: Add la_activity during application exit Carlos O'Donell
2022-04-12 18:41 ` [PATCH 21/27] elf: Fix initial-exec TLS access on audit modules (BZ #28096) Carlos O'Donell
2022-04-12 18:42 ` [PATCH 22/27] elf: Issue la_symbind for bind-now (BZ #23734) Carlos O'Donell
2022-04-12 18:42 ` [PATCH 23/27] elf: Fix runtime linker auditing on aarch64 (BZ #26643) Carlos O'Donell
2022-04-12 18:42 ` [PATCH 24/27] Fix elf/tst-audit25a with default bind now toolchains Carlos O'Donell
2022-04-12 18:42 ` [PATCH 25/27] elf: Replace tst-audit24bmod2.so with tst-audit24bmod2 Carlos O'Donell
2022-04-12 18:42 ` [PATCH 26/27] hppa: Fix bind-now audit (BZ #28857) Carlos O'Donell
2022-04-12 18:42 ` [PATCH 27/27] NEWS: Update fixed bug list for LD_AUDIT backports Carlos O'Donell

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220412184205.3343677-17-carlos@redhat.com \
    --to=carlos@redhat.com \
    --cc=adhemerval.zanella@linaro.org \
    --cc=amonakov@ispras.ru \
    --cc=fweimer@redhat.com \
    --cc=libc-stable@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).