diff mbox series

[v4,3/5] elf: Signal la_objopen for the proxy link map in dlmopen (bug 31985)

Message ID 7e0550b8f6d2c643d4a3ff05730c36d534261a0c.1732894673.git.fweimer@redhat.com
State Accepted
Delegated to: Adhemerval Zanella Netto
Headers
Series la_objclose fixes (bug 31985, bug 32065, 32066) |

Checks

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

Commit Message

Florian Weimer Nov. 29, 2024, 3:40 p.m. UTC 
  Previously, the ld.so link map was silently added to the namespace.
This change produces an auditing event for it.

---
 elf/dl-load.c     | 61 ++++++++++++++++++++++++++---------------------
 elf/tst-audit23.c |  8 +++++--
 2 files changed, 40 insertions(+), 29 deletions(-)

Comments

Adhemerval Zanella Dec. 3, 2024, 6:34 p.m. UTC | #1 
On 29/11/24 12:40, Florian Weimer wrote:
> Previously, the ld.so link map was silently added to the namespace.
> This change produces an auditing event for it.

Is https://sourceware.org/bugzilla/show_bug.cgi?id=31985#c4 related to this
issue?  The rest looks ok.

Reviewed-by: Adhemerval Zanella  <adhemerval.zanella@linaro.org>

> 
> ---
>  elf/dl-load.c     | 61 ++++++++++++++++++++++++++---------------------
>  elf/tst-audit23.c |  8 +++++--
>  2 files changed, 40 insertions(+), 29 deletions(-)
> 
> diff --git a/elf/dl-load.c b/elf/dl-load.c
> index e986d7faab..d5725fc714 100644
> --- a/elf/dl-load.c
> +++ b/elf/dl-load.c
> @@ -902,6 +902,37 @@ _dl_process_pt_gnu_property (struct link_map *l, int fd, const ElfW(Phdr) *ph)
>      }
>  }
>  
> +static void
> +_dl_notify_new_object (int mode, Lmid_t nsid, struct link_map *l)
> +{
> +  /* Signal that we are going to add new objects.  */
> +  struct r_debug *r = _dl_debug_update (nsid);
> +  if (r->r_state == RT_CONSISTENT)
> +    {
> +#ifdef SHARED
> +      /* Auditing checkpoint: we are going to add new objects.  Since this
> +         is called after _dl_add_to_namespace_list the namespace is guaranteed
> +	 to not be empty.  */
> +      if ((mode & __RTLD_AUDIT) == 0)
> +	_dl_audit_activity_nsid (nsid, LA_ACT_ADD);
> +#endif
> +
> +      /* Notify the debugger we have added some objects.  We need to
> +	 call _dl_debug_initialize in a static program in case dynamic
> +	 linking has not been used before.  */
> +      r->r_state = RT_ADD;
> +      _dl_debug_state ();
> +      LIBC_PROBE (map_start, 2, nsid, r);
> +    }
> +  else
> +    assert (r->r_state == RT_ADD);
> +
> +#ifdef SHARED
> +  /* Auditing checkpoint: we have a new object.  */
> +  if (!GL(dl_ns)[l->l_ns]._ns_loaded->l_auditing)
> +    _dl_audit_objopen (l, nsid);
> +#endif
> +}
>  
>  /* Map in the shared object NAME, actually located in REALNAME, and already
>     opened on FD.  */
> @@ -1002,6 +1033,8 @@ _dl_map_object_from_fd (const char *name, const char *origname, int fd,
>        /* Add the map for the mirrored object to the object list.  */
>        _dl_add_to_namespace_list (l, nsid);
>  
> +      _dl_notify_new_object (mode, nsid, l);
> +
>        return l;
>      }
>  #endif
> @@ -1426,33 +1459,7 @@ cannot enable executable stack as shared object requires");
>    if (mode & __RTLD_SPROF)
>      return l;
>  
> -  /* Signal that we are going to add new objects.  */
> -  struct r_debug *r = _dl_debug_update (nsid);
> -  if (r->r_state == RT_CONSISTENT)
> -    {
> -#ifdef SHARED
> -      /* Auditing checkpoint: we are going to add new objects.  Since this
> -         is called after _dl_add_to_namespace_list the namespace is guaranteed
> -	 to not be empty.  */
> -      if ((mode & __RTLD_AUDIT) == 0)
> -	_dl_audit_activity_nsid (nsid, LA_ACT_ADD);
> -#endif
> -
> -      /* Notify the debugger we have added some objects.  We need to
> -	 call _dl_debug_initialize in a static program in case dynamic
> -	 linking has not been used before.  */
> -      r->r_state = RT_ADD;
> -      _dl_debug_state ();
> -      LIBC_PROBE (map_start, 2, nsid, r);
> -    }
> -  else
> -    assert (r->r_state == RT_ADD);
> -
> -#ifdef SHARED
> -  /* Auditing checkpoint: we have a new object.  */
> -  if (!GL(dl_ns)[l->l_ns]._ns_loaded->l_auditing)
> -    _dl_audit_objopen (l, nsid);
> -#endif
> +  _dl_notify_new_object (mode, nsid, l);
>  
>    return l;
>  }
> diff --git a/elf/tst-audit23.c b/elf/tst-audit23.c
> index 895dab08ea..c8c6553b8f 100644
> --- a/elf/tst-audit23.c
> +++ b/elf/tst-audit23.c
> @@ -17,6 +17,7 @@
>     <https://www.gnu.org/licenses/>.  */
>  
>  #include <array_length.h>
> +#include <endswith.h>
>  #include <errno.h>
>  #include <getopt.h>
>  #include <link.h>
> @@ -106,8 +107,9 @@ do_test (int argc, char *argv[])
>       4. libgcc_s.so (one some architectures, for libsupport)
>       5. tst-audit23mod.so
>       6. libc.so (LM_ID_NEWLM).
> +     7. loader (proxy link map in new namespace)
>          vdso (optional and ignored).  */
> -  enum { max_objs = 6 };
> +  enum { max_objs = 7 };
>    struct la_obj_t
>    {
>      char *lname;
> @@ -236,7 +238,9 @@ do_test (int argc, char *argv[])
>  
>    for (size_t i = 0; i < nobjs; i++)
>      {
> -      TEST_COMPARE (objs[i].closed, true);
> +      /* This subtest currently does not pass because of bug 32065.  */
> +      if (! (endswith (objs[i].lname, LD_SO) && objs[i].lmid != LM_ID_BASE))
> +	TEST_COMPARE (objs[i].closed, true);
>        free (objs[i].lname);
>      }
>
diff mbox series

Patch

diff --git a/elf/dl-load.c b/elf/dl-load.c
index e986d7faab..d5725fc714 100644
--- a/elf/dl-load.c
+++ b/elf/dl-load.c
@@ -902,6 +902,37 @@  _dl_process_pt_gnu_property (struct link_map *l, int fd, const ElfW(Phdr) *ph)
     }
 }
 
+static void
+_dl_notify_new_object (int mode, Lmid_t nsid, struct link_map *l)
+{
+  /* Signal that we are going to add new objects.  */
+  struct r_debug *r = _dl_debug_update (nsid);
+  if (r->r_state == RT_CONSISTENT)
+    {
+#ifdef SHARED
+      /* Auditing checkpoint: we are going to add new objects.  Since this
+         is called after _dl_add_to_namespace_list the namespace is guaranteed
+	 to not be empty.  */
+      if ((mode & __RTLD_AUDIT) == 0)
+	_dl_audit_activity_nsid (nsid, LA_ACT_ADD);
+#endif
+
+      /* Notify the debugger we have added some objects.  We need to
+	 call _dl_debug_initialize in a static program in case dynamic
+	 linking has not been used before.  */
+      r->r_state = RT_ADD;
+      _dl_debug_state ();
+      LIBC_PROBE (map_start, 2, nsid, r);
+    }
+  else
+    assert (r->r_state == RT_ADD);
+
+#ifdef SHARED
+  /* Auditing checkpoint: we have a new object.  */
+  if (!GL(dl_ns)[l->l_ns]._ns_loaded->l_auditing)
+    _dl_audit_objopen (l, nsid);
+#endif
+}
 
 /* Map in the shared object NAME, actually located in REALNAME, and already
    opened on FD.  */
@@ -1002,6 +1033,8 @@  _dl_map_object_from_fd (const char *name, const char *origname, int fd,
       /* Add the map for the mirrored object to the object list.  */
       _dl_add_to_namespace_list (l, nsid);
 
+      _dl_notify_new_object (mode, nsid, l);
+
       return l;
     }
 #endif
@@ -1426,33 +1459,7 @@  cannot enable executable stack as shared object requires");
   if (mode & __RTLD_SPROF)
     return l;
 
-  /* Signal that we are going to add new objects.  */
-  struct r_debug *r = _dl_debug_update (nsid);
-  if (r->r_state == RT_CONSISTENT)
-    {
-#ifdef SHARED
-      /* Auditing checkpoint: we are going to add new objects.  Since this
-         is called after _dl_add_to_namespace_list the namespace is guaranteed
-	 to not be empty.  */
-      if ((mode & __RTLD_AUDIT) == 0)
-	_dl_audit_activity_nsid (nsid, LA_ACT_ADD);
-#endif
-
-      /* Notify the debugger we have added some objects.  We need to
-	 call _dl_debug_initialize in a static program in case dynamic
-	 linking has not been used before.  */
-      r->r_state = RT_ADD;
-      _dl_debug_state ();
-      LIBC_PROBE (map_start, 2, nsid, r);
-    }
-  else
-    assert (r->r_state == RT_ADD);
-
-#ifdef SHARED
-  /* Auditing checkpoint: we have a new object.  */
-  if (!GL(dl_ns)[l->l_ns]._ns_loaded->l_auditing)
-    _dl_audit_objopen (l, nsid);
-#endif
+  _dl_notify_new_object (mode, nsid, l);
 
   return l;
 }
diff --git a/elf/tst-audit23.c b/elf/tst-audit23.c
index 895dab08ea..c8c6553b8f 100644
--- a/elf/tst-audit23.c
+++ b/elf/tst-audit23.c
@@ -17,6 +17,7 @@ 
    <https://www.gnu.org/licenses/>.  */
 
 #include <array_length.h>
+#include <endswith.h>
 #include <errno.h>
 #include <getopt.h>
 #include <link.h>
@@ -106,8 +107,9 @@  do_test (int argc, char *argv[])
      4. libgcc_s.so (one some architectures, for libsupport)
      5. tst-audit23mod.so
      6. libc.so (LM_ID_NEWLM).
+     7. loader (proxy link map in new namespace)
         vdso (optional and ignored).  */
-  enum { max_objs = 6 };
+  enum { max_objs = 7 };
   struct la_obj_t
   {
     char *lname;
@@ -236,7 +238,9 @@  do_test (int argc, char *argv[])
 
   for (size_t i = 0; i < nobjs; i++)
     {
-      TEST_COMPARE (objs[i].closed, true);
+      /* This subtest currently does not pass because of bug 32065.  */
+      if (! (endswith (objs[i].lname, LD_SO) && objs[i].lmid != LM_ID_BASE))
+	TEST_COMPARE (objs[i].closed, true);
       free (objs[i].lname);
     }