public inbox for libc-alpha@sourceware.org
 help / color / mirror / Atom feed
* [PATCH 0/5] Some rtld-audit fixes
@ 2021-07-07 18:26 Adhemerval Zanella
  2021-07-07 18:26 ` [PATCH 1/5] elf: Fix audit regression Adhemerval Zanella
                   ` (5 more replies)
  0 siblings, 6 replies; 26+ messages in thread
From: Adhemerval Zanella @ 2021-07-07 18:26 UTC (permalink / raw)
  To: libc-alpha; +Cc: John Mellor-Crummey, Ben Woodard

This patchset fixes some rtld-audit issues brought by John
Mellor-Crummey [1] while trying to use it along with the HPCToolkit
tool.  This should cover the issues listed as 'Tier 1' [2], modulo
the aarch64 one, which I plan to address in a different patch set.

The first patch fixes a regression issue introduced by a
__libc_early_init() change.

The second patch is long-standing issue where the lazy resolution
trampolines are used even when the audit modules does not implement
the PLT or symbol binding callback.  The original patch from
Alexander Monakov is incomplete, since it also requires to take
la_symbind{32,64} in consideration.

The third patch add some tests to check if TLSDESC works along with
audit modules.

The forth patch fixes an issue when a dlmopen failure in a audit
module callback trigger an assert.

The final patch fixes another dlmopen failure when audit module
is used along with dlmopen.  This patch was proposed along with 
RTLD_SHARED support, so I added a regression test.

[1] https://sourceware.org/pipermail/libc-alpha/2021-June/127636.html
[2] https://docs.google.com/document/d/1dVaDBdzySecxQqD6hLLzDrEF18M1UtjDna9gL5BWWI0/edit#

Adhemerval Zanella (4):
  elf: Fix audit regression
  elf: Avoid unnecessary slowdown from profiling with audit (BZ#15533)
  elf: Add audit tests for modules with TLSDESC
  elf: Do not fail for failed dlopem on audit modules (BZ #28061)

Vivek Das Mohapatra (1):
  elf: Suppress audit calls when a (new) namespace is empty (BZ #28062)

 elf/Makefile                   |  44 +++++++++++-
 elf/dl-load.c                  |   7 +-
 elf/dl-open.c                  |   4 +-
 elf/dl-reloc.c                 |  13 +++-
 elf/rtld.c                     |   8 +--
 elf/tst-audit-tlsdesc-audit.c  |  23 ++++++
 elf/tst-audit-tlsdesc-dlopen.c |  67 +++++++++++++++++
 elf/tst-audit-tlsdesc.c        |  60 ++++++++++++++++
 elf/tst-audit17.c              |  25 +++++++
 elf/tst-audit18a.c             |  39 ++++++++++
 elf/tst-audit18b.c             |  30 ++++++++
 elf/tst-audit18bmod.c          |  22 ++++++
 elf/tst-audit19.c              |  25 +++++++
 elf/tst-audit20.c              | 128 +++++++++++++++++++++++++++++++++
 elf/tst-audit20mod.c           |  26 +++++++
 elf/tst-auditmod-tlsdesc1.c    |  41 +++++++++++
 elf/tst-auditmod-tlsdesc2.c    |  33 +++++++++
 elf/tst-auditmod17.c           |  24 +++++++
 elf/tst-auditmod18a.c          |  25 +++++++
 elf/tst-auditmod18b.c          |  48 +++++++++++++
 elf/tst-auditmod19.c           |  57 +++++++++++++++
 elf/tst-auditmod20.c           |  73 +++++++++++++++++++
 include/link.h                 |   2 +
 23 files changed, 810 insertions(+), 14 deletions(-)
 create mode 100644 elf/tst-audit-tlsdesc-audit.c
 create mode 100644 elf/tst-audit-tlsdesc-dlopen.c
 create mode 100644 elf/tst-audit-tlsdesc.c
 create mode 100644 elf/tst-audit17.c
 create mode 100644 elf/tst-audit18a.c
 create mode 100644 elf/tst-audit18b.c
 create mode 100644 elf/tst-audit18bmod.c
 create mode 100644 elf/tst-audit19.c
 create mode 100644 elf/tst-audit20.c
 create mode 100644 elf/tst-audit20mod.c
 create mode 100644 elf/tst-auditmod-tlsdesc1.c
 create mode 100644 elf/tst-auditmod-tlsdesc2.c
 create mode 100644 elf/tst-auditmod17.c
 create mode 100644 elf/tst-auditmod18a.c
 create mode 100644 elf/tst-auditmod18b.c
 create mode 100644 elf/tst-auditmod19.c
 create mode 100644 elf/tst-auditmod20.c

-- 
2.30.2


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

* [PATCH 1/5] elf: Fix audit regression
  2021-07-07 18:26 [PATCH 0/5] Some rtld-audit fixes Adhemerval Zanella
@ 2021-07-07 18:26 ` Adhemerval Zanella
  2021-07-07 19:02   ` Florian Weimer
  2021-07-07 19:51   ` Andreas Schwab
  2021-07-07 18:26 ` [PATCH 2/5] elf: Avoid unnecessary slowdown from profiling with audit (BZ#15533) Adhemerval Zanella
                   ` (4 subsequent siblings)
  5 siblings, 2 replies; 26+ messages in thread
From: Adhemerval Zanella @ 2021-07-07 18:26 UTC (permalink / raw)
  To: libc-alpha; +Cc: John Mellor-Crummey, Ben Woodard

Commit 03e187a41d9 added a regression when an audit module does not have
libc as DT_NEEDED (although unusual it is possible).

Checked on x86_64-linux-gnu.
---
 elf/Makefile         | 12 +++++++++++-
 elf/dl-open.c        |  2 +-
 elf/tst-audit17.c    | 25 +++++++++++++++++++++++++
 elf/tst-auditmod17.c | 24 ++++++++++++++++++++++++
 4 files changed, 61 insertions(+), 2 deletions(-)
 create mode 100644 elf/tst-audit17.c
 create mode 100644 elf/tst-auditmod17.c

diff --git a/elf/Makefile b/elf/Makefile
index b1e01d9516..5214196de6 100644
--- a/elf/Makefile
+++ b/elf/Makefile
@@ -219,7 +219,7 @@ 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-audit14 tst-audit15 tst-audit16 tst-audit17 \
 	 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 \
@@ -361,6 +361,7 @@ modules-names += tst-gnu2-tls1mod
 $(objpfx)tst-gnu2-tls1: $(objpfx)tst-gnu2-tls1mod.so
 tst-gnu2-tls1mod.so-no-z-defs = yes
 CFLAGS-tst-gnu2-tls1mod.c += -mtls-dialect=gnu2
+
 endif
 ifeq (yes,$(have-protected-data))
 modules-names += tst-protected1moda tst-protected1modb
@@ -1467,6 +1468,15 @@ $(objpfx)tst-auditlogmod-3.so: $(libsupport)
 $(objpfx)tst-audit16.out: \
   $(objpfx)tst-auditlogmod-1.so $(objpfx)tst-auditlogmod-2.so \
   $(objpfx)tst-auditlogmod-3.so
+$(objpfx)tst-audit17.out: $(objpfx)tst-auditmod17.so
+# The test check if a audit library without libc.so on DT_NEEDED works as
+# intended, so it uses an explicit link rule.
+$(objpfx)tst-auditmod17.so: $(objpfx)tst-auditmod17.os
+	$(CC) -nostdlib -nostartfiles -shared -o $@.new \
+	$(filter-out $(map-file),$^)
+	$(call after-link,$@.new)
+	mv -f $@.new $@
+tst-audit17-ENV = LD_AUDIT=$(objpfx)tst-auditmod17.so
 
 # tst-sonamemove links against an older implementation of the library.
 LDFLAGS-tst-sonamemove-linkmod1.so = \
diff --git a/elf/dl-open.c b/elf/dl-open.c
index a066f39bd0..66ec9d7ed5 100644
--- a/elf/dl-open.c
+++ b/elf/dl-open.c
@@ -771,7 +771,7 @@ dl_open_worker (void *a)
     {
       struct link_map *libc_map = GL(dl_ns)[args->nsid].libc_map;
 #ifdef SHARED
-      bool initial = libc_map->l_ns == LM_ID_BASE;
+      bool initial = libc_map != NULL ? libc_map->l_ns == LM_ID_BASE : false;
 #else
       /* In the static case, there is only one namespace, but it
 	 contains a secondary libc (the primary libc is statically
diff --git a/elf/tst-audit17.c b/elf/tst-audit17.c
new file mode 100644
index 0000000000..92986699d4
--- /dev/null
+++ b/elf/tst-audit17.c
@@ -0,0 +1,25 @@
+/* Check DT_AUDIT with audit not linked against libc.
+   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/>.  */
+
+static int
+do_test (void)
+{
+  return 0;
+}
+
+#include <support/test-driver.c>
diff --git a/elf/tst-auditmod17.c b/elf/tst-auditmod17.c
new file mode 100644
index 0000000000..762de376e2
--- /dev/null
+++ b/elf/tst-auditmod17.c
@@ -0,0 +1,24 @@
+/* Check DT_AUDIT with audit not linked against libc.
+   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;
+}
+
-- 
2.30.2


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

* [PATCH 2/5] elf: Avoid unnecessary slowdown from profiling with audit (BZ#15533)
  2021-07-07 18:26 [PATCH 0/5] Some rtld-audit fixes Adhemerval Zanella
  2021-07-07 18:26 ` [PATCH 1/5] elf: Fix audit regression Adhemerval Zanella
@ 2021-07-07 18:26 ` Adhemerval Zanella
  2021-07-07 19:20   ` Florian Weimer
  2021-07-07 18:26 ` [PATCH 3/5] elf: Add audit tests for modules with TLSDESC Adhemerval Zanella
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 26+ messages in thread
From: Adhemerval Zanella @ 2021-07-07 18:26 UTC (permalink / raw)
  To: libc-alpha; +Cc: John Mellor-Crummey, Ben Woodard, Alexander Monakov

The rtld-audit interfaces introduce 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 (la_plt{enter,exit}
symbols) or symbol bynding (la_symbind{32,64})hooks.  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.

Co-authored-by: Alexander Monakov <amonakov@ispras.dot.ru>
---
 elf/Makefile          | 13 +++++++++++-
 elf/dl-reloc.c        | 13 +++++++++++-
 elf/rtld.c            |  8 +-------
 elf/tst-audit18a.c    | 39 +++++++++++++++++++++++++++++++++++
 elf/tst-audit18b.c    | 30 +++++++++++++++++++++++++++
 elf/tst-audit18bmod.c | 22 ++++++++++++++++++++
 elf/tst-auditmod18a.c | 25 ++++++++++++++++++++++
 elf/tst-auditmod18b.c | 48 +++++++++++++++++++++++++++++++++++++++++++
 include/link.h        |  2 ++
 9 files changed, 191 insertions(+), 9 deletions(-)
 create mode 100644 elf/tst-audit18a.c
 create mode 100644 elf/tst-audit18b.c
 create mode 100644 elf/tst-audit18bmod.c
 create mode 100644 elf/tst-auditmod18a.c
 create mode 100644 elf/tst-auditmod18b.c

diff --git a/elf/Makefile b/elf/Makefile
index 5214196de6..a26f5ce320 100644
--- a/elf/Makefile
+++ b/elf/Makefile
@@ -219,7 +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-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 \
@@ -294,6 +295,7 @@ modules-names = testobj1 testobj2 testobj3 testobj4 testobj5 testobj6 \
 		tst-unique1mod1 tst-unique1mod2 \
 		tst-unique2mod1 tst-unique2mod2 \
 		tst-auditmod9a tst-auditmod9b \
+		tst-auditmod18a tst-audit18bmod tst-auditmod18b \
 		$(if $(CXX),tst-unique3lib tst-unique3lib2 tst-unique4lib \
 		  tst-nodelete-uniquemod tst-nodelete-rtldmod \
 		  tst-nodelete-zmod \
@@ -1478,6 +1480,15 @@ $(objpfx)tst-auditmod17.so: $(objpfx)tst-auditmod17.os
 	mv -f $@.new $@
 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-audit18bmod.so
+$(objpfx)tst-audit18b: $(objpfx)tst-audit18bmod.so
+LDFLAGS-tst-audit18bmod.so = -Wl,-z,now
+LDFLAGS-tst-audit18b = -Wl,-z,now
+tst-audit18b-ENV = LD_AUDIT=$(objpfx)tst-auditmod18b.so
+
 # 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..998cfef099 100644
--- a/elf/dl-reloc.c
+++ b/elf/dl-reloc.c
@@ -181,7 +181,18 @@ _dl_relocate_object (struct link_map *l, struct r_scope_elem *scope[],
 #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->symbind != NULL
+	      || afct->ARCH_LA_PLTENTER != NULL
+	      || afct->ARCH_LA_PLTEXIT != NULL)
+	    consider_profiling = 1;
+	  afct = afct->next;
+	}
+    }
 #elif defined PROF
   /* Never use dynamic linker profiling for gprof profiling code.  */
 # define consider_profiling 0
diff --git a/elf/rtld.c b/elf/rtld.c
index fbbd60b446..bf9eb5a33e 100644
--- a/elf/rtld.c
+++ b/elf/rtld.c
@@ -1014,13 +1014,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..21381fe950
--- /dev/null
+++ b/elf/tst-audit18b.c
@@ -0,0 +1,30 @@
+/* Check if DT_AUDIT a module built with bind-now does trigger la_symbind.
+   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/>.  */
+
+extern int foo (void);
+
+int
+do_test (void)
+{
+  foo ();
+  /* If audit la_symbind() callback is called it will exit the test with
+      EXIT_SUCCESS.  */
+  return 1;
+}
+
+#include <support/test-driver.c>
diff --git a/elf/tst-audit18bmod.c b/elf/tst-audit18bmod.c
new file mode 100644
index 0000000000..91f171b5bc
--- /dev/null
+++ b/elf/tst-audit18bmod.c
@@ -0,0 +1,22 @@
+/* Check if DT_AUDIT a module built with bind-now does trigger la_symbind.
+   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 foo (void)
+{
+  return 0;
+}
diff --git a/elf/tst-auditmod18a.c b/elf/tst-auditmod18a.c
new file mode 100644
index 0000000000..333db0077c
--- /dev/null
+++ b/elf/tst-auditmod18a.c
@@ -0,0 +1,25 @@
+/* Check if DT_ADIT 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/>.  */
+
+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..40ebcd5be2
--- /dev/null
+++ b/elf/tst-auditmod18b.c
@@ -0,0 +1,48 @@
+/* Check if DT_AUDIT a module built with bind-now does trigger la_symbind.
+   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 <stdlib.h>
+#include <string.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_BINDFROM | LA_FLG_BINDTO;
+}
+
+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
+{
+  /* Filter out glibc own symbols libc.so is not built with -z,now.  */
+  if (strcmp (symname, "foo") == 0)
+    exit (EXIT_SUCCESS);
+  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
-- 
2.30.2


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

* [PATCH 3/5] elf: Add audit tests for modules with TLSDESC
  2021-07-07 18:26 [PATCH 0/5] Some rtld-audit fixes Adhemerval Zanella
  2021-07-07 18:26 ` [PATCH 1/5] elf: Fix audit regression Adhemerval Zanella
  2021-07-07 18:26 ` [PATCH 2/5] elf: Avoid unnecessary slowdown from profiling with audit (BZ#15533) Adhemerval Zanella
@ 2021-07-07 18:26 ` Adhemerval Zanella
  2021-07-07 18:26 ` [PATCH 4/5] elf: Do not fail for failed dlmopen on audit modules (BZ #28061) Adhemerval Zanella
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 26+ messages in thread
From: Adhemerval Zanella @ 2021-07-07 18:26 UTC (permalink / raw)
  To: libc-alpha; +Cc: John Mellor-Crummey, Ben Woodard

---
 elf/Makefile                   | 13 +++++++
 elf/tst-audit-tlsdesc-audit.c  | 23 ++++++++++++
 elf/tst-audit-tlsdesc-dlopen.c | 67 ++++++++++++++++++++++++++++++++++
 elf/tst-audit-tlsdesc.c        | 60 ++++++++++++++++++++++++++++++
 elf/tst-auditmod-tlsdesc1.c    | 41 +++++++++++++++++++++
 elf/tst-auditmod-tlsdesc2.c    | 33 +++++++++++++++++
 6 files changed, 237 insertions(+)
 create mode 100644 elf/tst-audit-tlsdesc-audit.c
 create mode 100644 elf/tst-audit-tlsdesc-dlopen.c
 create mode 100644 elf/tst-audit-tlsdesc.c
 create mode 100644 elf/tst-auditmod-tlsdesc1.c
 create mode 100644 elf/tst-auditmod-tlsdesc2.c

diff --git a/elf/Makefile b/elf/Makefile
index a26f5ce320..19f885679a 100644
--- a/elf/Makefile
+++ b/elf/Makefile
@@ -364,6 +364,19 @@ $(objpfx)tst-gnu2-tls1: $(objpfx)tst-gnu2-tls1mod.so
 tst-gnu2-tls1mod.so-no-z-defs = yes
 CFLAGS-tst-gnu2-tls1mod.c += -mtls-dialect=gnu2
 
+tests += tst-audit-tlsdesc tst-audit-tlsdesc-dlopen
+modules-names += tst-auditmod-tlsdesc1 tst-auditmod-tlsdesc2 tst-audit-tlsdesc-audit
+$(objpfx)tst-audit-tlsdesc: $(objpfx)tst-auditmod-tlsdesc1.so \
+			    $(objpfx)tst-auditmod-tlsdesc2.so \
+			    $(shared-thread-library)
+CFLAGS-tst-auditmod-tlsdesc1.c += -mtls-dialect=gnu2
+CFLAGS-tst-auditmod-tlsdesc2.c += -mtls-dialect=gnu2
+$(objpfx)tst-audit-tlsdesc-dlopen: $(shared-thread-library)
+$(objpfx)tst-audit-tlsdesc-dlopen.out: $(objpfx)tst-auditmod-tlsdesc1.so \
+				       $(objpfx)tst-auditmod-tlsdesc2.so
+$(objpfx)tst-auditmod-tlsdesc1.so: $(objpfx)tst-auditmod-tlsdesc2.so
+tst-audit-tlsdesc-ENV = LD_AUDIT=$(objpfx)tst-audit-tlsdesc-audit.so
+tst-audit-tlsdesc-dlopen-ENV = LD_AUDIT=$(objpfx)tst-audit-tlsdesc-audit.so
 endif
 ifeq (yes,$(have-protected-data))
 modules-names += tst-protected1moda tst-protected1modb
diff --git a/elf/tst-audit-tlsdesc-audit.c b/elf/tst-audit-tlsdesc-audit.c
new file mode 100644
index 0000000000..53993830c9
--- /dev/null
+++ b/elf/tst-audit-tlsdesc-audit.c
@@ -0,0 +1,23 @@
+/* DT_AUDIT with modules with TLSDESC.
+   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-audit-tlsdesc-dlopen.c b/elf/tst-audit-tlsdesc-dlopen.c
new file mode 100644
index 0000000000..e4d631fc94
--- /dev/null
+++ b/elf/tst-audit-tlsdesc-dlopen.c
@@ -0,0 +1,67 @@
+/* DT_AUDIT with modules with TLSDESC.
+   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 <support/check.h>
+#include <support/xthread.h>
+#include <support/xdlfcn.h>
+
+static void *
+thr_func (void *mod)
+{
+  int* (*get_global1)(void) = xdlsym (mod, "get_global1");
+  int* (*get_global2)(void) = xdlsym (mod, "get_global2");
+  void (*set_global2)(int) = xdlsym (mod, "set_global2");
+  int* (*get_local1)(void) = xdlsym (mod, "get_local1");
+  int* (*get_local2)(void) = xdlsym (mod, "get_local2");
+
+  int *global1 = get_global1 ();
+  TEST_COMPARE (*global1, 0);
+  ++*global1;
+
+  int *global2 = get_global2 ();
+  TEST_COMPARE (*global2, 0);
+  ++*global2;
+  TEST_COMPARE (*global2, 1);
+
+  set_global2 (10);
+  TEST_COMPARE (*global2, 10);
+
+  int *local1 = get_local1 ();
+  TEST_COMPARE (*local1, 0);
+  ++*local1;
+
+  int *local2 = get_local2 ();
+  TEST_COMPARE (*local2, 0);
+  ++*local2;
+
+  return 0;
+}
+
+static int
+do_test (void)
+{
+  void *mod = xdlopen ("tst-auditmod-tlsdesc1.so", RTLD_LAZY);
+
+  pthread_t thr = xpthread_create (NULL, thr_func, mod);
+  void *r = xpthread_join (thr);
+  TEST_VERIFY (r == NULL);
+
+  return 0;
+}
+
+#include <support/test-driver.c>
diff --git a/elf/tst-audit-tlsdesc.c b/elf/tst-audit-tlsdesc.c
new file mode 100644
index 0000000000..3c8be81c95
--- /dev/null
+++ b/elf/tst-audit-tlsdesc.c
@@ -0,0 +1,60 @@
+/* DT_AUDIT with modules with TLSDESC.
+   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 <support/check.h>
+#include <support/xthread.h>
+
+extern __thread int global1;
+extern __thread int global2;
+void *get_local1 (void);
+void set_global2 (int v);
+void *get_local2 (void);
+
+static void *
+thr_func (void *clousure)
+{
+  TEST_COMPARE (global1, 0);
+  ++global1;
+  TEST_COMPARE (global2, 0);
+  ++global2;
+  TEST_COMPARE (global2, 1);
+
+  set_global2 (10);
+  TEST_COMPARE (global2, 10);
+
+  int *local1 = get_local1 ();
+  TEST_COMPARE (*local1, 0);
+  ++*local1;
+
+  int *local2 = get_local2 ();
+  TEST_COMPARE (*local2, 0);
+  ++*local2;
+
+  return 0;
+}
+
+static int
+do_test (void)
+{
+  pthread_t thr = xpthread_create (NULL, thr_func, NULL);
+  void *r = xpthread_join (thr);
+  TEST_VERIFY (r == NULL);
+  return 0;
+}
+
+#include <support/test-driver.c>
diff --git a/elf/tst-auditmod-tlsdesc1.c b/elf/tst-auditmod-tlsdesc1.c
new file mode 100644
index 0000000000..61c7dd99a2
--- /dev/null
+++ b/elf/tst-auditmod-tlsdesc1.c
@@ -0,0 +1,41 @@
+/* DT_AUDIT with modules with TLSDESC.
+   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/>.  */
+
+__thread int global1;
+
+int *
+get_global1 (void)
+{
+  return &global1;
+}
+
+static __thread int local1;
+
+void *
+get_local1 (void)
+{
+  return &local1;
+}
+
+extern __thread int global2;
+
+void
+set_global2 (int v)
+{
+  global2 = v;
+}
diff --git a/elf/tst-auditmod-tlsdesc2.c b/elf/tst-auditmod-tlsdesc2.c
new file mode 100644
index 0000000000..28aef635f6
--- /dev/null
+++ b/elf/tst-auditmod-tlsdesc2.c
@@ -0,0 +1,33 @@
+/* DT_AUDIT with modules with TLSDESC.
+   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/>.  */
+
+__thread int global2;
+
+int *
+get_global2 (void)
+{
+  return &global2;
+}
+
+static __thread int local2;
+
+void *
+get_local2 (void)
+{
+  return &local2;
+}
-- 
2.30.2


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

* [PATCH 4/5] elf: Do not fail for failed dlmopen on audit modules (BZ #28061)
  2021-07-07 18:26 [PATCH 0/5] Some rtld-audit fixes Adhemerval Zanella
                   ` (2 preceding siblings ...)
  2021-07-07 18:26 ` [PATCH 3/5] elf: Add audit tests for modules with TLSDESC Adhemerval Zanella
@ 2021-07-07 18:26 ` Adhemerval Zanella
  2021-07-07 18:26 ` [PATCH 5/5] elf: Suppress audit calls when a (new) namespace is empty (BZ #28062) Adhemerval Zanella
  2021-07-07 19:02 ` [PATCH 0/5] Some rtld-audit fixes John Mellor-Crummey
  5 siblings, 0 replies; 26+ messages in thread
From: Adhemerval Zanella @ 2021-07-07 18:26 UTC (permalink / raw)
  To: libc-alpha; +Cc: John Mellor-Crummey, Ben Woodard

The dl_main() sets the LM_ID_BASE to RT_ADD just before starting to
add load new shared objects.  The state is set to R_CONSISTENT just
after all objects are loaded.

However if a audit modules tries to dlmopen() an inexistent module,
the _dl_open() will assert that the namespace is in an inconsistent
state.

This is different than dlopen(), since first it will not use
LM_ID_BASE and second _dl_map_object_from_fd() is the sole responsible
to set and reset the r_state value.

So the assert() on _dl_open() can not really see if the state is
consistent since it is _dt_main() that reset is.  This patch removes
the assert.

Checked on x86_64-linux-gnu.
---
 elf/Makefile         |  7 ++++--
 elf/dl-open.c        |  2 --
 elf/tst-audit19.c    | 25 +++++++++++++++++++
 elf/tst-auditmod19.c | 57 ++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 87 insertions(+), 4 deletions(-)
 create mode 100644 elf/tst-audit19.c
 create mode 100644 elf/tst-auditmod19.c

diff --git a/elf/Makefile b/elf/Makefile
index 19f885679a..513b7e68a2 100644
--- a/elf/Makefile
+++ b/elf/Makefile
@@ -220,7 +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-audit18b tst-audit19 \
 	 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 \
@@ -295,7 +295,7 @@ modules-names = testobj1 testobj2 testobj3 testobj4 testobj5 testobj6 \
 		tst-unique1mod1 tst-unique1mod2 \
 		tst-unique2mod1 tst-unique2mod2 \
 		tst-auditmod9a tst-auditmod9b \
-		tst-auditmod18a tst-audit18bmod tst-auditmod18b \
+		tst-auditmod18a tst-audit18bmod tst-auditmod18b tst-auditmod19 \
 		$(if $(CXX),tst-unique3lib tst-unique3lib2 tst-unique4lib \
 		  tst-nodelete-uniquemod tst-nodelete-rtldmod \
 		  tst-nodelete-zmod \
@@ -1502,6 +1502,9 @@ LDFLAGS-tst-audit18bmod.so = -Wl,-z,now
 LDFLAGS-tst-audit18b = -Wl,-z,now
 tst-audit18b-ENV = LD_AUDIT=$(objpfx)tst-auditmod18b.so
 
+$(objpfx)tst-audit19.out: $(objpfx)tst-auditmod19.so
+tst-audit19-ENV = LD_AUDIT=$(objpfx)tst-auditmod19.so
+
 # 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-open.c b/elf/dl-open.c
index 66ec9d7ed5..696de8172f 100644
--- a/elf/dl-open.c
+++ b/elf/dl-open.c
@@ -916,8 +916,6 @@ no more namespaces available for dlmopen()"));
 	     the flag here.  */
 	}
 
-      assert (_dl_debug_initialize (0, args.nsid)->r_state == RT_CONSISTENT);
-
       /* Release the lock.  */
       __rtld_lock_unlock_recursive (GL(dl_load_lock));
 
diff --git a/elf/tst-audit19.c b/elf/tst-audit19.c
new file mode 100644
index 0000000000..6f39ccee86
--- /dev/null
+++ b/elf/tst-audit19.c
@@ -0,0 +1,25 @@
+/* Check dlopen failure on audit modules.
+   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/>.  */
+
+static int
+do_test (void)
+{
+  return 0;
+}
+
+#include <support/test-driver.c>
diff --git a/elf/tst-auditmod19.c b/elf/tst-auditmod19.c
new file mode 100644
index 0000000000..c57e50ee4e
--- /dev/null
+++ b/elf/tst-auditmod19.c
@@ -0,0 +1,57 @@
+/* Check dlopen failure on audit modules.
+   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 <dlfcn.h>
+#include <link.h>
+#include <stdlib.h>
+
+unsigned int
+la_version (unsigned int v)
+{
+  return LAV_CURRENT;
+}
+
+static void
+check (void)
+{
+  {
+    void *mod = dlopen ("nonexistent.so", RTLD_NOW);
+    if (mod != NULL)
+      abort ();
+  }
+
+  {
+    void *mod = dlmopen (LM_ID_BASE, "nonexistent.so", RTLD_NOW);
+    if (mod != NULL)
+      abort ();
+  }
+}
+
+void
+la_activity (uintptr_t *cookie, unsigned int flag)
+{
+  if (flag != LA_ACT_CONSISTENT)
+    return;
+  check ();
+}
+
+void
+la_preinit (uintptr_t *cookie)
+{
+  check ();
+}
-- 
2.30.2


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

* [PATCH 5/5] elf: Suppress audit calls when a (new) namespace is empty (BZ #28062)
  2021-07-07 18:26 [PATCH 0/5] Some rtld-audit fixes Adhemerval Zanella
                   ` (3 preceding siblings ...)
  2021-07-07 18:26 ` [PATCH 4/5] elf: Do not fail for failed dlmopen on audit modules (BZ #28061) Adhemerval Zanella
@ 2021-07-07 18:26 ` Adhemerval Zanella
  2021-07-07 19:02 ` [PATCH 0/5] Some rtld-audit fixes John Mellor-Crummey
  5 siblings, 0 replies; 26+ messages in thread
From: Adhemerval Zanella @ 2021-07-07 18:26 UTC (permalink / raw)
  To: libc-alpha; +Cc: John Mellor-Crummey, Ben Woodard, Vivek Das Mohapatra

From: Vivek Das Mohapatra <vivek@collabora.com>

For a new Lmid_t the namespace link_map list are empty, so it requires
to check if before using it.  This can happen for when audit module
is used along with dlmopen.

Checked on x86_64-linux-gnu.

Co-authored-by: Adhemerval Zanella  <adhemerval.zanella@linaro.org>
---
 elf/Makefile         |   7 ++-
 elf/dl-load.c        |   7 ++-
 elf/tst-audit20.c    | 128 +++++++++++++++++++++++++++++++++++++++++++
 elf/tst-audit20mod.c |  26 +++++++++
 elf/tst-auditmod20.c |  73 ++++++++++++++++++++++++
 5 files changed, 238 insertions(+), 3 deletions(-)
 create mode 100644 elf/tst-audit20.c
 create mode 100644 elf/tst-audit20mod.c
 create mode 100644 elf/tst-auditmod20.c

diff --git a/elf/Makefile b/elf/Makefile
index 513b7e68a2..53868909c5 100644
--- a/elf/Makefile
+++ b/elf/Makefile
@@ -220,7 +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-audit19 \
+	 tst-audit18b tst-audit19 tst-audit20 \
 	 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 \
@@ -296,6 +296,7 @@ modules-names = testobj1 testobj2 testobj3 testobj4 testobj5 testobj6 \
 		tst-unique2mod1 tst-unique2mod2 \
 		tst-auditmod9a tst-auditmod9b \
 		tst-auditmod18a tst-audit18bmod tst-auditmod18b tst-auditmod19 \
+		tst-auditmod20 tst-audit20mod \
 		$(if $(CXX),tst-unique3lib tst-unique3lib2 tst-unique4lib \
 		  tst-nodelete-uniquemod tst-nodelete-rtldmod \
 		  tst-nodelete-zmod \
@@ -1505,6 +1506,10 @@ tst-audit18b-ENV = LD_AUDIT=$(objpfx)tst-auditmod18b.so
 $(objpfx)tst-audit19.out: $(objpfx)tst-auditmod19.so
 tst-audit19-ENV = LD_AUDIT=$(objpfx)tst-auditmod19.so
 
+$(objpfx)tst-audit20.out: $(objpfx)tst-auditmod20.so \
+			  $(objpfx)tst-audit20mod.so
+tst-audit20-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-load.c b/elf/dl-load.c
index a08df001af..d2075f3c02 100644
--- a/elf/dl-load.c
+++ b/elf/dl-load.c
@@ -1062,8 +1062,11 @@ _dl_map_object_from_fd (const char *name, const char *origname, int fd,
 	  && __glibc_unlikely (GLRO(dl_naudit) > 0))
 	{
 	  struct link_map *head = GL(dl_ns)[nsid]._ns_loaded;
-	  /* Do not call the functions for any auditing object.  */
-	  if (head->l_auditing == 0)
+	  /* Do not call the functions for any auditing object and also do not
+	     try to call auditing functions if the namespace is currently
+	     empty.  This happens when opening the first DSO in a new
+	     namespace.  */
+	  if (head != NULL && head->l_auditing == 0)
 	    {
 	      struct audit_ifaces *afct = GLRO(dl_audit);
 	      for (unsigned int cnt = 0; cnt < GLRO(dl_naudit); ++cnt)
diff --git a/elf/tst-audit20.c b/elf/tst-audit20.c
new file mode 100644
index 0000000000..5c5d42b723
--- /dev/null
+++ b/elf/tst-audit20.c
@@ -0,0 +1,128 @@
+/* Check DT_AUDIT with dlmopen.
+   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 <array_length.h>
+#include <getopt.h>
+#include <string.h>
+#include <stdlib.h>
+#include <unistd.h>
+#include <gnu/lib-names.h>
+#include <support/capture_subprocess.h>
+#include <support/check.h>
+#include <support/xdlfcn.h>
+#include <support/xstdio.h>
+#include <support/support.h>
+
+static int restart;
+#define CMDLINE_OPTIONS \
+  { "restart", no_argument, &restart, 1 },
+
+static int
+handle_restart (void)
+{
+  {
+    void *h = xdlmopen (LM_ID_NEWLM, LIBC_SO, RTLD_NOW);
+
+    pid_t (*s)(void) = xdlsym (h, "getpid");
+    TEST_COMPARE (s (), getpid ());
+
+    xdlclose (h);
+  }
+
+  {
+    void *h = xdlmopen (LM_ID_NEWLM, "tst-audit20mod.so", RTLD_NOW);
+
+    int (*foo)(void) = xdlsym (h, "foo");
+    TEST_COMPARE (foo (), 10);
+
+    xdlclose (h);
+  }
+
+  return 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-auditmod20.so", 0);
+  struct support_capture_subprocess result
+    = support_capture_subprogram (spargv[0], spargv);
+  support_capture_subprocess_check (&result, "tst-audit20", 0, sc_allow_stderr);
+
+  struct
+  {
+    const char *name;
+    bool found;
+  } audit_iface[] =
+  {
+    { "la_version", false },
+    { "la_objsearch", false },
+    { "la_activity", false },
+    { "la_objopen", false },
+    { "la_objclose", false },
+    { "la_preinit", false },
+#if __WORDSIZE == 32
+    { "la_symbind32", false },
+#elif __WORDSIZE == 64
+    { "la_symbind64", false },
+#endif
+  };
+
+  /* Some hooks are called more than once but it only checkis if they are
+     called at least once.  */
+  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)) {
+    for (int i = 0; i < array_length (audit_iface); i++)
+      if (strncmp (buffer, audit_iface[i].name,
+		   strlen (audit_iface[i].name)) == 0)
+	audit_iface[i].found = true;
+  }
+  free (buffer);
+  xfclose (out);
+
+  for (int i = 0; i < array_length (audit_iface); i++)
+    TEST_COMPARE (audit_iface[i].found, true);
+
+  support_capture_subprocess_free (&result);
+
+  return 0;
+}
+
+#define TEST_FUNCTION_ARGV do_test
+#include <support/test-driver.c>
diff --git a/elf/tst-audit20mod.c b/elf/tst-audit20mod.c
new file mode 100644
index 0000000000..f229c4139b
--- /dev/null
+++ b/elf/tst-audit20mod.c
@@ -0,0 +1,26 @@
+/* Check DT_AUDIT with dlmopen.
+   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 <fcntl.h>
+#include <gnu/lib-names.h>
+
+int
+foo (void)
+{
+  return 10;
+}
diff --git a/elf/tst-auditmod20.c b/elf/tst-auditmod20.c
new file mode 100644
index 0000000000..182992e9fd
--- /dev/null
+++ b/elf/tst-auditmod20.c
@@ -0,0 +1,73 @@
+/* Check DT_AUDIT with dlmopen.
+   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 <stdio.h>
+#include <link.h>
+
+unsigned int
+la_version (unsigned int version)
+{
+  fprintf (stderr, "%s\n", __func__);
+  return LAV_CURRENT;
+}
+
+char *
+la_objsearch (const char *name, uintptr_t *cookie, unsigned int flag)
+{
+  fprintf (stderr, "%s\n", __func__);
+  return (char *) name;
+}
+
+void
+la_activity (uintptr_t *cookie, unsigned int flag)
+{
+  fprintf (stderr, "%s\n", __func__);
+}
+
+unsigned int
+la_objopen (struct link_map *map, Lmid_t lmid, uintptr_t *cookie)
+{
+  fprintf (stderr, "%s\n", __func__);
+  return LA_FLG_BINDTO | LA_FLG_BINDFROM;
+}
+
+unsigned int
+la_objclose (uintptr_t *cookie)
+{
+  fprintf (stderr, "%s\n", __func__);
+  return 0;
+}
+
+void
+la_preinit (uintptr_t *cookie)
+{
+  fprintf (stderr, "%s\n", __func__);
+}
+
+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, "%s\n", __func__);
+  return sym->st_value;
+}
-- 
2.30.2


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

* Re: [PATCH 1/5] elf: Fix audit regression
  2021-07-07 18:26 ` [PATCH 1/5] elf: Fix audit regression Adhemerval Zanella
@ 2021-07-07 19:02   ` Florian Weimer
  2021-07-07 19:07     ` Adhemerval Zanella
  2021-07-07 19:51   ` Andreas Schwab
  1 sibling, 1 reply; 26+ messages in thread
From: Florian Weimer @ 2021-07-07 19:02 UTC (permalink / raw)
  To: Adhemerval Zanella via Libc-alpha; +Cc: Adhemerval Zanella, John Mellor-Crummey

* Adhemerval Zanella via Libc-alpha:

> diff --git a/elf/Makefile b/elf/Makefile
> index b1e01d9516..5214196de6 100644
> --- a/elf/Makefile
> +++ b/elf/Makefile

> @@ -361,6 +361,7 @@ modules-names += tst-gnu2-tls1mod
>  $(objpfx)tst-gnu2-tls1: $(objpfx)tst-gnu2-tls1mod.so
>  tst-gnu2-tls1mod.so-no-z-defs = yes
>  CFLAGS-tst-gnu2-tls1mod.c += -mtls-dialect=gnu2
> +

Spurious whitespace change.  Or rather, missing CFLAGS for
tst-auditmod17 to disable stack protector.  I expect the test to fail
when building glibc with -fstack-protector-all.

> diff --git a/elf/dl-open.c b/elf/dl-open.c
> index a066f39bd0..66ec9d7ed5 100644
> --- a/elf/dl-open.c
> +++ b/elf/dl-open.c
> @@ -771,7 +771,7 @@ dl_open_worker (void *a)
>      {
>        struct link_map *libc_map = GL(dl_ns)[args->nsid].libc_map;
>  #ifdef SHARED
> -      bool initial = libc_map->l_ns == LM_ID_BASE;
> +      bool initial = libc_map != NULL ? libc_map->l_ns == LM_ID_BASE : false;
>  #else
>        /* In the static case, there is only one namespace, but it
>  	 contains a secondary libc (the primary libc is statically

Looks okay to me.  The previous code was just broken.

Thanks,
Florian


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

* Re: [PATCH 0/5] Some rtld-audit fixes
  2021-07-07 18:26 [PATCH 0/5] Some rtld-audit fixes Adhemerval Zanella
                   ` (4 preceding siblings ...)
  2021-07-07 18:26 ` [PATCH 5/5] elf: Suppress audit calls when a (new) namespace is empty (BZ #28062) Adhemerval Zanella
@ 2021-07-07 19:02 ` John Mellor-Crummey
  2021-07-07 19:09   ` Adhemerval Zanella
  5 siblings, 1 reply; 26+ messages in thread
From: John Mellor-Crummey @ 2021-07-07 19:02 UTC (permalink / raw)
  To: Adhemerval Zanella
  Cc: John Mellor-Crummey, libc-alpha, Ben Woodard, Jonathon Anderson,
	Xiaozhu Meng, Mark W. Krentel, Matt Legendre

Adhemerval,

We are delighted to see your patches for the most troublesome problems (our Tier 1 issues) in glibc 2.34!! We look forward to installing a version of glibc with these patches soon and testing them.

In your email below, you mention that the ARM bugs are not fixed with this patch set and that they will be handled in a different patch set. Are you expecting that patch set to make it into 2.34 or not?
--
John Mellor-Crummey		Professor
Dept of Computer Science	Rice University
email: johnmc@rice.edu		phone: 713-348-5179

> On Jul 7, 2021, at 1:26 PM, Adhemerval Zanella <adhemerval.zanella@linaro.org> wrote:
> 
> This patchset fixes some rtld-audit issues brought by John
> Mellor-Crummey [1] while trying to use it along with the HPCToolkit
> tool.  This should cover the issues listed as 'Tier 1' [2], modulo
> the aarch64 one, which I plan to address in a different patch set.
> 
> The first patch fixes a regression issue introduced by a
> __libc_early_init() change.
> 
> The second patch is long-standing issue where the lazy resolution
> trampolines are used even when the audit modules does not implement
> the PLT or symbol binding callback.  The original patch from
> Alexander Monakov is incomplete, since it also requires to take
> la_symbind{32,64} in consideration.
> 
> The third patch add some tests to check if TLSDESC works along with
> audit modules.
> 
> The forth patch fixes an issue when a dlmopen failure in a audit
> module callback trigger an assert.
> 
> The final patch fixes another dlmopen failure when audit module
> is used along with dlmopen.  This patch was proposed along with 
> RTLD_SHARED support, so I added a regression test.
> 
> [1] https://sourceware.org/pipermail/libc-alpha/2021-June/127636.html
> [2] https://docs.google.com/document/d/1dVaDBdzySecxQqD6hLLzDrEF18M1UtjDna9gL5BWWI0/edit#
> 
> Adhemerval Zanella (4):
>  elf: Fix audit regression
>  elf: Avoid unnecessary slowdown from profiling with audit (BZ#15533)
>  elf: Add audit tests for modules with TLSDESC
>  elf: Do not fail for failed dlopem on audit modules (BZ #28061)
> 
> Vivek Das Mohapatra (1):
>  elf: Suppress audit calls when a (new) namespace is empty (BZ #28062)
> 
> elf/Makefile                   |  44 +++++++++++-
> elf/dl-load.c                  |   7 +-
> elf/dl-open.c                  |   4 +-
> elf/dl-reloc.c                 |  13 +++-
> elf/rtld.c                     |   8 +--
> elf/tst-audit-tlsdesc-audit.c  |  23 ++++++
> elf/tst-audit-tlsdesc-dlopen.c |  67 +++++++++++++++++
> elf/tst-audit-tlsdesc.c        |  60 ++++++++++++++++
> elf/tst-audit17.c              |  25 +++++++
> elf/tst-audit18a.c             |  39 ++++++++++
> elf/tst-audit18b.c             |  30 ++++++++
> elf/tst-audit18bmod.c          |  22 ++++++
> elf/tst-audit19.c              |  25 +++++++
> elf/tst-audit20.c              | 128 +++++++++++++++++++++++++++++++++
> elf/tst-audit20mod.c           |  26 +++++++
> elf/tst-auditmod-tlsdesc1.c    |  41 +++++++++++
> elf/tst-auditmod-tlsdesc2.c    |  33 +++++++++
> elf/tst-auditmod17.c           |  24 +++++++
> elf/tst-auditmod18a.c          |  25 +++++++
> elf/tst-auditmod18b.c          |  48 +++++++++++++
> elf/tst-auditmod19.c           |  57 +++++++++++++++
> elf/tst-auditmod20.c           |  73 +++++++++++++++++++
> include/link.h                 |   2 +
> 23 files changed, 810 insertions(+), 14 deletions(-)
> create mode 100644 elf/tst-audit-tlsdesc-audit.c
> create mode 100644 elf/tst-audit-tlsdesc-dlopen.c
> create mode 100644 elf/tst-audit-tlsdesc.c
> create mode 100644 elf/tst-audit17.c
> create mode 100644 elf/tst-audit18a.c
> create mode 100644 elf/tst-audit18b.c
> create mode 100644 elf/tst-audit18bmod.c
> create mode 100644 elf/tst-audit19.c
> create mode 100644 elf/tst-audit20.c
> create mode 100644 elf/tst-audit20mod.c
> create mode 100644 elf/tst-auditmod-tlsdesc1.c
> create mode 100644 elf/tst-auditmod-tlsdesc2.c
> create mode 100644 elf/tst-auditmod17.c
> create mode 100644 elf/tst-auditmod18a.c
> create mode 100644 elf/tst-auditmod18b.c
> create mode 100644 elf/tst-auditmod19.c
> create mode 100644 elf/tst-auditmod20.c
> 
> -- 
> 2.30.2
> 


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

* Re: [PATCH 1/5] elf: Fix audit regression
  2021-07-07 19:02   ` Florian Weimer
@ 2021-07-07 19:07     ` Adhemerval Zanella
  0 siblings, 0 replies; 26+ messages in thread
From: Adhemerval Zanella @ 2021-07-07 19:07 UTC (permalink / raw)
  To: Florian Weimer, Adhemerval Zanella via Libc-alpha; +Cc: John Mellor-Crummey



On 07/07/2021 16:02, Florian Weimer wrote:
> * Adhemerval Zanella via Libc-alpha:
> 
>> diff --git a/elf/Makefile b/elf/Makefile
>> index b1e01d9516..5214196de6 100644
>> --- a/elf/Makefile
>> +++ b/elf/Makefile
> 
>> @@ -361,6 +361,7 @@ modules-names += tst-gnu2-tls1mod
>>  $(objpfx)tst-gnu2-tls1: $(objpfx)tst-gnu2-tls1mod.so
>>  tst-gnu2-tls1mod.so-no-z-defs = yes
>>  CFLAGS-tst-gnu2-tls1mod.c += -mtls-dialect=gnu2
>> +
> 
> Spurious whitespace change.  Or rather, missing CFLAGS for
> tst-auditmod17 to disable stack protector.  I expect the test to fail
> when building glibc with -fstack-protector-all.

I will check building with -fstack-protector-all and remove the 
spurious whitespace.

> 
>> diff --git a/elf/dl-open.c b/elf/dl-open.c
>> index a066f39bd0..66ec9d7ed5 100644
>> --- a/elf/dl-open.c
>> +++ b/elf/dl-open.c
>> @@ -771,7 +771,7 @@ dl_open_worker (void *a)
>>      {
>>        struct link_map *libc_map = GL(dl_ns)[args->nsid].libc_map;
>>  #ifdef SHARED
>> -      bool initial = libc_map->l_ns == LM_ID_BASE;
>> +      bool initial = libc_map != NULL ? libc_map->l_ns == LM_ID_BASE : false;
>>  #else
>>        /* In the static case, there is only one namespace, but it
>>  	 contains a secondary libc (the primary libc is statically
> 
> Looks okay to me.  The previous code was just broken.
> 
> Thanks,
> Florian
> 

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

* Re: [PATCH 0/5] Some rtld-audit fixes
  2021-07-07 19:02 ` [PATCH 0/5] Some rtld-audit fixes John Mellor-Crummey
@ 2021-07-07 19:09   ` Adhemerval Zanella
  2021-07-07 19:22     ` John Mellor-Crummey
  2021-07-08  0:09     ` John Mellor-Crummey
  0 siblings, 2 replies; 26+ messages in thread
From: Adhemerval Zanella @ 2021-07-07 19:09 UTC (permalink / raw)
  To: John Mellor-Crummey
  Cc: libc-alpha, Ben Woodard, Jonathon Anderson, Xiaozhu Meng,
	Mark W. Krentel, Matt Legendre



On 07/07/2021 16:02, John Mellor-Crummey wrote:
> Adhemerval,
> 
> We are delighted to see your patches for the most troublesome problems (our Tier 1 issues) in glibc 2.34!! We look forward to installing a version of glibc with these patches soon and testing them.
> 
> In your email below, you mention that the ARM bugs are not fixed with this patch set and that they will be handled in a different patch set. Are you expecting that patch set to make it into 2.34 or not?

Hi John,

It is unlikely that they will make into 2.34 because we in the slush freeze
that precedes the release and I haven't posted them previously. And they
most likely required a ldaudit version bump.  I am aiming for next 2.35
release. 


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

* Re: [PATCH 2/5] elf: Avoid unnecessary slowdown from profiling with audit (BZ#15533)
  2021-07-07 18:26 ` [PATCH 2/5] elf: Avoid unnecessary slowdown from profiling with audit (BZ#15533) Adhemerval Zanella
@ 2021-07-07 19:20   ` Florian Weimer
  2021-07-07 20:05     ` Adhemerval Zanella
  0 siblings, 1 reply; 26+ messages in thread
From: Florian Weimer @ 2021-07-07 19:20 UTC (permalink / raw)
  To: Adhemerval Zanella via Libc-alpha; +Cc: Adhemerval Zanella, John Mellor-Crummey

* Adhemerval Zanella via Libc-alpha:

> diff --git a/elf/dl-reloc.c b/elf/dl-reloc.c
> index e13a672ade..998cfef099 100644
> --- a/elf/dl-reloc.c
> +++ b/elf/dl-reloc.c
> @@ -181,7 +181,18 @@ _dl_relocate_object (struct link_map *l, struct r_scope_elem *scope[],
>  #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->symbind != NULL
> +	      || afct->ARCH_LA_PLTENTER != NULL
> +	      || afct->ARCH_LA_PLTEXIT != NULL)
> +	    consider_profiling = 1;
> +	  afct = afct->next;
> +	}
> +    }

Is the afct->symbind check really necessary?  Looking at _dl_fixup, it
should be safe to call symbind with just the standard trampoline.

I think this needs a NEWS entry, describing how to activate this
optimization.

Thanks,
Florian


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

* Re: [PATCH 0/5] Some rtld-audit fixes
  2021-07-07 19:09   ` Adhemerval Zanella
@ 2021-07-07 19:22     ` John Mellor-Crummey
  2021-07-08  6:25       ` Florian Weimer
  2021-07-08  0:09     ` John Mellor-Crummey
  1 sibling, 1 reply; 26+ messages in thread
From: John Mellor-Crummey @ 2021-07-07 19:22 UTC (permalink / raw)
  To: Adhemerval Zanella
  Cc: John Mellor-Crummey, libc-alpha, Ben Woodard, Jonathon Anderson,
	Xiaozhu Meng, Mark W. Krentel, Matt Legendre

Thanks for the clarification that the ARM patches will miss 2.34. That is what I expected.

Now I am a bit worried. Are your expecting the LD_AUDIT patches you posted today to make it into 2.34 or will they be delayed until 2.35 as well?
--
John Mellor-Crummey		Professor
Dept of Computer Science	Rice University
email: johnmc@rice.edu		phone: 713-348-5179

> On Jul 7, 2021, at 2:09 PM, Adhemerval Zanella <adhemerval.zanella@linaro.org> wrote:
> 
> 
> 
> On 07/07/2021 16:02, John Mellor-Crummey wrote:
>> Adhemerval,
>> 
>> We are delighted to see your patches for the most troublesome problems (our Tier 1 issues) in glibc 2.34!! We look forward to installing a version of glibc with these patches soon and testing them.
>> 
>> In your email below, you mention that the ARM bugs are not fixed with this patch set and that they will be handled in a different patch set. Are you expecting that patch set to make it into 2.34 or not?
> 
> Hi John,
> 
> It is unlikely that they will make into 2.34 because we in the slush freeze
> that precedes the release and I haven't posted them previously. And they
> most likely required a ldaudit version bump.  I am aiming for next 2.35
> release. 
> 


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

* Re: [PATCH 1/5] elf: Fix audit regression
  2021-07-07 18:26 ` [PATCH 1/5] elf: Fix audit regression Adhemerval Zanella
  2021-07-07 19:02   ` Florian Weimer
@ 2021-07-07 19:51   ` Andreas Schwab
  2021-07-07 19:57     ` Adhemerval Zanella
  2021-07-07 20:02     ` Florian Weimer
  1 sibling, 2 replies; 26+ messages in thread
From: Andreas Schwab @ 2021-07-07 19:51 UTC (permalink / raw)
  To: Adhemerval Zanella via Libc-alpha; +Cc: Adhemerval Zanella, John Mellor-Crummey

On Jul 07 2021, Adhemerval Zanella via Libc-alpha wrote:

> @@ -771,7 +771,7 @@ dl_open_worker (void *a)
>      {
>        struct link_map *libc_map = GL(dl_ns)[args->nsid].libc_map;
>  #ifdef SHARED
> -      bool initial = libc_map->l_ns == LM_ID_BASE;
> +      bool initial = libc_map != NULL ? libc_map->l_ns == LM_ID_BASE : false;

         bool initial = libc_map != NULL && libc_map->l_ns == LM_ID_BASE;

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

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

* Re: [PATCH 1/5] elf: Fix audit regression
  2021-07-07 19:51   ` Andreas Schwab
@ 2021-07-07 19:57     ` Adhemerval Zanella
  2021-07-07 20:02     ` Florian Weimer
  1 sibling, 0 replies; 26+ messages in thread
From: Adhemerval Zanella @ 2021-07-07 19:57 UTC (permalink / raw)
  To: Andreas Schwab, Adhemerval Zanella via Libc-alpha; +Cc: John Mellor-Crummey



On 07/07/2021 16:51, Andreas Schwab wrote:
> On Jul 07 2021, Adhemerval Zanella via Libc-alpha wrote:
> 
>> @@ -771,7 +771,7 @@ dl_open_worker (void *a)
>>      {
>>        struct link_map *libc_map = GL(dl_ns)[args->nsid].libc_map;
>>  #ifdef SHARED
>> -      bool initial = libc_map->l_ns == LM_ID_BASE;
>> +      bool initial = libc_map != NULL ? libc_map->l_ns == LM_ID_BASE : false;
> 
>          bool initial = libc_map != NULL && libc_map->l_ns == LM_ID_BASE;
> 
> Andreas.
> 

Ack.

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

* Re: [PATCH 1/5] elf: Fix audit regression
  2021-07-07 19:51   ` Andreas Schwab
  2021-07-07 19:57     ` Adhemerval Zanella
@ 2021-07-07 20:02     ` Florian Weimer
  2021-07-07 20:07       ` Florian Weimer
  1 sibling, 1 reply; 26+ messages in thread
From: Florian Weimer @ 2021-07-07 20:02 UTC (permalink / raw)
  To: Andreas Schwab; +Cc: Adhemerval Zanella via Libc-alpha, John Mellor-Crummey

* Andreas Schwab:

> On Jul 07 2021, Adhemerval Zanella via Libc-alpha wrote:
>
>> @@ -771,7 +771,7 @@ dl_open_worker (void *a)
>>      {
>>        struct link_map *libc_map = GL(dl_ns)[args->nsid].libc_map;
>>  #ifdef SHARED
>> -      bool initial = libc_map->l_ns == LM_ID_BASE;
>> +      bool initial = libc_map != NULL ? libc_map->l_ns == LM_ID_BASE : false;
>
>          bool initial = libc_map != NULL && libc_map->l_ns == LM_ID_BASE;

True … but:

This is only used by dlopen/dlmopen, right?  And even if dlmopen is
called from an auditor (to load another libc), it is *never* the initial
libc in the base namespace.

The actual base namespace libc is handled in elf/rtld.c:dl_main:

  /* Relocation is complete.  Perform early libc initialization.  This
     is the initial libc, even if audit modules have been loaded with
     other libcs.  */
  _dl_call_libc_early_init (GL(dl_ns)[LM_ID_BASE].libc_map, true);

And I think the dl_open_worker should mirror that and just do:

  if (!args->libc_already_loaded)
    /* This is never the initial libc because it has been loaded via
       dlmopen.  */
    _dl_call_libc_early_init (libc_map, false);

Thanks,
Florian


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

* Re: [PATCH 2/5] elf: Avoid unnecessary slowdown from profiling with audit (BZ#15533)
  2021-07-07 19:20   ` Florian Weimer
@ 2021-07-07 20:05     ` Adhemerval Zanella
  2021-07-07 20:15       ` Florian Weimer
  0 siblings, 1 reply; 26+ messages in thread
From: Adhemerval Zanella @ 2021-07-07 20:05 UTC (permalink / raw)
  To: Florian Weimer, Adhemerval Zanella via Libc-alpha; +Cc: John Mellor-Crummey



On 07/07/2021 16:20, Florian Weimer wrote:
> * Adhemerval Zanella via Libc-alpha:
> 
>> diff --git a/elf/dl-reloc.c b/elf/dl-reloc.c
>> index e13a672ade..998cfef099 100644
>> --- a/elf/dl-reloc.c
>> +++ b/elf/dl-reloc.c
>> @@ -181,7 +181,18 @@ _dl_relocate_object (struct link_map *l, struct r_scope_elem *scope[],
>>  #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->symbind != NULL
>> +	      || afct->ARCH_LA_PLTENTER != NULL
>> +	      || afct->ARCH_LA_PLTEXIT != NULL)
>> +	    consider_profiling = 1;
>> +	  afct = afct->next;
>> +	}
>> +    }
> 
> Is the afct->symbind check really necessary?  Looking at _dl_fixup, it
> should be safe to call symbind with just the standard trampoline.

Yes, elf/tst-audit18b check specifically for this.  Without it,
returning LA_FLG_BINDFROM | LA_FLG_BINDTO from la_objopen() won't
trigger a la_symbind{32,64}.

> 
> I think this needs a NEWS entry, describing how to activate this
> optimization.

I can add a NEW entry, although it is not really a 'new feature'.
What about:

  * The audit libraries will avoid unnecessary slowdown if it is not
    required either PLT tracking or symbol binding profiling (enabled
    with LA_FLG_BINDFROM or LA_FLG_BINDTO from la_objopen() callback).

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

* Re: [PATCH 1/5] elf: Fix audit regression
  2021-07-07 20:02     ` Florian Weimer
@ 2021-07-07 20:07       ` Florian Weimer
  2021-07-08 12:13         ` Adhemerval Zanella
  0 siblings, 1 reply; 26+ messages in thread
From: Florian Weimer @ 2021-07-07 20:07 UTC (permalink / raw)
  To: Andreas Schwab; +Cc: Adhemerval Zanella via Libc-alpha, John Mellor-Crummey

* Florian Weimer:

> * Andreas Schwab:
>
>> On Jul 07 2021, Adhemerval Zanella via Libc-alpha wrote:
>>
>>> @@ -771,7 +771,7 @@ dl_open_worker (void *a)
>>>      {
>>>        struct link_map *libc_map = GL(dl_ns)[args->nsid].libc_map;
>>>  #ifdef SHARED
>>> -      bool initial = libc_map->l_ns == LM_ID_BASE;
>>> +      bool initial = libc_map != NULL ? libc_map->l_ns == LM_ID_BASE : false;
>>
>>          bool initial = libc_map != NULL && libc_map->l_ns == LM_ID_BASE;
>
> True … but:
>
> This is only used by dlopen/dlmopen, right?  And even if dlmopen is
> called from an auditor (to load another libc), it is *never* the initial
> libc in the base namespace.
>
> The actual base namespace libc is handled in elf/rtld.c:dl_main:
>
>   /* Relocation is complete.  Perform early libc initialization.  This
>      is the initial libc, even if audit modules have been loaded with
>      other libcs.  */
>   _dl_call_libc_early_init (GL(dl_ns)[LM_ID_BASE].libc_map, true);
>
> And I think the dl_open_worker should mirror that and just do:
>
>   if (!args->libc_already_loaded)
>     /* This is never the initial libc because it has been loaded via
>        dlmopen.  */
>     _dl_call_libc_early_init (libc_map, false);

Eh, or rather:

  if (!args->libc_already_loaded)
    {
      struct link_map *libc_map = GL(dl_ns)[args->nsid].libc_map;
      /* This is never the initial libc because it has been loaded via
         dlmopen.  */
      _dl_call_libc_early_init (libc_map, false);
    }

(_dl_call_libc_early_init checks for a null link map.)

Thanks,
Florian


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

* Re: [PATCH 2/5] elf: Avoid unnecessary slowdown from profiling with audit (BZ#15533)
  2021-07-07 20:05     ` Adhemerval Zanella
@ 2021-07-07 20:15       ` Florian Weimer
  2021-07-07 20:53         ` John Mellor-Crummey
  2021-07-19 13:17         ` Adhemerval Zanella
  0 siblings, 2 replies; 26+ messages in thread
From: Florian Weimer @ 2021-07-07 20:15 UTC (permalink / raw)
  To: Adhemerval Zanella; +Cc: Adhemerval Zanella via Libc-alpha, John Mellor-Crummey

* Adhemerval Zanella:

> On 07/07/2021 16:20, Florian Weimer wrote:
>> * Adhemerval Zanella via Libc-alpha:
>> 
>>> diff --git a/elf/dl-reloc.c b/elf/dl-reloc.c
>>> index e13a672ade..998cfef099 100644
>>> --- a/elf/dl-reloc.c
>>> +++ b/elf/dl-reloc.c
>>> @@ -181,7 +181,18 @@ _dl_relocate_object (struct link_map *l, struct r_scope_elem *scope[],
>>>  #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->symbind != NULL
>>> +	      || afct->ARCH_LA_PLTENTER != NULL
>>> +	      || afct->ARCH_LA_PLTEXIT != NULL)
>>> +	    consider_profiling = 1;
>>> +	  afct = afct->next;
>>> +	}
>>> +    }
>> 
>> Is the afct->symbind check really necessary?  Looking at _dl_fixup, it
>> should be safe to call symbind with just the standard trampoline.
>
> Yes, elf/tst-audit18b check specifically for this.  Without it,
> returning LA_FLG_BINDFROM | LA_FLG_BINDTO from la_objopen() won't
> trigger a la_symbind{32,64}.

Hmm, I'm surprised we get a la_symbind call with BIND_NOW at all.  And
for BIND_NOW the choice of trampoline really should not matter anyway.

Anyway, I think we have a feature request that without
la_ltenter/la_pltexit defined, the presence of la_symbind should not
incur the overhead from the profiling trampoline.  The afct->symbind
check defeats that.

Thanks,
Florian


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

* Re: [PATCH 2/5] elf: Avoid unnecessary slowdown from profiling with audit (BZ#15533)
  2021-07-07 20:15       ` Florian Weimer
@ 2021-07-07 20:53         ` John Mellor-Crummey
  2021-07-19 13:17         ` Adhemerval Zanella
  1 sibling, 0 replies; 26+ messages in thread
From: John Mellor-Crummey @ 2021-07-07 20:53 UTC (permalink / raw)
  To: Florian Weimer
  Cc: John Mellor-Crummey, Adhemerval Zanella,
	Adhemerval Zanella via Libc-alpha

Forian is correct that what the HPCToolkit team wants is to use la_symbind without la_ltenter/la_pltexit defined.  We don’t want the overhead from the profiling trampoline just to get the la_symbind call.
--
John Mellor-Crummey		Professor
Dept of Computer Science	Rice University
email: johnmc@rice.edu		phone: 713-348-5179

> On Jul 7, 2021, at 3:15 PM, Florian Weimer <fweimer@redhat.com> wrote:
> 
> * Adhemerval Zanella:
> 
>> On 07/07/2021 16:20, Florian Weimer wrote:
>>> * Adhemerval Zanella via Libc-alpha:
>>> 
>>>> diff --git a/elf/dl-reloc.c b/elf/dl-reloc.c
>>>> index e13a672ade..998cfef099 100644
>>>> --- a/elf/dl-reloc.c
>>>> +++ b/elf/dl-reloc.c
>>>> @@ -181,7 +181,18 @@ _dl_relocate_object (struct link_map *l, struct r_scope_elem *scope[],
>>>> #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->symbind != NULL
>>>> +	      || afct->ARCH_LA_PLTENTER != NULL
>>>> +	      || afct->ARCH_LA_PLTEXIT != NULL)
>>>> +	    consider_profiling = 1;
>>>> +	  afct = afct->next;
>>>> +	}
>>>> +    }
>>> 
>>> Is the afct->symbind check really necessary?  Looking at _dl_fixup, it
>>> should be safe to call symbind with just the standard trampoline.
>> 
>> Yes, elf/tst-audit18b check specifically for this.  Without it,
>> returning LA_FLG_BINDFROM | LA_FLG_BINDTO from la_objopen() won't
>> trigger a la_symbind{32,64}.
> 
> Hmm, I'm surprised we get a la_symbind call with BIND_NOW at all.  And
> for BIND_NOW the choice of trampoline really should not matter anyway.
> 
> Anyway, I think we have a feature request that without
> la_ltenter/la_pltexit defined, the presence of la_symbind should not
> incur the overhead from the profiling trampoline.  The afct->symbind
> check defeats that.
> 
> Thanks,
> Florian
> 


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

* Re: [PATCH 0/5] Some rtld-audit fixes
  2021-07-07 19:09   ` Adhemerval Zanella
  2021-07-07 19:22     ` John Mellor-Crummey
@ 2021-07-08  0:09     ` John Mellor-Crummey
  2021-07-08  0:11       ` John Mellor-Crummey
  1 sibling, 1 reply; 26+ messages in thread
From: John Mellor-Crummey @ 2021-07-08  0:09 UTC (permalink / raw)
  To: Adhemerval Zanella
  Cc: libc-alpha, Ben Woodard, Jonathon Anderson, Xiaozhu Meng,
	Mark W. Krentel, Matt Legendre

actually, i am out july 17-30. schedule it the following week. the delay will not be a peoblem with the dept. 

(sent from my phone)

> On Jul 7, 2021, at 3:09 PM, Adhemerval Zanella <adhemerval.zanella@linaro.org> wrote:
> 
> 
> 
>> On 07/07/2021 16:02, John Mellor-Crummey wrote:
>> Adhemerval,
>> 
>> We are delighted to see your patches for the most troublesome problems (our Tier 1 issues) in glibc 2.34!! We look forward to installing a version of glibc with these patches soon and testing them.
>> 
>> In your email below, you mention that the ARM bugs are not fixed with this patch set and that they will be handled in a different patch set. Are you expecting that patch set to make it into 2.34 or not?
> 
> Hi John,
> 
> It is unlikely that they will make into 2.34 because we in the slush freeze
> that precedes the release and I haven't posted them previously. And they
> most likely required a ldaudit version bump.  I am aiming for next 2.35
> release. 
> 


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

* Re: [PATCH 0/5] Some rtld-audit fixes
  2021-07-08  0:09     ` John Mellor-Crummey
@ 2021-07-08  0:11       ` John Mellor-Crummey
  0 siblings, 0 replies; 26+ messages in thread
From: John Mellor-Crummey @ 2021-07-08  0:11 UTC (permalink / raw)
  To: Adhemerval Zanella
  Cc: libc-alpha, Ben Woodard, Jonathon Anderson, Xiaozhu Meng,
	Mark W. Krentel, Matt Legendre

sorry. my reply went to the wrong recipients.

(sent from my phone)

> On Jul 7, 2021, at 8:09 PM, John Mellor-Crummey <johnmc@rice.edu> wrote:
> 
> actually, i am out july 17-30. schedule it the following week. the delay will not be a peoblem with the dept. 
> 
> (sent from my phone)
> 
>> On Jul 7, 2021, at 3:09 PM, Adhemerval Zanella <adhemerval.zanella@linaro.org> wrote:
>> 
>> 
>> 
>>>> On 07/07/2021 16:02, John Mellor-Crummey wrote:
>>> Adhemerval,
>>> 
>>> We are delighted to see your patches for the most troublesome problems (our Tier 1 issues) in glibc 2.34!! We look forward to installing a version of glibc with these patches soon and testing them.
>>> 
>>> In your email below, you mention that the ARM bugs are not fixed with this patch set and that they will be handled in a different patch set. Are you expecting that patch set to make it into 2.34 or not?
>> 
>> Hi John,
>> 
>> It is unlikely that they will make into 2.34 because we in the slush freeze
>> that precedes the release and I haven't posted them previously. And they
>> most likely required a ldaudit version bump.  I am aiming for next 2.35
>> release. 
>> 


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

* Re: [PATCH 0/5] Some rtld-audit fixes
  2021-07-07 19:22     ` John Mellor-Crummey
@ 2021-07-08  6:25       ` Florian Weimer
  2021-07-08  8:03         ` John Mellor-Crummey
  0 siblings, 1 reply; 26+ messages in thread
From: Florian Weimer @ 2021-07-08  6:25 UTC (permalink / raw)
  To: John Mellor-Crummey via Libc-alpha
  Cc: Adhemerval Zanella, John Mellor-Crummey, Xiaozhu Meng,
	Matt Legendre, Mark W. Krentel

* John Mellor-Crummey via Libc-alpha:

> Now I am a bit worried. Are your expecting the LD_AUDIT patches you
> posted today to make it into 2.34 or will they be delayed until 2.35
> as well?

Let me approach this differently.  Why is the glibc 2.34 release
important to you?

To be honest, I expect whether the fixes land in glibc 2.34 or 2.35 has
little impact on when the changes land in the hands of your users.  So
far, everything looks backportable.  I'm aware of just one distribution
with long-term support that plans to use glibc 2.34 (Red Hat Enterprise
Linux 9), and given that there is a strong desire to have the changes in
Red Hat Enterprise Linux 8, we'd have to backport them to Red Hat
Enterprise Linux 9 if they aren't included in the glibc 2.34 release
proper.  (I'd consider a LAV_CURRENT bump backportable, too.)

Thanks,
Florian


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

* Re: [PATCH 0/5] Some rtld-audit fixes
  2021-07-08  6:25       ` Florian Weimer
@ 2021-07-08  8:03         ` John Mellor-Crummey
  2021-07-08  8:21           ` Florian Weimer
  0 siblings, 1 reply; 26+ messages in thread
From: John Mellor-Crummey @ 2021-07-08  8:03 UTC (permalink / raw)
  To: Florian Weimer
  Cc: John Mellor-Crummey via Libc-alpha, Adhemerval Zanella,
	Xiaozhu Meng, Matt Legendre, Mark W. Krentel

Florian,

I’d like these fixes for a supercomputer being delivered in the fall. I have a discussion next week about getting a glibc that I need on the system. Having a version that I can name that will be finalized imminently will help with that. I don’t know anything about the glibc release schedule. When would you expect 2.35 to be finalized?

(sent from my phone)

> On Jul 8, 2021, at 2:26 AM, Florian Weimer <fweimer@redhat.com> wrote:
> 
> * John Mellor-Crummey via Libc-alpha:
> 
>> Now I am a bit worried. Are your expecting the LD_AUDIT patches you
>> posted today to make it into 2.34 or will they be delayed until 2.35
>> as well?
> 
> Let me approach this differently.  Why is the glibc 2.34 release
> important to you?
> 
> To be honest, I expect whether the fixes land in glibc 2.34 or 2.35 has
> little impact on when the changes land in the hands of your users.  So
> far, everything looks backportable.  I'm aware of just one distribution
> with long-term support that plans to use glibc 2.34 (Red Hat Enterprise
> Linux 9), and given that there is a strong desire to have the changes in
> Red Hat Enterprise Linux 8, we'd have to backport them to Red Hat
> Enterprise Linux 9 if they aren't included in the glibc 2.34 release
> proper.  (I'd consider a LAV_CURRENT bump backportable, too.)
> 
> Thanks,
> Florian
> 


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

* Re: [PATCH 0/5] Some rtld-audit fixes
  2021-07-08  8:03         ` John Mellor-Crummey
@ 2021-07-08  8:21           ` Florian Weimer
  0 siblings, 0 replies; 26+ messages in thread
From: Florian Weimer @ 2021-07-08  8:21 UTC (permalink / raw)
  To: John Mellor-Crummey
  Cc: John Mellor-Crummey via Libc-alpha, Adhemerval Zanella,
	Xiaozhu Meng, Matt Legendre, Mark W. Krentel

* John Mellor-Crummey:

> Florian,
>
> I’d like these fixes for a supercomputer being delivered in the
> fall. I have a discussion next week about getting a glibc that I need
> on the system. Having a version that I can name that will be finalized
> imminently will help with that. I don’t know anything about the glibc
> release schedule. When would you expect 2.35 to be finalized?

It's scheduled for February 2022.

I agree that you need to talk to your software technology partner.  I
doubt they would want to use glibc 2.34 so closely after the upstream
release.  They will have to backport the fixes to their stabilized
environment.

Thanks,
Florian


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

* Re: [PATCH 1/5] elf: Fix audit regression
  2021-07-07 20:07       ` Florian Weimer
@ 2021-07-08 12:13         ` Adhemerval Zanella
  0 siblings, 0 replies; 26+ messages in thread
From: Adhemerval Zanella @ 2021-07-08 12:13 UTC (permalink / raw)
  To: Florian Weimer, Andreas Schwab
  Cc: John Mellor-Crummey, Adhemerval Zanella via Libc-alpha



On 07/07/2021 17:07, Florian Weimer via Libc-alpha wrote:
> * Florian Weimer:
> 
>> * Andreas Schwab:
>>
>>> On Jul 07 2021, Adhemerval Zanella via Libc-alpha wrote:
>>>
>>>> @@ -771,7 +771,7 @@ dl_open_worker (void *a)
>>>>      {
>>>>        struct link_map *libc_map = GL(dl_ns)[args->nsid].libc_map;
>>>>  #ifdef SHARED
>>>> -      bool initial = libc_map->l_ns == LM_ID_BASE;
>>>> +      bool initial = libc_map != NULL ? libc_map->l_ns == LM_ID_BASE : false;
>>>
>>>          bool initial = libc_map != NULL && libc_map->l_ns == LM_ID_BASE;
>>
>> True … but:
>>
>> This is only used by dlopen/dlmopen, right?  And even if dlmopen is
>> called from an auditor (to load another libc), it is *never* the initial
>> libc in the base namespace.
>>
>> The actual base namespace libc is handled in elf/rtld.c:dl_main:
>>
>>   /* Relocation is complete.  Perform early libc initialization.  This
>>      is the initial libc, even if audit modules have been loaded with
>>      other libcs.  */
>>   _dl_call_libc_early_init (GL(dl_ns)[LM_ID_BASE].libc_map, true);
>>
>> And I think the dl_open_worker should mirror that and just do:
>>
>>   if (!args->libc_already_loaded)
>>     /* This is never the initial libc because it has been loaded via
>>        dlmopen.  */
>>     _dl_call_libc_early_init (libc_map, false);
> 
> Eh, or rather:
> 
>   if (!args->libc_already_loaded)
>     {
>       struct link_map *libc_map = GL(dl_ns)[args->nsid].libc_map;
>       /* This is never the initial libc because it has been loaded via
>          dlmopen.  */
>       _dl_call_libc_early_init (libc_map, false);
>     }
> 
> (_dl_call_libc_early_init checks for a null link map.)

I think this make sense and it also simplifes the code a bit. I will
adjust the patch.

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

* Re: [PATCH 2/5] elf: Avoid unnecessary slowdown from profiling with audit (BZ#15533)
  2021-07-07 20:15       ` Florian Weimer
  2021-07-07 20:53         ` John Mellor-Crummey
@ 2021-07-19 13:17         ` Adhemerval Zanella
  1 sibling, 0 replies; 26+ messages in thread
From: Adhemerval Zanella @ 2021-07-19 13:17 UTC (permalink / raw)
  To: Florian Weimer; +Cc: Adhemerval Zanella via Libc-alpha, John Mellor-Crummey



On 07/07/2021 17:15, Florian Weimer wrote:
> * Adhemerval Zanella:
> 
>> On 07/07/2021 16:20, Florian Weimer wrote:
>>> * Adhemerval Zanella via Libc-alpha:
>>>
>>>> diff --git a/elf/dl-reloc.c b/elf/dl-reloc.c
>>>> index e13a672ade..998cfef099 100644
>>>> --- a/elf/dl-reloc.c
>>>> +++ b/elf/dl-reloc.c
>>>> @@ -181,7 +181,18 @@ _dl_relocate_object (struct link_map *l, struct r_scope_elem *scope[],
>>>>  #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->symbind != NULL
>>>> +	      || afct->ARCH_LA_PLTENTER != NULL
>>>> +	      || afct->ARCH_LA_PLTEXIT != NULL)
>>>> +	    consider_profiling = 1;
>>>> +	  afct = afct->next;
>>>> +	}
>>>> +    }
>>>
>>> Is the afct->symbind check really necessary?  Looking at _dl_fixup, it
>>> should be safe to call symbind with just the standard trampoline.
>>
>> Yes, elf/tst-audit18b check specifically for this.  Without it,
>> returning LA_FLG_BINDFROM | LA_FLG_BINDTO from la_objopen() won't
>> trigger a la_symbind{32,64}.
> 
> Hmm, I'm surprised we get a la_symbind call with BIND_NOW at all.  And
> for BIND_NOW the choice of trampoline really should not matter anyway.
> 
> Anyway, I think we have a feature request that without
> la_ltenter/la_pltexit defined, the presence of la_symbind should not
> incur the overhead from the profiling trampoline.  The afct->symbind
> check defeats that.

Indeed, we will need to track the la_symbind for bind-now in a different 
fix then.  I will update the patch.

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

end of thread, other threads:[~2021-07-19 13:17 UTC | newest]

Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-07 18:26 [PATCH 0/5] Some rtld-audit fixes Adhemerval Zanella
2021-07-07 18:26 ` [PATCH 1/5] elf: Fix audit regression Adhemerval Zanella
2021-07-07 19:02   ` Florian Weimer
2021-07-07 19:07     ` Adhemerval Zanella
2021-07-07 19:51   ` Andreas Schwab
2021-07-07 19:57     ` Adhemerval Zanella
2021-07-07 20:02     ` Florian Weimer
2021-07-07 20:07       ` Florian Weimer
2021-07-08 12:13         ` Adhemerval Zanella
2021-07-07 18:26 ` [PATCH 2/5] elf: Avoid unnecessary slowdown from profiling with audit (BZ#15533) Adhemerval Zanella
2021-07-07 19:20   ` Florian Weimer
2021-07-07 20:05     ` Adhemerval Zanella
2021-07-07 20:15       ` Florian Weimer
2021-07-07 20:53         ` John Mellor-Crummey
2021-07-19 13:17         ` Adhemerval Zanella
2021-07-07 18:26 ` [PATCH 3/5] elf: Add audit tests for modules with TLSDESC Adhemerval Zanella
2021-07-07 18:26 ` [PATCH 4/5] elf: Do not fail for failed dlmopen on audit modules (BZ #28061) Adhemerval Zanella
2021-07-07 18:26 ` [PATCH 5/5] elf: Suppress audit calls when a (new) namespace is empty (BZ #28062) Adhemerval Zanella
2021-07-07 19:02 ` [PATCH 0/5] Some rtld-audit fixes John Mellor-Crummey
2021-07-07 19:09   ` Adhemerval Zanella
2021-07-07 19:22     ` John Mellor-Crummey
2021-07-08  6:25       ` Florian Weimer
2021-07-08  8:03         ` John Mellor-Crummey
2021-07-08  8:21           ` Florian Weimer
2021-07-08  0:09     ` John Mellor-Crummey
2021-07-08  0:11       ` John Mellor-Crummey

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).