[3/3] elf: Reorder audit events in dlcose to match _dl_fini (bug 32066)

Message ID f1649b1c23b465beb8ca6da49d232fbfdf9edbf4.1723239322.git.fweimer@redhat.com
State Superseded
Headers
Series Audit fixes for proxy link maps |

Checks

Context Check Description
redhat-pt-bot/TryBot-32bit success Build for i686
redhat-pt-bot/TryBot-apply_patch success Patch applied to master at the time it was sent
linaro-tcwg-bot/tcwg_glibc_build--master-arm success Build passed
linaro-tcwg-bot/tcwg_glibc_check--master-arm fail Test failed
linaro-tcwg-bot/tcwg_glibc_build--master-aarch64 success Build passed
linaro-tcwg-bot/tcwg_glibc_check--master-aarch64 success Test passed

Commit Message

Florian Weimer Aug. 9, 2024, 9:44 p.m. UTC
  This was discovered after extending elf/tst-audit23 to cover
dlclose of the dlmopen namespace.

Auditors already experience the new order during process
shutdown (_dl_fini), so no LAV_CURRENT bump or backwards
compatibility code seems necessary.
---
 elf/dl-close.c    | 14 ++++++++------
 elf/tst-audit23.c | 40 ++++++++++++++++++++++++++++++----------
 2 files changed, 38 insertions(+), 16 deletions(-)
  

Patch

diff --git a/elf/dl-close.c b/elf/dl-close.c
index 88226245eb..432a35b141 100644
--- a/elf/dl-close.c
+++ b/elf/dl-close.c
@@ -264,6 +264,12 @@  _dl_close_worker (struct link_map *map, bool force)
 	    _dl_catch_exception (NULL, _dl_call_fini, imap);
 
 #ifdef SHARED
+	  /* Auditing checkpoint: we will start deleting objects.
+	     This is supposed to happen before la_objclose (see _dl_fini),
+	     but only once per non-recursive dlclose call.  */
+	  if (!unload_any)
+	    _dl_audit_activity_nsid (nsid, LA_ACT_DELETE);
+
 	  /* Auditing checkpoint: we remove an object.  */
 	  _dl_audit_objclose (imap);
 #endif
@@ -424,12 +430,8 @@  _dl_close_worker (struct link_map *map, bool force)
   if (!unload_any)
     goto out;
 
-#ifdef SHARED
-  /* Auditing checkpoint: we will start deleting objects.  */
-  _dl_audit_activity_nsid (nsid, LA_ACT_DELETE);
-#endif
-
-  /* Notify the debugger we are about to remove some loaded objects.  */
+  /* Notify the debugger we are about to remove some loaded objects.
+     LA_ACT_DELETE has already been signalled above for !unload_any.  */
   struct r_debug *r = _dl_debug_update (nsid);
   r->r_state = RT_DELETE;
   _dl_debug_state ();
diff --git a/elf/tst-audit23.c b/elf/tst-audit23.c
index 09318981e3..ec702b5b30 100644
--- a/elf/tst-audit23.c
+++ b/elf/tst-audit23.c
@@ -30,16 +30,21 @@ 
 #include <support/xstdio.h>
 #include <support/xdlfcn.h>
 #include <support/support.h>
+#include <support/test-driver.h>
 
 static int restart;
+static int do_dlclose;
 #define CMDLINE_OPTIONS \
-  { "restart", no_argument, &restart, 1 },
+  { "restart", no_argument, &restart, 1 }, \
+  { "dlclose", no_argument, &do_dlclose, 1 }, \
 
 static int
 handle_restart (void)
 {
   xdlopen ("tst-audit23mod.so", RTLD_NOW);
-  xdlmopen (LM_ID_NEWLM, LIBC_SO, RTLD_NOW);
+  void *handle = xdlmopen (LM_ID_NEWLM, LIBC_SO, RTLD_NOW);
+  if (do_dlclose)
+    xdlclose (handle);
 
   return 0;
 }
@@ -67,8 +72,8 @@  is_vdso (const char *str)
 	 || startswith (str, "linux-vdso");
 }
 
-static int
-do_test (int argc, char *argv[])
+static void
+do_one_test (int argc, char *argv[], bool pass_dlclose_flag)
 {
   /* We must have either:
      - One or four parameters left if called initially:
@@ -76,16 +81,15 @@  do_test (int argc, char *argv[])
        + "--library-path"      optional
        + the library path      optional
        + the application name  */
-  if (restart)
-    return handle_restart ();
-
-  char *spargv[9];
+  char *spargv[10];
   TEST_VERIFY_EXIT (((argc - 1) + 3) < array_length (spargv));
   int i = 0;
   for (; i < argc - 1; i++)
     spargv[i] = argv[i + 1];
   spargv[i++] = (char *) "--direct";
   spargv[i++] = (char *) "--restart";
+  if (pass_dlclose_flag)
+    spargv[i++] = (char *) "--dlclose";
   spargv[i] = NULL;
 
   setenv ("LD_AUDIT", "tst-auditmod23.so", 0);
@@ -125,6 +129,7 @@  do_test (int argc, char *argv[])
   size_t buffer_length = 0;
   while (xgetline (&buffer, &buffer_length, out))
     {
+      verbose_printf ("info: log line: %s", buffer);
       if (startswith (buffer, "la_activity: "))
 	{
 	  uintptr_t cookie;
@@ -135,8 +140,14 @@  do_test (int argc, char *argv[])
 
 	  /* The cookie identifies the object at the head of the link map,
 	     so we only add a new namespace if it changes from the previous
-	     one.  This works since dlmopen is the last in the test body.  */
-	  if (cookie != last_act_cookie && last_act_cookie != -1)
+	     one.  This works since dlmopen is the last in the test body.
+
+	     Currently, this does not work as expected because there
+	     is no head link map if a namespace is completely deleted.
+	     No LA_ACT_CONSISTENT event is generated in that case.
+	     See the comment in _dl_audit_activity_nsid and bug 32068.  */
+	  if (cookie != last_act_cookie && last_act_cookie != -1
+	      && !pass_dlclose_flag)
 	    TEST_COMPARE (last_act, LA_ACT_CONSISTENT);
 
 	  if (this_act == LA_ACT_ADD && acts[nacts] != cookie)
@@ -254,7 +265,16 @@  do_test (int argc, char *argv[])
 
   free (buffer);
   xfclose (out);
+}
+
+static int
+do_test (int argc, char *argv[])
+{
+  if (restart)
+    return handle_restart ();
 
+  do_one_test (argc, argv, false);
+  do_one_test (argc, argv, true);
   return 0;
 }