[PATCH 0/3] Add DT_AUDIT support [BZ #24943]

[PATCH 0/3] Add DT_AUDIT support [BZ #24943]

[Florian Weimer]

xgetline has been committed separately with slightly different behavior.
I think the zero-on-EOF behavior is preferable, so I'm switching the
implementation to that.

It turns out that it is possible to simplify the audit string list
processing, so the second commit does that.

Tested on x86_64-linux-gnu and i686-linux-gnu.

Thanks,
Florian

Florian Weimer (3):
  support: Change xgetline to return 0 on EOF
  elf: Simplify handling of lists of audit strings
  elf: Implement DT_AUDIT, DT_DEPAUDIT support [BZ #24943]

 NEWS                            |   3 +
 elf/Makefile                    |  22 ++-
 elf/rtld.c                      | 247 +++++++++++++++++---------------
 elf/tst-audit14.c               |  46 ++++++
 elf/tst-audit15.c               |  50 +++++++
 elf/tst-audit16.c               |  54 +++++++
 elf/tst-auditlogmod-1.c         |  27 ++++
 elf/tst-auditlogmod-2.c         |  27 ++++
 elf/tst-auditlogmod-3.c         |  27 ++++
 support/support_process_state.c |   2 +-
 support/xgetline.c              |  22 +--
 support/xstdio.h                |   5 +-
 12 files changed, 405 insertions(+), 127 deletions(-)
 create mode 100644 elf/tst-audit14.c
 create mode 100644 elf/tst-audit15.c
 create mode 100644 elf/tst-audit16.c
 create mode 100644 elf/tst-auditlogmod-1.c
 create mode 100644 elf/tst-auditlogmod-2.c
 create mode 100644 elf/tst-auditlogmod-3.c

-- 
2.25.1

[PATCH 1/3] support: Change xgetline to return 0 on EOF

[Florian Weimer]

The advantage is that the buffer will always contain the number
of characters as returned from the function, which allows one to use
a sequence like

  /* No more audit module output.  */
  line_length = xgetline (&buffer, &buffer_length, fp);
  TEST_COMPARE_BLOB ("", 0, buffer, line_length);

to check for an expected EOF, while also reporting any unexpected
extra data encountered.
---
 support/support_process_state.c |  2 +-
 support/xgetline.c              | 22 ++++++++++++++--------
 support/xstdio.h                |  5 ++++-
 3 files changed, 19 insertions(+), 10 deletions(-)

diff --git a/support/support_process_state.c b/support/support_process_state.c
index 76dc798728..e303c78fc8 100644
--- a/support/support_process_state.c
+++ b/support/support_process_state.c
@@ -59,7 +59,7 @@ support_process_state_wait (pid_t pid, enum support_process_state state)
   for (;;)
     {
       char cur_state = -1;
-      while (xgetline (&line, &linesiz, fstatus) != -1)
+      while (xgetline (&line, &linesiz, fstatus) > 0)
 	if (strncmp (line, "State:", strlen ("State:")) == 0)
 	  {
 	    TEST_COMPARE (sscanf (line, "%*s %c", &cur_state), 1);
diff --git a/support/xgetline.c b/support/xgetline.c
index 180bc2db95..d91c09ac10 100644
--- a/support/xgetline.c
+++ b/support/xgetline.c
@@ -18,16 +18,22 @@
 
 #include <support/xstdio.h>
 #include <support/check.h>
-#include <errno.h>
 
-ssize_t
+size_t
 xgetline (char **lineptr, size_t *n, FILE *stream)
 {
-  int old_errno = errno;
-  errno = 0;
-  size_t ret = getline (lineptr, n, stream);
-  if (!feof (stream) && ferror (stream))
-    FAIL_EXIT1 ("getline failed: %m");
-  errno = old_errno;
+  TEST_VERIFY (!ferror (stream));
+  ssize_t ret = getline (lineptr, n, stream);
+  if (ferror (stream))
+    {
+      TEST_VERIFY (ret < 0);
+      FAIL_EXIT1 ("getline: %m");
+    }
+  if (feof (stream))
+    {
+      TEST_VERIFY (ret <= 0);
+      return 0;
+    }
+  TEST_VERIFY (ret > 0);
   return ret;
 }
diff --git a/support/xstdio.h b/support/xstdio.h
index 143957b6a8..807c3afc43 100644
--- a/support/xstdio.h
+++ b/support/xstdio.h
@@ -27,7 +27,10 @@ __BEGIN_DECLS
 FILE *xfopen (const char *path, const char *mode);
 void xfclose (FILE *);
 
-ssize_t xgetline (char **lineptr, size_t *n, FILE *stream);
+/* Read a line from FP, using getline.  *BUFFER must be NULL, or a
+   heap-allocated pointer of *LENGTH bytes.  Return the number of
+   bytes in the line if a line was read, or 0 on EOF.  */
+size_t xgetline (char **lineptr, size_t *n, FILE *stream);
 
 __END_DECLS
 
-- 
2.25.1

[PATCH 2/3] elf: Simplify handling of lists of audit strings

[Florian Weimer]

All list elements are colon-separated strings, and there is a hard
upper limit for the number of audit modules, so it is possible to
pre-allocate a fixed-size array of strings to which the LD_AUDIT
environment variable and --audit arguments are added.

Also eliminate the global variables for the audit list because
the list is only needed briefly during startup.

There is a slight behavior change: All duplicate LD_AUDIT environment
variables are now processed, not just the last one as before.  However,
such environment vectors are invalid anyway.
---
 elf/rtld.c | 230 ++++++++++++++++++++++++++---------------------------
 1 file changed, 113 insertions(+), 117 deletions(-)

diff --git a/elf/rtld.c b/elf/rtld.c
index 51dfaf966a..fe4dfbec67 100644
--- a/elf/rtld.c
+++ b/elf/rtld.c
@@ -45,6 +45,7 @@
 #include <stap-probe.h>
 #include <stackinfo.h>
 #include <not-cancel.h>
+#include <array_length.h>
 
 #include <assert.h>
 
@@ -109,8 +110,53 @@ static void print_missing_version (int errcode, const char *objname,
 /* Print the various times we collected.  */
 static void print_statistics (const hp_timing_t *total_timep);
 
-/* Add audit objects.  */
-static void process_dl_audit (char *str);
+/* Length limits for names and paths, to protect the dynamic linker,
+   particularly when __libc_enable_secure is active.  */
+#ifdef NAME_MAX
+# define SECURE_NAME_LIMIT NAME_MAX
+#else
+# define SECURE_NAME_LIMIT 255
+#endif
+#ifdef PATH_MAX
+# define SECURE_PATH_LIMIT PATH_MAX
+#else
+# define SECURE_PATH_LIMIT 1024
+#endif
+
+/* Strings containing colon-separated lists of audit modules.  */
+struct audit_list
+{
+  /* Array of strings containing colon-separated path lists.  Each
+     audit module needs its own namespace, so pre-allocate the largest
+     possible list.  */
+  const char *audit_strings[DL_NNS];
+
+  /* Number of entries added to audit_strings.  */
+  size_t length;
+
+  /* Index into the audit_strings array (for the iteration phase).  */
+  size_t current_index;
+
+  /* Tail of audit_strings[current_index] which still needs
+     processing.  */
+  const char *current_tail;
+
+  /* Scratch buffer for returning a name which is part of the strings
+     in audit_strings.  */
+  char fname[SECURE_NAME_LIMIT];
+};
+
+/* Creates an empty audit list.  */
+static void audit_list_init (struct audit_list *);
+
+/* Add a string to the end of the audit list, for later parsing.  Must
+   not be called after audit_list_next.  */
+static void audit_list_add_string (struct audit_list *, const char *);
+
+/* Extract the next audit module from the audit list.  Only modules
+   for which dso_name_valid_for_suid is true are returned.  Must be
+   called after all the audit_list_add_string calls.  */
+static const char *audit_list_next (struct audit_list *);
 
 /* This is a list of all the modes the dynamic loader can be in.  */
 enum mode { normal, list, verify, trace };
@@ -118,7 +164,7 @@ enum mode { normal, list, verify, trace };
 /* Process all environments variables the dynamic linker must recognize.
    Since all of them start with `LD_' we are a bit smarter while finding
    all the entries.  */
-static void process_envvars (enum mode *modep);
+static void process_envvars (enum mode *modep, struct audit_list *);
 
 #ifdef DL_ARGV_NOT_RELRO
 int _dl_argc attribute_hidden;
@@ -146,19 +192,6 @@ uintptr_t __pointer_chk_guard_local
 strong_alias (__pointer_chk_guard_local, __pointer_chk_guard)
 #endif
 
-/* Length limits for names and paths, to protect the dynamic linker,
-   particularly when __libc_enable_secure is active.  */
-#ifdef NAME_MAX
-# define SECURE_NAME_LIMIT NAME_MAX
-#else
-# define SECURE_NAME_LIMIT 255
-#endif
-#ifdef PATH_MAX
-# define SECURE_PATH_LIMIT PATH_MAX
-#else
-# define SECURE_PATH_LIMIT 1024
-#endif
-
 /* Check that AT_SECURE=0, or that the passed name does not contain
    directories and is not overly long.  Reject empty names
    unconditionally.  */
@@ -176,89 +209,76 @@ dso_name_valid_for_suid (const char *p)
   return *p != '\0';
 }
 
-/* LD_AUDIT variable contents.  Must be processed before the
-   audit_list below.  */
-const char *audit_list_string;
-
-/* Cyclic list of auditing DSOs.  audit_list->next is the first
-   element.  */
-static struct audit_list
+static void
+audit_list_init (struct audit_list *list)
 {
-  const char *name;
-  struct audit_list *next;
-} *audit_list;
+  list->length = 0;
+  list->current_index = 0;
+  list->current_tail = NULL;
+}
 
-/* Iterator for audit_list_string followed by audit_list.  */
-struct audit_list_iter
+static void
+audit_list_add_string (struct audit_list *list, const char *string)
 {
-  /* Tail of audit_list_string still needing processing, or NULL.  */
-  const char *audit_list_tail;
+  /* Empty strings do not load anything.  */
+  if (*string == '\0')
+    return;
 
-  /* The list element returned in the previous iteration.  NULL before
-     the first element.  */
-  struct audit_list *previous;
+  /* Grow the array if necessary.  */
+  if (list->length == array_length (list->audit_strings))
+    _dl_fatal_printf ("Too many audit modules requested\n");
 
-  /* Scratch buffer for returning a name which is part of
-     audit_list_string.  */
-  char fname[SECURE_NAME_LIMIT];
-};
+  list->audit_strings[list->length++] = string;
 
-/* Initialize an audit list iterator.  */
-static void
-audit_list_iter_init (struct audit_list_iter *iter)
-{
-  iter->audit_list_tail = audit_list_string;
-  iter->previous = NULL;
+  /* Initialize processing of the first string for
+     audit_list_next.  */
+  if (list->length == 1)
+    list->current_tail = string;
 }
 
-/* Iterate through both audit_list_string and audit_list.  */
 static const char *
-audit_list_iter_next (struct audit_list_iter *iter)
+audit_list_next (struct audit_list *list)
 {
-  if (iter->audit_list_tail != NULL)
+  if (list->current_tail == NULL)
+    return NULL;
+
+  while (true)
     {
-      /* First iterate over audit_list_string.  */
-      while (*iter->audit_list_tail != '\0')
+      /* Advance to the next string in audit_strings if the current
+	 string has been exhausted.  */
+      while (*list->current_tail == '\0')
 	{
-	  /* Split audit list at colon.  */
-	  size_t len = strcspn (iter->audit_list_tail, ":");
-	  if (len > 0 && len < sizeof (iter->fname))
+	  ++list->current_index;
+	  if (list->current_index == list->length)
 	    {
-	      memcpy (iter->fname, iter->audit_list_tail, len);
-	      iter->fname[len] = '\0';
+	      list->current_tail = NULL;
+	      return NULL;
 	    }
-	  else
-	    /* Do not return this name to the caller.  */
-	    iter->fname[0] = '\0';
-
-	  /* Skip over the substring and the following delimiter.  */
-	  iter->audit_list_tail += len;
-	  if (*iter->audit_list_tail == ':')
-	    ++iter->audit_list_tail;
-
-	  /* If the name is valid, return it.  */
-	  if (dso_name_valid_for_suid (iter->fname))
-	    return iter->fname;
-	  /* Otherwise, wrap around and try the next name.  */
+	  list->current_tail = list->audit_strings[list->current_index];
 	}
-      /* Fall through to the procesing of audit_list.  */
-    }
 
-  if (iter->previous == NULL)
-    {
-      if (audit_list == NULL)
-	/* No pre-parsed audit list.  */
-	return NULL;
-      /* Start of audit list.  The first list element is at
-	 audit_list->next (cyclic list).  */
-      iter->previous = audit_list->next;
-      return iter->previous->name;
+      /* Split the in-string audit list at the next colon colon.  */
+      size_t len = strcspn (list->current_tail, ":");
+      if (len > 0 && len < sizeof (list->fname))
+	{
+	  memcpy (list->fname, list->current_tail, len);
+	  list->fname[len] = '\0';
+	}
+      else
+	/* Mark the name as unusable for dso_name_valid_for_suid.  */
+	list->fname[0] = '\0';
+
+      /* Skip over the substring and the following delimiter.  */
+      list->current_tail += len;
+      if (*list->current_tail == ':')
+	++list->current_tail;
+
+      /* If the name is valid, return it.  */
+      if (dso_name_valid_for_suid (list->fname))
+	return list->fname;
+
+      /* Otherwise wrap around to find the next list element. .  */
     }
-  if (iter->previous == audit_list)
-    /* Cyclic list wrap-around.  */
-    return NULL;
-  iter->previous = iter->previous->next;
-  return iter->previous->name;
 }
 
 #ifndef HAVE_INLINED_SYSCALLS
@@ -1062,15 +1082,13 @@ notify_audit_modules_of_loaded_object (struct link_map *map)
 
 /* Load all audit modules.  */
 static void
-load_audit_modules (struct link_map *main_map)
+load_audit_modules (struct link_map *main_map, struct audit_list *audit_list)
 {
   struct audit_ifaces *last_audit = NULL;
-  struct audit_list_iter al_iter;
-  audit_list_iter_init (&al_iter);
 
   while (true)
     {
-      const char *name = audit_list_iter_next (&al_iter);
+      const char *name = audit_list_next (audit_list);
       if (name == NULL)
 	break;
       load_audit_module (name, &last_audit);
@@ -1102,6 +1120,9 @@ dl_main (const ElfW(Phdr) *phdr,
   bool rtld_is_main = false;
   void *tcbp = NULL;
 
+  struct audit_list audit_list;
+  audit_list_init (&audit_list);
+
   GL(dl_init_static_tls) = &_dl_nothread_init_static_tls;
 
 #if defined SHARED && defined _LIBC_REENTRANT \
@@ -1115,7 +1136,7 @@ dl_main (const ElfW(Phdr) *phdr,
   GL(dl_make_stack_executable_hook) = &_dl_make_stack_executable;
 
   /* Process the environment variable which control the behaviour.  */
-  process_envvars (&mode);
+  process_envvars (&mode, &audit_list);
 
 #ifndef HAVE_INLINED_SYSCALLS
   /* Set up a flag which tells we are just starting.  */
@@ -1189,7 +1210,7 @@ dl_main (const ElfW(Phdr) *phdr,
 	  }
 	else if (! strcmp (_dl_argv[1], "--audit") && _dl_argc > 2)
 	  {
-	    process_dl_audit (_dl_argv[2]);
+	    audit_list_add_string (&audit_list, _dl_argv[2]);
 
 	    _dl_skip_args += 2;
 	    _dl_argc -= 2;
@@ -1619,8 +1640,7 @@ ERROR: '%s': cannot process note segment.\n", _dl_argv[0]);
 
   /* If we have auditing DSOs to load, do it now.  */
   bool need_security_init = true;
-  if (__glibc_unlikely (audit_list != NULL)
-      || __glibc_unlikely (audit_list_string != NULL))
+  if (audit_list.length > 0)
     {
       /* Since we start using the auditing DSOs right away we need to
 	 initialize the data structures now.  */
@@ -1633,7 +1653,7 @@ ERROR: '%s': cannot process note segment.\n", _dl_argv[0]);
       security_init ();
       need_security_init = false;
 
-      load_audit_modules (main_map);
+      load_audit_modules (main_map, &audit_list);
     }
 
   /* Keep track of the currently loaded modules to count how many
@@ -2507,30 +2527,6 @@ a filename can be specified using the LD_DEBUG_OUTPUT environment variable.\n");
     }
 }
 \f
-static void
-process_dl_audit (char *str)
-{
-  /* The parameter is a colon separated list of DSO names.  */
-  char *p;
-
-  while ((p = (strsep) (&str, ":")) != NULL)
-    if (dso_name_valid_for_suid (p))
-      {
-	/* This is using the local malloc, not the system malloc.  The
-	   memory can never be freed.  */
-	struct audit_list *newp = malloc (sizeof (*newp));
-	newp->name = p;
-
-	if (audit_list == NULL)
-	  audit_list = newp->next = newp;
-	else
-	  {
-	    newp->next = audit_list->next;
-	    audit_list = audit_list->next = newp;
-	  }
-      }
-}
-\f
 /* Process all environments variables the dynamic linker must recognize.
    Since all of them start with `LD_' we are a bit smarter while finding
    all the entries.  */
@@ -2538,7 +2534,7 @@ extern char **_environ attribute_hidden;
 
 
 static void
-process_envvars (enum mode *modep)
+process_envvars (enum mode *modep, struct audit_list *audit_list)
 {
   char **runp = _environ;
   char *envline;
@@ -2578,7 +2574,7 @@ process_envvars (enum mode *modep)
 	      break;
 	    }
 	  if (memcmp (envline, "AUDIT", 5) == 0)
-	    audit_list_string = &envline[6];
+	    audit_list_add_string (audit_list, &envline[6]);
 	  break;
 
 	case 7:
-- 
2.25.1

[PATCH 3/3] elf: Implement DT_AUDIT, DT_DEPAUDIT support [BZ #24943]

[Florian Weimer]

binutils ld has supported --audit, --depaudit for a long time,
only support in glibc has been missing.
---
 NEWS                    |  3 +++
 elf/Makefile            | 22 +++++++++++++++--
 elf/rtld.c              | 23 +++++++++++++++++-
 elf/tst-audit14.c       | 46 +++++++++++++++++++++++++++++++++++
 elf/tst-audit15.c       | 50 ++++++++++++++++++++++++++++++++++++++
 elf/tst-audit16.c       | 54 +++++++++++++++++++++++++++++++++++++++++
 elf/tst-auditlogmod-1.c | 27 +++++++++++++++++++++
 elf/tst-auditlogmod-2.c | 27 +++++++++++++++++++++
 elf/tst-auditlogmod-3.c | 27 +++++++++++++++++++++
 9 files changed, 276 insertions(+), 3 deletions(-)
 create mode 100644 elf/tst-audit14.c
 create mode 100644 elf/tst-audit15.c
 create mode 100644 elf/tst-audit16.c
 create mode 100644 elf/tst-auditlogmod-1.c
 create mode 100644 elf/tst-auditlogmod-2.c
 create mode 100644 elf/tst-auditlogmod-3.c

diff --git a/NEWS b/NEWS
index 466c203633..b5d7b34f8a 100644
--- a/NEWS
+++ b/NEWS
@@ -11,6 +11,9 @@ Major new features:
 
   * New locale added: ckb_IQ (Kurdish/Sorani spoken in Iraq)
 
+* The GNU C Library now loads audit modules listed in the DT_AUDIT and
+  DT_DEPAUDIT dynamic section entries of the main executable.
+
 Deprecated and removed features, and other changes affecting compatibility:
 
   [Add deprecations, removals and changes affecting compatibility here]
diff --git a/elf/Makefile b/elf/Makefile
index da689a2c7b..9b1d58c7ad 100644
--- a/elf/Makefile
+++ b/elf/Makefile
@@ -202,7 +202,8 @@ tests += restest1 preloadtest loadfail multiload origtest resolvfail \
 	 tst-sonamemove-link tst-sonamemove-dlopen tst-dlopen-tlsmodid \
 	 tst-dlopen-self tst-auditmany tst-initfinilazyfail tst-dlopenfail \
 	 tst-dlopenfail-2 \
-	 tst-filterobj tst-filterobj-dlopen tst-auxobj tst-auxobj-dlopen
+	 tst-filterobj tst-filterobj-dlopen tst-auxobj tst-auxobj-dlopen \
+	 tst-audit14 tst-audit15 tst-audit16
 #	 reldep9
 tests-internal += loadtest unload unload2 circleload1 \
 	 neededtest neededtest2 neededtest3 neededtest4 \
@@ -314,7 +315,8 @@ modules-names = testobj1 testobj2 testobj3 testobj4 testobj5 testobj6 \
 		tst-initlazyfailmod tst-finilazyfailmod \
 		tst-dlopenfailmod1 tst-dlopenfaillinkmod tst-dlopenfailmod2 \
 		tst-dlopenfailmod3 tst-ldconfig-ld-mod \
-		tst-filterobj-flt tst-filterobj-aux tst-filterobj-filtee
+		tst-filterobj-flt tst-filterobj-aux tst-filterobj-filtee \
+		tst-auditlogmod-1 tst-auditlogmod-2 tst-auditlogmod-3
 # Most modules build with _ISOMAC defined, but those filtered out
 # depend on internal headers.
 modules-names-tests = $(filter-out ifuncmod% tst-libc_dlvsym-dso tst-tlsmod%,\
@@ -1498,6 +1500,22 @@ $(objpfx)tst-auditmany.out: $(objpfx)tst-auditmanymod1.so \
 tst-auditmany-ENV = \
   LD_AUDIT=tst-auditmanymod1.so:tst-auditmanymod2.so:tst-auditmanymod3.so:tst-auditmanymod4.so:tst-auditmanymod5.so:tst-auditmanymod6.so:tst-auditmanymod7.so:tst-auditmanymod8.so:tst-auditmanymod9.so
 
+LDFLAGS-tst-audit14 = -Wl,--audit=tst-auditlogmod-1.so
+$(objpfx)tst-auditlogmod-1.so: $(libsupport)
+$(objpfx)tst-audit14.out: $(objpfx)tst-auditlogmod-1.so
+LDFLAGS-tst-audit15 = \
+  -Wl,--audit=tst-auditlogmod-1.so,--depaudit=tst-auditlogmod-2.so
+$(objpfx)tst-auditlogmod-2.so: $(libsupport)
+$(objpfx)tst-audit15.out: \
+  $(objpfx)tst-auditlogmod-1.so $(objpfx)tst-auditlogmod-2.so
+LDFLAGS-tst-audit16 = \
+  -Wl,--audit=tst-auditlogmod-1.so:tst-auditlogmod-2.so \
+  -Wl,--depaudit=tst-auditlogmod-3.so
+$(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
+
 # 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/rtld.c b/elf/rtld.c
index fe4dfbec67..b1938913a1 100644
--- a/elf/rtld.c
+++ b/elf/rtld.c
@@ -153,9 +153,17 @@ static void audit_list_init (struct audit_list *);
    not be called after audit_list_next.  */
 static void audit_list_add_string (struct audit_list *, const char *);
 
+/* Add the audit strings from the link map, found in the dynamic
+   segment at TG (either DT_AUDIT and DT_DEPAUDIT).  Must be called
+   before audit_list_next.  */
+static void audit_list_add_dynamic_tag (struct audit_list *,
+					struct link_map *,
+					unsigned int tag);
+
 /* Extract the next audit module from the audit list.  Only modules
    for which dso_name_valid_for_suid is true are returned.  Must be
-   called after all the audit_list_add_string calls.  */
+   called after all the audit_list_add_string,
+   audit_list_add_dynamic_tags calls.  */
 static const char *audit_list_next (struct audit_list *);
 
 /* This is a list of all the modes the dynamic loader can be in.  */
@@ -236,6 +244,16 @@ audit_list_add_string (struct audit_list *list, const char *string)
     list->current_tail = string;
 }
 
+static void
+audit_list_add_dynamic_tag (struct audit_list *list, struct link_map *main_map,
+			    unsigned int tag)
+{
+  ElfW(Dyn) *info = main_map->l_info[ADDRIDX (tag)];
+  const char *strtab = (const char *) D_PTR (main_map, l_info[DT_STRTAB]);
+  if (info != NULL)
+    audit_list_add_string (list, strtab + info->d_un.d_val);
+}
+
 static const char *
 audit_list_next (struct audit_list *list)
 {
@@ -1638,6 +1656,9 @@ ERROR: '%s': cannot process note segment.\n", _dl_argv[0]);
     /* Assign a module ID.  Do this before loading any audit modules.  */
     GL(dl_rtld_map).l_tls_modid = _dl_next_tls_modid ();
 
+  audit_list_add_dynamic_tag (&audit_list, main_map, DT_AUDIT);
+  audit_list_add_dynamic_tag (&audit_list, main_map, DT_DEPAUDIT);
+
   /* If we have auditing DSOs to load, do it now.  */
   bool need_security_init = true;
   if (audit_list.length > 0)
diff --git a/elf/tst-audit14.c b/elf/tst-audit14.c
new file mode 100644
index 0000000000..73e6634e35
--- /dev/null
+++ b/elf/tst-audit14.c
@@ -0,0 +1,46 @@
+/* Main program with DT_AUDIT.  One audit module.
+   Copyright (C) 2020 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
+   <http://www.gnu.org/licenses/>.  */
+
+#include <stdlib.h>
+#include <string.h>
+#include <support/check.h>
+#include <support/xstdio.h>
+
+static int
+do_test (void)
+{
+  /* Verify what the audit module has written.  This test assumes that
+     standard output has been redirected to a regular file.  */
+  FILE *fp = xfopen ("/dev/stdout", "r");
+
+  char *buffer = NULL;
+  size_t buffer_length = 0;
+  size_t line_length = xgetline (&buffer, &buffer_length, fp);
+  const char *message = "info: tst-auditlogmod-1.so loaded\n";
+  TEST_COMPARE_BLOB (message, strlen (message), buffer, line_length);
+
+  /* No more audit module output.  */
+  line_length = xgetline (&buffer, &buffer_length, fp);
+  TEST_COMPARE_BLOB ("", 0, buffer, line_length);
+
+  free (buffer);
+  xfclose (fp);
+  return 0;
+}
+
+#include <support/test-driver.c>
diff --git a/elf/tst-audit15.c b/elf/tst-audit15.c
new file mode 100644
index 0000000000..3d6a31c242
--- /dev/null
+++ b/elf/tst-audit15.c
@@ -0,0 +1,50 @@
+/* Main program with DT_AUDIT and DT_DEPAUDIT.  Two audit modules.
+   Copyright (C) 2020 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
+   <http://www.gnu.org/licenses/>.  */
+
+#include <stdlib.h>
+#include <string.h>
+#include <support/check.h>
+#include <support/xstdio.h>
+
+static int
+do_test (void)
+{
+  /* Verify what the audit modules have written.  This test assumes
+     that standard output has been redirected to a regular file.  */
+  FILE *fp = xfopen ("/dev/stdout", "r");
+
+  char *buffer = NULL;
+  size_t buffer_length = 0;
+  size_t line_length = xgetline (&buffer, &buffer_length, fp);
+  const char *message = "info: tst-auditlogmod-1.so loaded\n";
+  TEST_COMPARE_BLOB (message, strlen (message), buffer, line_length);
+
+  line_length = xgetline (&buffer, &buffer_length, fp);
+  message = "info: tst-auditlogmod-2.so loaded\n";
+  TEST_COMPARE_BLOB (message, strlen (message), buffer, line_length);
+
+  /* No more audit module output.  */
+  line_length = xgetline (&buffer, &buffer_length, fp);
+  TEST_COMPARE_BLOB ("", 0, buffer, line_length);
+
+  free (buffer);
+  xfclose (fp);
+  return 0;
+}
+
+#include <support/test-driver.c>
diff --git a/elf/tst-audit16.c b/elf/tst-audit16.c
new file mode 100644
index 0000000000..1a7d6eee5e
--- /dev/null
+++ b/elf/tst-audit16.c
@@ -0,0 +1,54 @@
+/* Main program with DT_AUDIT and DT_DEPAUDIT.  Three audit modules.
+   Copyright (C) 2020 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
+   <http://www.gnu.org/licenses/>.  */
+
+#include <stdlib.h>
+#include <string.h>
+#include <support/check.h>
+#include <support/xstdio.h>
+
+static int
+do_test (void)
+{
+  /* Verify what the audit modules have written.  This test assumes
+     that standard output has been redirected to a regular file.  */
+  FILE *fp = xfopen ("/dev/stdout", "r");
+
+  char *buffer = NULL;
+  size_t buffer_length = 0;
+  size_t line_length = xgetline (&buffer, &buffer_length, fp);
+  const char *message = "info: tst-auditlogmod-1.so loaded\n";
+  TEST_COMPARE_BLOB (message, strlen (message), buffer, line_length);
+
+  line_length = xgetline (&buffer, &buffer_length, fp);
+  message = "info: tst-auditlogmod-2.so loaded\n";
+  TEST_COMPARE_BLOB (message, strlen (message), buffer, line_length);
+
+  line_length = xgetline (&buffer, &buffer_length, fp);
+  message = "info: tst-auditlogmod-3.so loaded\n";
+  TEST_COMPARE_BLOB (message, strlen (message), buffer, line_length);
+
+  /* No more audit module output.  */
+  line_length = xgetline (&buffer, &buffer_length, fp);
+  TEST_COMPARE_BLOB ("", 0, buffer, line_length);
+
+  free (buffer);
+  xfclose (fp);
+  return 0;
+}
+
+#include <support/test-driver.c>
diff --git a/elf/tst-auditlogmod-1.c b/elf/tst-auditlogmod-1.c
new file mode 100644
index 0000000000..e6b8cd9094
--- /dev/null
+++ b/elf/tst-auditlogmod-1.c
@@ -0,0 +1,27 @@
+/* Audit module which logs that it was loaded.  Variant 1.
+   Copyright (C) 2020 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
+   <http://www.gnu.org/licenses/>.  */
+
+#include <link.h>
+#include <support/support.h>
+
+unsigned int
+la_version (unsigned int v)
+{
+  write_message ("info: tst-auditlogmod-1.so loaded\n");
+  return LAV_CURRENT;
+}
diff --git a/elf/tst-auditlogmod-2.c b/elf/tst-auditlogmod-2.c
new file mode 100644
index 0000000000..9e7f0acabc
--- /dev/null
+++ b/elf/tst-auditlogmod-2.c
@@ -0,0 +1,27 @@
+/* Audit module which logs that it was loaded.  Variant 2.
+   Copyright (C) 2020 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
+   <http://www.gnu.org/licenses/>.  */
+
+#include <link.h>
+#include <support/support.h>
+
+unsigned int
+la_version (unsigned int v)
+{
+  write_message ("info: tst-auditlogmod-2.so loaded\n");
+  return LAV_CURRENT;
+}
diff --git a/elf/tst-auditlogmod-3.c b/elf/tst-auditlogmod-3.c
new file mode 100644
index 0000000000..c4c1a58145
--- /dev/null
+++ b/elf/tst-auditlogmod-3.c
@@ -0,0 +1,27 @@
+/* Audit module which logs that it was loaded.  Variant 3.
+   Copyright (C) 2020 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
+   <http://www.gnu.org/licenses/>.  */
+
+#include <link.h>
+#include <support/support.h>
+
+unsigned int
+la_version (unsigned int v)
+{
+  write_message ("info: tst-auditlogmod-3.so loaded\n");
+  return LAV_CURRENT;
+}
-- 
2.25.1

Re: [PATCH 0/3] Add DT_AUDIT support [BZ #24943]

[Carlos O'Donell]

On 4/3/20 8:02 AM, Florian Weimer via Libc-alpha wrote:
> xgetline has been committed separately with slightly different behavior.
> I think the zero-on-EOF behavior is preferable, so I'm switching the
> implementation to that.
> 
> It turns out that it is possible to simplify the audit string list
> processing, so the second commit does that.
> 
> Tested on x86_64-linux-gnu and i686-linux-gnu.

Thanks for working on this.

The auditors, despite the implementation being a bit immature, are a
useful way to alter the dynamic loaders behaviour.

Your patches go a long way to improving this framework, and you add
some nice internal API design examples.

-- 
Cheers,
Carlos.

Re: [PATCH 1/3] support: Change xgetline to return 0 on EOF

[Carlos O'Donell]

On 4/3/20 8:02 AM, Florian Weimer via Libc-alpha wrote:
> The advantage is that the buffer will always contain the number
> of characters as returned from the function, which allows one to use
> a sequence like

I agree that returning -1 is not the most useful semantic here, and
relaxing xgetline to be more like getline is better.

>   /* No more audit module output.  */
>   line_length = xgetline (&buffer, &buffer_length, fp);
>   TEST_COMPARE_BLOB ("", 0, buffer, line_length);
> 
> to check for an expected EOF, while also reporting any unexpected
> extra data encountered.

OK for master.

Reviewed-by: Carlos O'Donell <carlos@redhat.com>

> ---
>  support/support_process_state.c |  2 +-
>  support/xgetline.c              | 22 ++++++++++++++--------
>  support/xstdio.h                |  5 ++++-
>  3 files changed, 19 insertions(+), 10 deletions(-)
> 
> diff --git a/support/support_process_state.c b/support/support_process_state.c
> index 76dc798728..e303c78fc8 100644
> --- a/support/support_process_state.c
> +++ b/support/support_process_state.c
> @@ -59,7 +59,7 @@ support_process_state_wait (pid_t pid, enum support_process_state state)
>    for (;;)
>      {
>        char cur_state = -1;
> -      while (xgetline (&line, &linesiz, fstatus) != -1)
> +      while (xgetline (&line, &linesiz, fstatus) > 0)

OK.

>  	if (strncmp (line, "State:", strlen ("State:")) == 0)
>  	  {
>  	    TEST_COMPARE (sscanf (line, "%*s %c", &cur_state), 1);
> diff --git a/support/xgetline.c b/support/xgetline.c
> index 180bc2db95..d91c09ac10 100644
> --- a/support/xgetline.c
> +++ b/support/xgetline.c
> @@ -18,16 +18,22 @@
>  
>  #include <support/xstdio.h>
>  #include <support/check.h>
> -#include <errno.h>
>  
> -ssize_t
> +size_t
>  xgetline (char **lineptr, size_t *n, FILE *stream)
>  {
> -  int old_errno = errno;
> -  errno = 0;
> -  size_t ret = getline (lineptr, n, stream);
> -  if (!feof (stream) && ferror (stream))
> -    FAIL_EXIT1 ("getline failed: %m");
> -  errno = old_errno;
> +  TEST_VERIFY (!ferror (stream));
> +  ssize_t ret = getline (lineptr, n, stream);
> +  if (ferror (stream))
> +    {
> +      TEST_VERIFY (ret < 0);
> +      FAIL_EXIT1 ("getline: %m");
> +    }
> +  if (feof (stream))
> +    {
> +      TEST_VERIFY (ret <= 0);
> +      return 0;
> +    }
> +  TEST_VERIFY (ret > 0);

OK.

>    return ret;
>  }
> diff --git a/support/xstdio.h b/support/xstdio.h
> index 143957b6a8..807c3afc43 100644
> --- a/support/xstdio.h
> +++ b/support/xstdio.h
> @@ -27,7 +27,10 @@ __BEGIN_DECLS
>  FILE *xfopen (const char *path, const char *mode);
>  void xfclose (FILE *);
>  
> -ssize_t xgetline (char **lineptr, size_t *n, FILE *stream);
> +/* Read a line from FP, using getline.  *BUFFER must be NULL, or a
> +   heap-allocated pointer of *LENGTH bytes.  Return the number of
> +   bytes in the line if a line was read, or 0 on EOF.  */
> +size_t xgetline (char **lineptr, size_t *n, FILE *stream);

OK.

>  
>  __END_DECLS
>  
> 


-- 
Cheers,
Carlos.

Re: [PATCH 2/3] elf: Simplify handling of lists of audit strings

[Carlos O'Donell]

On 4/3/20 8:02 AM, Florian Weimer via Libc-alpha wrote:
> All list elements are colon-separated strings, and there is a hard
> upper limit for the number of audit modules, so it is possible to
> pre-allocate a fixed-size array of strings to which the LD_AUDIT
> environment variable and --audit arguments are added.
> 
> Also eliminate the global variables for the audit list because
> the list is only needed briefly during startup.
> 
> There is a slight behavior change: All duplicate LD_AUDIT environment
> variables are now processed, not just the last one as before.  However,
> such environment vectors are invalid anyway.

Agreed.

OK for master.

Reviewed-by: Carlos O'Donell <carlos@redhat.com>

> ---
>  elf/rtld.c | 230 ++++++++++++++++++++++++++---------------------------
>  1 file changed, 113 insertions(+), 117 deletions(-)
> 
> diff --git a/elf/rtld.c b/elf/rtld.c
> index 51dfaf966a..fe4dfbec67 100644
> --- a/elf/rtld.c
> +++ b/elf/rtld.c
> @@ -45,6 +45,7 @@
>  #include <stap-probe.h>
>  #include <stackinfo.h>
>  #include <not-cancel.h>
> +#include <array_length.h>
>  
>  #include <assert.h>
>  
> @@ -109,8 +110,53 @@ static void print_missing_version (int errcode, const char *objname,
>  /* Print the various times we collected.  */
>  static void print_statistics (const hp_timing_t *total_timep);
>  
> -/* Add audit objects.  */
> -static void process_dl_audit (char *str);
> +/* Length limits for names and paths, to protect the dynamic linker,
> +   particularly when __libc_enable_secure is active.  */
> +#ifdef NAME_MAX
> +# define SECURE_NAME_LIMIT NAME_MAX
> +#else
> +# define SECURE_NAME_LIMIT 255
> +#endif
> +#ifdef PATH_MAX
> +# define SECURE_PATH_LIMIT PATH_MAX
> +#else
> +# define SECURE_PATH_LIMIT 1024
> +#endif

OK.

> +
> +/* Strings containing colon-separated lists of audit modules.  */
> +struct audit_list
> +{
> +  /* Array of strings containing colon-separated path lists.  Each
> +     audit module needs its own namespace, so pre-allocate the largest
> +     possible list.  */
> +  const char *audit_strings[DL_NNS];
> +
> +  /* Number of entries added to audit_strings.  */
> +  size_t length;
> +
> +  /* Index into the audit_strings array (for the iteration phase).  */
> +  size_t current_index;
> +
> +  /* Tail of audit_strings[current_index] which still needs
> +     processing.  */
> +  const char *current_tail;
> +
> +  /* Scratch buffer for returning a name which is part of the strings
> +     in audit_strings.  */
> +  char fname[SECURE_NAME_LIMIT];
> +};
> +
> +/* Creates an empty audit list.  */
> +static void audit_list_init (struct audit_list *);
> +
> +/* Add a string to the end of the audit list, for later parsing.  Must
> +   not be called after audit_list_next.  */
> +static void audit_list_add_string (struct audit_list *, const char *);
> +
> +/* Extract the next audit module from the audit list.  Only modules
> +   for which dso_name_valid_for_suid is true are returned.  Must be
> +   called after all the audit_list_add_string calls.  */
> +static const char *audit_list_next (struct audit_list *);
>  
>  /* This is a list of all the modes the dynamic loader can be in.  */
>  enum mode { normal, list, verify, trace };
> @@ -118,7 +164,7 @@ enum mode { normal, list, verify, trace };
>  /* Process all environments variables the dynamic linker must recognize.
>     Since all of them start with `LD_' we are a bit smarter while finding
>     all the entries.  */
> -static void process_envvars (enum mode *modep);
> +static void process_envvars (enum mode *modep, struct audit_list *);
>  
>  #ifdef DL_ARGV_NOT_RELRO
>  int _dl_argc attribute_hidden;
> @@ -146,19 +192,6 @@ uintptr_t __pointer_chk_guard_local
>  strong_alias (__pointer_chk_guard_local, __pointer_chk_guard)
>  #endif
>  
> -/* Length limits for names and paths, to protect the dynamic linker,
> -   particularly when __libc_enable_secure is active.  */
> -#ifdef NAME_MAX
> -# define SECURE_NAME_LIMIT NAME_MAX
> -#else
> -# define SECURE_NAME_LIMIT 255
> -#endif
> -#ifdef PATH_MAX
> -# define SECURE_PATH_LIMIT PATH_MAX
> -#else
> -# define SECURE_PATH_LIMIT 1024
> -#endif
> -
>  /* Check that AT_SECURE=0, or that the passed name does not contain
>     directories and is not overly long.  Reject empty names
>     unconditionally.  */
> @@ -176,89 +209,76 @@ dso_name_valid_for_suid (const char *p)
>    return *p != '\0';
>  }
>  
> -/* LD_AUDIT variable contents.  Must be processed before the
> -   audit_list below.  */
> -const char *audit_list_string;
> -
> -/* Cyclic list of auditing DSOs.  audit_list->next is the first
> -   element.  */
> -static struct audit_list
> +static void
> +audit_list_init (struct audit_list *list)
>  {
> -  const char *name;
> -  struct audit_list *next;
> -} *audit_list;
> +  list->length = 0;
> +  list->current_index = 0;
> +  list->current_tail = NULL;
> +}

OK.

>  
> -/* Iterator for audit_list_string followed by audit_list.  */
> -struct audit_list_iter
> +static void
> +audit_list_add_string (struct audit_list *list, const char *string)
>  {
> -  /* Tail of audit_list_string still needing processing, or NULL.  */
> -  const char *audit_list_tail;
> +  /* Empty strings do not load anything.  */
> +  if (*string == '\0')
> +    return;
>  
> -  /* The list element returned in the previous iteration.  NULL before
> -     the first element.  */
> -  struct audit_list *previous;
> +  /* Grow the array if necessary.  */
> +  if (list->length == array_length (list->audit_strings))
> +    _dl_fatal_printf ("Too many audit modules requested\n");
>  
> -  /* Scratch buffer for returning a name which is part of
> -     audit_list_string.  */
> -  char fname[SECURE_NAME_LIMIT];
> -};
> +  list->audit_strings[list->length++] = string;
>  
> -/* Initialize an audit list iterator.  */
> -static void
> -audit_list_iter_init (struct audit_list_iter *iter)
> -{
> -  iter->audit_list_tail = audit_list_string;
> -  iter->previous = NULL;
> +  /* Initialize processing of the first string for
> +     audit_list_next.  */
> +  if (list->length == 1)
> +    list->current_tail = string;
>  }
>  
> -/* Iterate through both audit_list_string and audit_list.  */
>  static const char *
> -audit_list_iter_next (struct audit_list_iter *iter)
> +audit_list_next (struct audit_list *list)
>  {
> -  if (iter->audit_list_tail != NULL)
> +  if (list->current_tail == NULL)
> +    return NULL;
> +
> +  while (true)
>      {
> -      /* First iterate over audit_list_string.  */
> -      while (*iter->audit_list_tail != '\0')
> +      /* Advance to the next string in audit_strings if the current
> +	 string has been exhausted.  */
> +      while (*list->current_tail == '\0')
>  	{
> -	  /* Split audit list at colon.  */
> -	  size_t len = strcspn (iter->audit_list_tail, ":");
> -	  if (len > 0 && len < sizeof (iter->fname))
> +	  ++list->current_index;
> +	  if (list->current_index == list->length)
>  	    {
> -	      memcpy (iter->fname, iter->audit_list_tail, len);
> -	      iter->fname[len] = '\0';
> +	      list->current_tail = NULL;
> +	      return NULL;
>  	    }
> -	  else
> -	    /* Do not return this name to the caller.  */
> -	    iter->fname[0] = '\0';
> -
> -	  /* Skip over the substring and the following delimiter.  */
> -	  iter->audit_list_tail += len;
> -	  if (*iter->audit_list_tail == ':')
> -	    ++iter->audit_list_tail;
> -
> -	  /* If the name is valid, return it.  */
> -	  if (dso_name_valid_for_suid (iter->fname))
> -	    return iter->fname;
> -	  /* Otherwise, wrap around and try the next name.  */
> +	  list->current_tail = list->audit_strings[list->current_index];
>  	}
> -      /* Fall through to the procesing of audit_list.  */
> -    }
>  
> -  if (iter->previous == NULL)
> -    {
> -      if (audit_list == NULL)
> -	/* No pre-parsed audit list.  */
> -	return NULL;
> -      /* Start of audit list.  The first list element is at
> -	 audit_list->next (cyclic list).  */
> -      iter->previous = audit_list->next;
> -      return iter->previous->name;
> +      /* Split the in-string audit list at the next colon colon.  */
> +      size_t len = strcspn (list->current_tail, ":");
> +      if (len > 0 && len < sizeof (list->fname))
> +	{
> +	  memcpy (list->fname, list->current_tail, len);
> +	  list->fname[len] = '\0';
> +	}
> +      else
> +	/* Mark the name as unusable for dso_name_valid_for_suid.  */
> +	list->fname[0] = '\0';
> +
> +      /* Skip over the substring and the following delimiter.  */
> +      list->current_tail += len;
> +      if (*list->current_tail == ':')
> +	++list->current_tail;
> +
> +      /* If the name is valid, return it.  */
> +      if (dso_name_valid_for_suid (list->fname))
> +	return list->fname;
> +
> +      /* Otherwise wrap around to find the next list element. .  */
>      }
> -  if (iter->previous == audit_list)
> -    /* Cyclic list wrap-around.  */
> -    return NULL;
> -  iter->previous = iter->previous->next;
> -  return iter->previous->name;
>  }
>  
>  #ifndef HAVE_INLINED_SYSCALLS
> @@ -1062,15 +1082,13 @@ notify_audit_modules_of_loaded_object (struct link_map *map)
>  
>  /* Load all audit modules.  */
>  static void
> -load_audit_modules (struct link_map *main_map)
> +load_audit_modules (struct link_map *main_map, struct audit_list *audit_list)
>  {
>    struct audit_ifaces *last_audit = NULL;
> -  struct audit_list_iter al_iter;
> -  audit_list_iter_init (&al_iter);
>  
>    while (true)
>      {
> -      const char *name = audit_list_iter_next (&al_iter);
> +      const char *name = audit_list_next (audit_list);
>        if (name == NULL)
>  	break;
>        load_audit_module (name, &last_audit);
> @@ -1102,6 +1120,9 @@ dl_main (const ElfW(Phdr) *phdr,
>    bool rtld_is_main = false;
>    void *tcbp = NULL;
>  
> +  struct audit_list audit_list;
> +  audit_list_init (&audit_list);
> +
>    GL(dl_init_static_tls) = &_dl_nothread_init_static_tls;
>  
>  #if defined SHARED && defined _LIBC_REENTRANT \
> @@ -1115,7 +1136,7 @@ dl_main (const ElfW(Phdr) *phdr,
>    GL(dl_make_stack_executable_hook) = &_dl_make_stack_executable;
>  
>    /* Process the environment variable which control the behaviour.  */
> -  process_envvars (&mode);
> +  process_envvars (&mode, &audit_list);
>  
>  #ifndef HAVE_INLINED_SYSCALLS
>    /* Set up a flag which tells we are just starting.  */
> @@ -1189,7 +1210,7 @@ dl_main (const ElfW(Phdr) *phdr,
>  	  }
>  	else if (! strcmp (_dl_argv[1], "--audit") && _dl_argc > 2)
>  	  {
> -	    process_dl_audit (_dl_argv[2]);
> +	    audit_list_add_string (&audit_list, _dl_argv[2]);
>  
>  	    _dl_skip_args += 2;
>  	    _dl_argc -= 2;
> @@ -1619,8 +1640,7 @@ ERROR: '%s': cannot process note segment.\n", _dl_argv[0]);
>  
>    /* If we have auditing DSOs to load, do it now.  */
>    bool need_security_init = true;
> -  if (__glibc_unlikely (audit_list != NULL)
> -      || __glibc_unlikely (audit_list_string != NULL))
> +  if (audit_list.length > 0)
>      {
>        /* Since we start using the auditing DSOs right away we need to
>  	 initialize the data structures now.  */
> @@ -1633,7 +1653,7 @@ ERROR: '%s': cannot process note segment.\n", _dl_argv[0]);
>        security_init ();
>        need_security_init = false;
>  
> -      load_audit_modules (main_map);
> +      load_audit_modules (main_map, &audit_list);
>      }
>  
>    /* Keep track of the currently loaded modules to count how many
> @@ -2507,30 +2527,6 @@ a filename can be specified using the LD_DEBUG_OUTPUT environment variable.\n");
>      }
>  }
>  \f
> -static void
> -process_dl_audit (char *str)
> -{
> -  /* The parameter is a colon separated list of DSO names.  */
> -  char *p;
> -
> -  while ((p = (strsep) (&str, ":")) != NULL)
> -    if (dso_name_valid_for_suid (p))
> -      {
> -	/* This is using the local malloc, not the system malloc.  The
> -	   memory can never be freed.  */
> -	struct audit_list *newp = malloc (sizeof (*newp));
> -	newp->name = p;
> -
> -	if (audit_list == NULL)
> -	  audit_list = newp->next = newp;
> -	else
> -	  {
> -	    newp->next = audit_list->next;
> -	    audit_list = audit_list->next = newp;
> -	  }
> -      }
> -}
> -\f
>  /* Process all environments variables the dynamic linker must recognize.
>     Since all of them start with `LD_' we are a bit smarter while finding
>     all the entries.  */
> @@ -2538,7 +2534,7 @@ extern char **_environ attribute_hidden;
>  
>  
>  static void
> -process_envvars (enum mode *modep)
> +process_envvars (enum mode *modep, struct audit_list *audit_list)
>  {
>    char **runp = _environ;
>    char *envline;
> @@ -2578,7 +2574,7 @@ process_envvars (enum mode *modep)
>  	      break;
>  	    }
>  	  if (memcmp (envline, "AUDIT", 5) == 0)
> -	    audit_list_string = &envline[6];
> +	    audit_list_add_string (audit_list, &envline[6]);

OK.

>  	  break;
>  
>  	case 7:
> 


-- 
Cheers,
Carlos.

Re: [PATCH 3/3] elf: Implement DT_AUDIT, DT_DEPAUDIT support [BZ #24943]

[Carlos O'Donell]

On 4/3/20 8:03 AM, Florian Weimer via Libc-alpha wrote:
> binutils ld has supported --audit, --depaudit for a long time,
> only support in glibc has been missing.

OK for master.

Reviewed-by: Carlos O'Donell <carlos@redhat.com>

> ---
>  NEWS                    |  3 +++
>  elf/Makefile            | 22 +++++++++++++++--
>  elf/rtld.c              | 23 +++++++++++++++++-
>  elf/tst-audit14.c       | 46 +++++++++++++++++++++++++++++++++++
>  elf/tst-audit15.c       | 50 ++++++++++++++++++++++++++++++++++++++
>  elf/tst-audit16.c       | 54 +++++++++++++++++++++++++++++++++++++++++
>  elf/tst-auditlogmod-1.c | 27 +++++++++++++++++++++
>  elf/tst-auditlogmod-2.c | 27 +++++++++++++++++++++
>  elf/tst-auditlogmod-3.c | 27 +++++++++++++++++++++
>  9 files changed, 276 insertions(+), 3 deletions(-)
>  create mode 100644 elf/tst-audit14.c
>  create mode 100644 elf/tst-audit15.c
>  create mode 100644 elf/tst-audit16.c
>  create mode 100644 elf/tst-auditlogmod-1.c
>  create mode 100644 elf/tst-auditlogmod-2.c
>  create mode 100644 elf/tst-auditlogmod-3.c
> 
> diff --git a/NEWS b/NEWS
> index 466c203633..b5d7b34f8a 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -11,6 +11,9 @@ Major new features:
>  
>    * New locale added: ckb_IQ (Kurdish/Sorani spoken in Iraq)
>  
> +* The GNU C Library now loads audit modules listed in the DT_AUDIT and
> +  DT_DEPAUDIT dynamic section entries of the main executable.

OK.

> +
>  Deprecated and removed features, and other changes affecting compatibility:
>  
>    [Add deprecations, removals and changes affecting compatibility here]
> diff --git a/elf/Makefile b/elf/Makefile
> index da689a2c7b..9b1d58c7ad 100644
> --- a/elf/Makefile
> +++ b/elf/Makefile
> @@ -202,7 +202,8 @@ tests += restest1 preloadtest loadfail multiload origtest resolvfail \
>  	 tst-sonamemove-link tst-sonamemove-dlopen tst-dlopen-tlsmodid \
>  	 tst-dlopen-self tst-auditmany tst-initfinilazyfail tst-dlopenfail \
>  	 tst-dlopenfail-2 \
> -	 tst-filterobj tst-filterobj-dlopen tst-auxobj tst-auxobj-dlopen
> +	 tst-filterobj tst-filterobj-dlopen tst-auxobj tst-auxobj-dlopen \
> +	 tst-audit14 tst-audit15 tst-audit16
>  #	 reldep9
>  tests-internal += loadtest unload unload2 circleload1 \
>  	 neededtest neededtest2 neededtest3 neededtest4 \
> @@ -314,7 +315,8 @@ modules-names = testobj1 testobj2 testobj3 testobj4 testobj5 testobj6 \
>  		tst-initlazyfailmod tst-finilazyfailmod \
>  		tst-dlopenfailmod1 tst-dlopenfaillinkmod tst-dlopenfailmod2 \
>  		tst-dlopenfailmod3 tst-ldconfig-ld-mod \
> -		tst-filterobj-flt tst-filterobj-aux tst-filterobj-filtee
> +		tst-filterobj-flt tst-filterobj-aux tst-filterobj-filtee \
> +		tst-auditlogmod-1 tst-auditlogmod-2 tst-auditlogmod-3
>  # Most modules build with _ISOMAC defined, but those filtered out
>  # depend on internal headers.
>  modules-names-tests = $(filter-out ifuncmod% tst-libc_dlvsym-dso tst-tlsmod%,\
> @@ -1498,6 +1500,22 @@ $(objpfx)tst-auditmany.out: $(objpfx)tst-auditmanymod1.so \
>  tst-auditmany-ENV = \
>    LD_AUDIT=tst-auditmanymod1.so:tst-auditmanymod2.so:tst-auditmanymod3.so:tst-auditmanymod4.so:tst-auditmanymod5.so:tst-auditmanymod6.so:tst-auditmanymod7.so:tst-auditmanymod8.so:tst-auditmanymod9.so
>  
> +LDFLAGS-tst-audit14 = -Wl,--audit=tst-auditlogmod-1.so
> +$(objpfx)tst-auditlogmod-1.so: $(libsupport)
> +$(objpfx)tst-audit14.out: $(objpfx)tst-auditlogmod-1.so
> +LDFLAGS-tst-audit15 = \
> +  -Wl,--audit=tst-auditlogmod-1.so,--depaudit=tst-auditlogmod-2.so
> +$(objpfx)tst-auditlogmod-2.so: $(libsupport)
> +$(objpfx)tst-audit15.out: \
> +  $(objpfx)tst-auditlogmod-1.so $(objpfx)tst-auditlogmod-2.so
> +LDFLAGS-tst-audit16 = \
> +  -Wl,--audit=tst-auditlogmod-1.so:tst-auditlogmod-2.so \

OK. At least one test which exercises the splitting on delimiter.

> +  -Wl,--depaudit=tst-auditlogmod-3.so
> +$(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
> +
>  # 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/rtld.c b/elf/rtld.c
> index fe4dfbec67..b1938913a1 100644
> --- a/elf/rtld.c
> +++ b/elf/rtld.c
> @@ -153,9 +153,17 @@ static void audit_list_init (struct audit_list *);
>     not be called after audit_list_next.  */
>  static void audit_list_add_string (struct audit_list *, const char *);
>  
> +/* Add the audit strings from the link map, found in the dynamic
> +   segment at TG (either DT_AUDIT and DT_DEPAUDIT).  Must be called
> +   before audit_list_next.  */
> +static void audit_list_add_dynamic_tag (struct audit_list *,
> +					struct link_map *,
> +					unsigned int tag);

OK.

> +
>  /* Extract the next audit module from the audit list.  Only modules
>     for which dso_name_valid_for_suid is true are returned.  Must be
> -   called after all the audit_list_add_string calls.  */
> +   called after all the audit_list_add_string,
> +   audit_list_add_dynamic_tags calls.  */
>  static const char *audit_list_next (struct audit_list *);
>  
>  /* This is a list of all the modes the dynamic loader can be in.  */
> @@ -236,6 +244,16 @@ audit_list_add_string (struct audit_list *list, const char *string)
>      list->current_tail = string;
>  }
>  
> +static void
> +audit_list_add_dynamic_tag (struct audit_list *list, struct link_map *main_map,
> +			    unsigned int tag)
> +{
> +  ElfW(Dyn) *info = main_map->l_info[ADDRIDX (tag)];
> +  const char *strtab = (const char *) D_PTR (main_map, l_info[DT_STRTAB]);
> +  if (info != NULL)
> +    audit_list_add_string (list, strtab + info->d_un.d_val);
> +}

OK.

> +
>  static const char *
>  audit_list_next (struct audit_list *list)
>  {
> @@ -1638,6 +1656,9 @@ ERROR: '%s': cannot process note segment.\n", _dl_argv[0]);
>      /* Assign a module ID.  Do this before loading any audit modules.  */
>      GL(dl_rtld_map).l_tls_modid = _dl_next_tls_modid ();
>  
> +  audit_list_add_dynamic_tag (&audit_list, main_map, DT_AUDIT);
> +  audit_list_add_dynamic_tag (&audit_list, main_map, DT_DEPAUDIT);

OK. I like seeing good APIs that make adding functionality easy :-)

> +
>    /* If we have auditing DSOs to load, do it now.  */
>    bool need_security_init = true;
>    if (audit_list.length > 0)
> diff --git a/elf/tst-audit14.c b/elf/tst-audit14.c
> new file mode 100644
> index 0000000000..73e6634e35
> --- /dev/null
> +++ b/elf/tst-audit14.c
> @@ -0,0 +1,46 @@
> +/* Main program with DT_AUDIT.  One audit module.

OK.

> +   Copyright (C) 2020 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
> +   <http://www.gnu.org/licenses/>.  */
> +
> +#include <stdlib.h>
> +#include <string.h>
> +#include <support/check.h>
> +#include <support/xstdio.h>
> +
> +static int
> +do_test (void)
> +{
> +  /* Verify what the audit module has written.  This test assumes that
> +     standard output has been redirected to a regular file.  */
> +  FILE *fp = xfopen ("/dev/stdout", "r");
> +
> +  char *buffer = NULL;
> +  size_t buffer_length = 0;
> +  size_t line_length = xgetline (&buffer, &buffer_length, fp);
> +  const char *message = "info: tst-auditlogmod-1.so loaded\n";
> +  TEST_COMPARE_BLOB (message, strlen (message), buffer, line_length);
> +
> +  /* No more audit module output.  */
> +  line_length = xgetline (&buffer, &buffer_length, fp);
> +  TEST_COMPARE_BLOB ("", 0, buffer, line_length);
> +
> +  free (buffer);
> +  xfclose (fp);
> +  return 0;
> +}
> +
> +#include <support/test-driver.c>
> diff --git a/elf/tst-audit15.c b/elf/tst-audit15.c
> new file mode 100644
> index 0000000000..3d6a31c242
> --- /dev/null
> +++ b/elf/tst-audit15.c
> @@ -0,0 +1,50 @@
> +/* Main program with DT_AUDIT and DT_DEPAUDIT.  Two audit modules.

OK.

> +   Copyright (C) 2020 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
> +   <http://www.gnu.org/licenses/>.  */
> +
> +#include <stdlib.h>
> +#include <string.h>
> +#include <support/check.h>
> +#include <support/xstdio.h>
> +
> +static int
> +do_test (void)
> +{
> +  /* Verify what the audit modules have written.  This test assumes
> +     that standard output has been redirected to a regular file.  */
> +  FILE *fp = xfopen ("/dev/stdout", "r");
> +
> +  char *buffer = NULL;
> +  size_t buffer_length = 0;
> +  size_t line_length = xgetline (&buffer, &buffer_length, fp);
> +  const char *message = "info: tst-auditlogmod-1.so loaded\n";
> +  TEST_COMPARE_BLOB (message, strlen (message), buffer, line_length);
> +
> +  line_length = xgetline (&buffer, &buffer_length, fp);
> +  message = "info: tst-auditlogmod-2.so loaded\n";
> +  TEST_COMPARE_BLOB (message, strlen (message), buffer, line_length);
> +
> +  /* No more audit module output.  */
> +  line_length = xgetline (&buffer, &buffer_length, fp);
> +  TEST_COMPARE_BLOB ("", 0, buffer, line_length);
> +
> +  free (buffer);
> +  xfclose (fp);
> +  return 0;
> +}
> +
> +#include <support/test-driver.c>
> diff --git a/elf/tst-audit16.c b/elf/tst-audit16.c
> new file mode 100644
> index 0000000000..1a7d6eee5e
> --- /dev/null
> +++ b/elf/tst-audit16.c
> @@ -0,0 +1,54 @@
> +/* Main program with DT_AUDIT and DT_DEPAUDIT.  Three audit modules.

OK.

> +   Copyright (C) 2020 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
> +   <http://www.gnu.org/licenses/>.  */
> +
> +#include <stdlib.h>
> +#include <string.h>
> +#include <support/check.h>
> +#include <support/xstdio.h>
> +
> +static int
> +do_test (void)
> +{
> +  /* Verify what the audit modules have written.  This test assumes
> +     that standard output has been redirected to a regular file.  */
> +  FILE *fp = xfopen ("/dev/stdout", "r");
> +
> +  char *buffer = NULL;
> +  size_t buffer_length = 0;
> +  size_t line_length = xgetline (&buffer, &buffer_length, fp);
> +  const char *message = "info: tst-auditlogmod-1.so loaded\n";
> +  TEST_COMPARE_BLOB (message, strlen (message), buffer, line_length);
> +
> +  line_length = xgetline (&buffer, &buffer_length, fp);
> +  message = "info: tst-auditlogmod-2.so loaded\n";
> +  TEST_COMPARE_BLOB (message, strlen (message), buffer, line_length);
> +
> +  line_length = xgetline (&buffer, &buffer_length, fp);
> +  message = "info: tst-auditlogmod-3.so loaded\n";
> +  TEST_COMPARE_BLOB (message, strlen (message), buffer, line_length);
> +
> +  /* No more audit module output.  */
> +  line_length = xgetline (&buffer, &buffer_length, fp);
> +  TEST_COMPARE_BLOB ("", 0, buffer, line_length);
> +
> +  free (buffer);
> +  xfclose (fp);
> +  return 0;
> +}
> +
> +#include <support/test-driver.c>
> diff --git a/elf/tst-auditlogmod-1.c b/elf/tst-auditlogmod-1.c
> new file mode 100644
> index 0000000000..e6b8cd9094
> --- /dev/null
> +++ b/elf/tst-auditlogmod-1.c
> @@ -0,0 +1,27 @@
> +/* Audit module which logs that it was loaded.  Variant 1.
> +   Copyright (C) 2020 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
> +   <http://www.gnu.org/licenses/>.  */
> +
> +#include <link.h>
> +#include <support/support.h>
> +
> +unsigned int
> +la_version (unsigned int v)
> +{
> +  write_message ("info: tst-auditlogmod-1.so loaded\n");
> +  return LAV_CURRENT;
> +}
> diff --git a/elf/tst-auditlogmod-2.c b/elf/tst-auditlogmod-2.c
> new file mode 100644
> index 0000000000..9e7f0acabc
> --- /dev/null
> +++ b/elf/tst-auditlogmod-2.c
> @@ -0,0 +1,27 @@
> +/* Audit module which logs that it was loaded.  Variant 2.
> +   Copyright (C) 2020 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
> +   <http://www.gnu.org/licenses/>.  */
> +
> +#include <link.h>
> +#include <support/support.h>
> +
> +unsigned int
> +la_version (unsigned int v)
> +{
> +  write_message ("info: tst-auditlogmod-2.so loaded\n");
> +  return LAV_CURRENT;
> +}
> diff --git a/elf/tst-auditlogmod-3.c b/elf/tst-auditlogmod-3.c
> new file mode 100644
> index 0000000000..c4c1a58145
> --- /dev/null
> +++ b/elf/tst-auditlogmod-3.c
> @@ -0,0 +1,27 @@
> +/* Audit module which logs that it was loaded.  Variant 3.
> +   Copyright (C) 2020 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
> +   <http://www.gnu.org/licenses/>.  */
> +
> +#include <link.h>
> +#include <support/support.h>
> +
> +unsigned int
> +la_version (unsigned int v)
> +{
> +  write_message ("info: tst-auditlogmod-3.so loaded\n");
> +  return LAV_CURRENT;
> +}
> 


-- 
Cheers,
Carlos.

Re: [PATCH 2/3] elf: Simplify handling of lists of audit strings

[Florian Weimer]

* Carlos O'Donell:

> On 4/3/20 8:02 AM, Florian Weimer via Libc-alpha wrote:
>> All list elements are colon-separated strings, and there is a hard
>> upper limit for the number of audit modules, so it is possible to
>> pre-allocate a fixed-size array of strings to which the LD_AUDIT
>> environment variable and --audit arguments are added.
>> 
>> Also eliminate the global variables for the audit list because
>> the list is only needed briefly during startup.
>> 
>> There is a slight behavior change: All duplicate LD_AUDIT environment
>> variables are now processed, not just the last one as before.  However,
>> such environment vectors are invalid anyway.
>
> Agreed.
>
> OK for master.
>
> Reviewed-by: Carlos O'Donell <carlos@redhat.com>

Thanks.

>> +  /* Grow the array if necessary.  */
>> +  if (list->length == array_length (list->audit_strings))
>> +    _dl_fatal_printf ("Too many audit modules requested\n");

I'm going to change this to:

  Fatal glibc error: Too many audit modules requested

Thanks,
Florian

Re: [PATCH 2/3] elf: Simplify handling of lists of audit strings

[Carlos O'Donell]

On 4/3/20 8:58 AM, Florian Weimer wrote:
> * Carlos O'Donell:
> 
>> On 4/3/20 8:02 AM, Florian Weimer via Libc-alpha wrote:
>>> All list elements are colon-separated strings, and there is a hard
>>> upper limit for the number of audit modules, so it is possible to
>>> pre-allocate a fixed-size array of strings to which the LD_AUDIT
>>> environment variable and --audit arguments are added.
>>>
>>> Also eliminate the global variables for the audit list because
>>> the list is only needed briefly during startup.
>>>
>>> There is a slight behavior change: All duplicate LD_AUDIT environment
>>> variables are now processed, not just the last one as before.  However,
>>> such environment vectors are invalid anyway.
>>
>> Agreed.
>>
>> OK for master.
>>
>> Reviewed-by: Carlos O'Donell <carlos@redhat.com>
> 
> Thanks.
> 
>>> +  /* Grow the array if necessary.  */
>>> +  if (list->length == array_length (list->audit_strings))
>>> +    _dl_fatal_printf ("Too many audit modules requested\n");
> 
> I'm going to change this to:
> 
>   Fatal glibc error: Too many audit modules requested

That seems fine to me.

-- 
Cheers,
Carlos.

Re: [PATCH 2/3] elf: Simplify handling of lists of audit strings

[Andreas Schwab]

On Apr 03 2020, Florian Weimer via Libc-alpha wrote:

> +  /* Grow the array if necessary.  */
> +  if (list->length == array_length (list->audit_strings))
> +    _dl_fatal_printf ("Too many audit modules requested\n");

That comment doesn't make sense.  You are definitely not growing
anything here.

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

Re: [PATCH 2/3] elf: Simplify handling of lists of audit strings

[Florian Weimer]

* Andreas Schwab:

> On Apr 03 2020, Florian Weimer via Libc-alpha wrote:
>
>> +  /* Grow the array if necessary.  */
>> +  if (list->length == array_length (list->audit_strings))
>> +    _dl_fatal_printf ("Too many audit modules requested\n");
>
> That comment doesn't make sense.  You are definitely not growing
> anything here.

Right, it's a leftover from previous versions.  I will fix it.  Thanks.

Florian