[v6,17/20] elf: Add la_activity during application exit

Message ID 20211115183734.531155-18-adhemerval.zanella@linaro.org
State Superseded
Headers
Series Multiple rtld-audit fixes |

Checks

Context Check Description
dj/TryBot-apply_patch success Patch applied to master at the time it was sent

Commit Message

Adhemerval Zanella Nov. 15, 2021, 6:37 p.m. UTC
  la_activity() is not called during application exit, even though
la_objclose is.

Checked on x86_64-linux-gnu, i686-linux-gnu, and aarch64-linux-gnu.
---
 elf/Makefile         |   7 ++
 elf/dl-fini.c        |   9 +++
 elf/tst-audit23.c    | 173 +++++++++++++++++++++++++++++++++++++++++++
 elf/tst-audit23mod.c |  23 ++++++
 elf/tst-auditmod23.c |  68 +++++++++++++++++
 5 files changed, 280 insertions(+)
 create mode 100644 elf/tst-audit23.c
 create mode 100644 elf/tst-audit23mod.c
 create mode 100644 elf/tst-auditmod23.c
  

Comments

Florian Weimer Dec. 20, 2021, 1:34 p.m. UTC | #1
* Adhemerval Zanella:

> diff --git a/elf/dl-fini.c b/elf/dl-fini.c
> index 1720cfaeb8..7c5f2f0e40 100644
> --- a/elf/dl-fini.c
> +++ b/elf/dl-fini.c
> @@ -64,6 +64,10 @@ _dl_fini (void)
>  	__rtld_lock_unlock_recursive (GL(dl_load_lock));
>        else
>  	{
> +#ifdef SHARED
> +	  /* Auditing checkpoint: we will start deleting objects.  */
> +	  _dl_audit_activity_nsid (ns, LA_ACT_DELETE);
> +#endif
>  	  /* Now we can allocate an array to hold all the pointers and
>  	     copy the pointers in.  */
>  	  struct link_map *maps[nloaded];
> @@ -153,6 +157,11 @@ _dl_fini (void)
>  	      /* Correct the previous increment.  */
>  	      --l->l_direct_opencount;
>  	    }
> +
> +#ifdef SHARED
> +	  /* Auditing checkpoint: we will start deleting objects.  */
> +	  _dl_audit_activity_nsid (ns, LA_ACT_CONSISTENT);
> +#endif
>  	}
>      }

This looks okay in principle.  I don't know if we want to merge my
_dl_fine rework first, though, in which case this change has to be
redone from scratch against the new version.

Thanks,
Florian
  
Adhemerval Zanella Dec. 20, 2021, 7:46 p.m. UTC | #2
On 20/12/2021 10:34, Florian Weimer wrote:
> * Adhemerval Zanella:
> 
>> diff --git a/elf/dl-fini.c b/elf/dl-fini.c
>> index 1720cfaeb8..7c5f2f0e40 100644
>> --- a/elf/dl-fini.c
>> +++ b/elf/dl-fini.c
>> @@ -64,6 +64,10 @@ _dl_fini (void)
>>  	__rtld_lock_unlock_recursive (GL(dl_load_lock));
>>        else
>>  	{
>> +#ifdef SHARED
>> +	  /* Auditing checkpoint: we will start deleting objects.  */
>> +	  _dl_audit_activity_nsid (ns, LA_ACT_DELETE);
>> +#endif
>>  	  /* Now we can allocate an array to hold all the pointers and
>>  	     copy the pointers in.  */
>>  	  struct link_map *maps[nloaded];
>> @@ -153,6 +157,11 @@ _dl_fini (void)
>>  	      /* Correct the previous increment.  */
>>  	      --l->l_direct_opencount;
>>  	    }
>> +
>> +#ifdef SHARED
>> +	  /* Auditing checkpoint: we will start deleting objects.  */
>> +	  _dl_audit_activity_nsid (ns, LA_ACT_CONSISTENT);
>> +#endif
>>  	}
>>      }
> 
> This looks okay in principle.  I don't know if we want to merge my
> _dl_fine rework first, though, in which case this change has to be
> redone from scratch against the new version.

I will send as-is based on current master, if your rework got it first
I will rebase it.
  

Patch

diff --git a/elf/Makefile b/elf/Makefile
index 35e45d969e..ee4daae0fc 100644
--- a/elf/Makefile
+++ b/elf/Makefile
@@ -236,6 +236,7 @@  tests += restest1 preloadtest loadfail multiload origtest resolvfail \
 	 tst-audit20 \
 	 tst-audit21 \
 	 tst-audit22 \
+	 tst-audit23 \
 #	 reldep9
 tests-internal += loadtest unload unload2 circleload1 \
 	 neededtest neededtest2 neededtest3 neededtest4 \
@@ -385,6 +386,8 @@  modules-names = testobj1 testobj2 testobj3 testobj4 testobj5 testobj6 \
 		tst-auditmod21a \
 		tst-auditmod21b \
 		tst-auditmod22 \
+		tst-auditmod23 \
+		tst-audit23mod \
 
 # Most modules build with _ISOMAC defined, but those filtered out
 # depend on internal headers.
@@ -1588,6 +1591,10 @@  tst-audit21-ENV = LD_AUDIT=$(objpfx)tst-auditmod21a.so
 $(objpfx)tst-audit22.out: $(objpfx)tst-auditmod22.so
 tst-audit22-ARGS = -- $(host-test-program-cmd)
 
+$(objpfx)tst-audit23.out: $(objpfx)tst-auditmod23.so \
+			  $(objpfx)tst-audit23mod.so
+tst-audit23-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-fini.c b/elf/dl-fini.c
index 1720cfaeb8..7c5f2f0e40 100644
--- a/elf/dl-fini.c
+++ b/elf/dl-fini.c
@@ -64,6 +64,10 @@  _dl_fini (void)
 	__rtld_lock_unlock_recursive (GL(dl_load_lock));
       else
 	{
+#ifdef SHARED
+	  /* Auditing checkpoint: we will start deleting objects.  */
+	  _dl_audit_activity_nsid (ns, LA_ACT_DELETE);
+#endif
 	  /* Now we can allocate an array to hold all the pointers and
 	     copy the pointers in.  */
 	  struct link_map *maps[nloaded];
@@ -153,6 +157,11 @@  _dl_fini (void)
 	      /* Correct the previous increment.  */
 	      --l->l_direct_opencount;
 	    }
+
+#ifdef SHARED
+	  /* Auditing checkpoint: we will start deleting objects.  */
+	  _dl_audit_activity_nsid (ns, LA_ACT_CONSISTENT);
+#endif
 	}
     }
 
diff --git a/elf/tst-audit23.c b/elf/tst-audit23.c
new file mode 100644
index 0000000000..c0070b3b00
--- /dev/null
+++ b/elf/tst-audit23.c
@@ -0,0 +1,173 @@ 
+/* Check DT_AUDIT la_objopen and la_objclose for all objects.
+   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 <errno.h>
+#include <getopt.h>
+#include <limits.h>
+#include <inttypes.h>
+#include <gnu/lib-names.h>
+#include <string.h>
+#include <stdlib.h>
+#include <support/capture_subprocess.h>
+#include <support/check.h>
+#include <support/xstdio.h>
+#include <support/xdlfcn.h>
+#include <support/support.h>
+
+static int restart;
+#define CMDLINE_OPTIONS \
+  { "restart", no_argument, &restart, 1 },
+
+static int
+handle_restart (void)
+{
+  xdlopen ("tst-audit23mod.so", RTLD_NOW);
+  xdlmopen (LM_ID_NEWLM, LIBC_SO, RTLD_NOW);
+
+  return 0;
+}
+
+static inline bool
+startswith (const char *str, const char *pre)
+{
+  size_t lenpre = strlen (pre);
+  size_t lenstr = strlen (str);
+  return lenstr < lenpre ? false : memcmp (pre, str, lenpre) == 0;
+}
+
+static inline bool
+is_name_vdso (const char *str)
+{
+  return startswith (str, "linux-gate")
+	 || startswith (str, "linux-vdso");
+}
+
+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-auditmod23.so", 0);
+  struct support_capture_subprocess result
+    = support_capture_subprogram (spargv[0], spargv);
+  support_capture_subprocess_check (&result, "tst-audit22", 0, sc_allow_stderr);
+
+
+  /* We expect la_objopen/la_objclose for the objects:
+     1. executable
+     2. loader
+     3. libc.so
+     4. tst-audit23mod.so
+     5. libc.so (LM_ID_NEWLM).
+     6. vdso (optional and ignored).  */
+  enum { max_objs = 6 };
+  struct la_obj_t
+  {
+    char *lname;
+    uintptr_t laddr;
+    Lmid_t lmid;
+    bool closed;
+  } objs[max_objs] = { [0 ... max_objs-1] = { .closed = false } };
+  size_t nobjs = 0;
+
+  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))
+    {
+      enum { LA_OBJOPEN, LA_OBJCLOSE} mode;
+      ptrdiff_t offset;
+      if (startswith (buffer, "la_objopen: "))
+	{
+	  offset = strlen ("la_objopen: ");
+	  mode = LA_OBJOPEN;
+	}
+      else if (startswith (buffer, "la_objclose: "))
+	{
+	  offset = strlen ("la_objclose: ");
+	  mode = LA_OBJCLOSE;
+	}
+      else
+	continue;
+
+      char *lname;
+      uintptr_t laddr;
+      Lmid_t lmid;
+      int r = sscanf (buffer + offset, "%ms %"SCNxPTR" %ld", &lname, &laddr,
+		      &lmid);
+      TEST_COMPARE (r, 3);
+
+      if (mode == LA_OBJOPEN)
+	{
+	  /* la_objclose is not triggered by vDSO because glibc does not
+	     unload it.  */
+	  if (is_name_vdso (lname))
+	    continue;
+	  if (nobjs == max_objs)
+	    FAIL_EXIT1 ("non expected la_objopen: %s %"PRIxPTR" %ld",
+			lname, laddr, lmid);
+	  objs[nobjs].lname = lname;
+	  objs[nobjs].laddr = laddr;
+	  objs[nobjs].lmid = lmid;
+	  objs[nobjs].closed = false;
+	  nobjs++;
+	}
+      else if (mode == LA_OBJCLOSE)
+	{
+	  for (size_t i = 0; i < nobjs; i++)
+	    {
+	      if (strcmp (lname, objs[i].lname) == 0 && lmid == objs[i].lmid)
+		{
+		  TEST_COMPARE (objs[i].closed, false);
+		  objs[i].closed = true;
+		  break;
+		}
+	    }
+	}
+    }
+
+  for (size_t i = 0; i < nobjs; i++)
+    {
+      TEST_COMPARE (objs[i].closed, true);
+      free (objs[i].lname);
+    }
+
+  free (buffer);
+  xfclose (out);
+
+  return 0;
+}
+
+#define TEST_FUNCTION_ARGV do_test
+#include <support/test-driver.c>
diff --git a/elf/tst-audit23mod.c b/elf/tst-audit23mod.c
new file mode 100644
index 0000000000..4ca66cf772
--- /dev/null
+++ b/elf/tst-audit23mod.c
@@ -0,0 +1,23 @@ 
+/* Extra modules for tst-audit23
+   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-auditmod23.c b/elf/tst-auditmod23.c
new file mode 100644
index 0000000000..869afd372f
--- /dev/null
+++ b/elf/tst-auditmod23.c
@@ -0,0 +1,68 @@ 
+/* Audit modules loaded by tst-audit23.
+   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 <inttypes.h>
+#include <stdlib.h>
+#include <stdio.h>
+#include <string.h>
+#include <sys/auxv.h>
+
+unsigned int
+la_version (unsigned int version)
+{
+  return LAV_CURRENT;
+}
+
+struct map_desc_t
+{
+  char *lname;
+  uintptr_t laddr;
+  Lmid_t lmid;
+};
+
+unsigned int
+la_objopen (struct link_map *map, Lmid_t lmid, uintptr_t *cookie)
+{
+  fprintf (stderr, "%s: %s %"PRIxPTR" %ld\n", __func__,
+	   map->l_name, map->l_addr, lmid);
+
+  struct map_desc_t *map_desc = malloc (sizeof (struct map_desc_t));
+  if (map_desc == NULL)
+    abort ();
+
+  map_desc->lname = strdup (map->l_name);
+  map_desc->laddr = map->l_addr;
+  map_desc->lmid = lmid;
+
+  *cookie = (uintptr_t) map_desc;
+
+  return 0;
+}
+
+unsigned int
+la_objclose (uintptr_t *cookie)
+{
+  struct map_desc_t *map_desc = (struct map_desc_t *) *cookie;
+  fprintf (stderr, "%s: %s %"PRIxPTR" %ld\n", __func__,
+	   map_desc->lname, map_desc->laddr, map_desc->lmid);
+  free (map_desc->lname);
+  free (map_desc);
+
+  return 0;
+}