rtld: Reject overly long LD_AUDIT path elements

rtld: Simplify audit module processing

2017-06-26  Florian Weimer  <fweimer@redhat.com>

	* elf/rtld.c (struct audit_list_iter, audit_list_iter_init)
	(audit_list_iter_next): Remove.
	(handle_one_audit_module): New function, extracted from dl_main.
	(handle_audit_modules): New function.
	(dl_main): Call handle_audit_modules to process audit modules.

diff --git a/elf/rtld.c b/elf/rtld.c
index 65647fb..e91dcd8 100644
--- a/elf/rtld.c
+++ b/elf/rtld.c
@@ -141,79 +141,6 @@  static struct audit_list
   struct audit_list *next;
 } *audit_list;
 
-/* Iterator for audit_list_string followed by audit_list.  */
-struct audit_list_iter
-{
-  /* Tail of audit_list_string still needing processing, or NULL.  */
-  const char *audit_list_tail;
-
-  /* The list element returned in the previous iteration.  NULL before
-     the first element.  */
-  struct audit_list *previous;
-
-  /* Scratch buffer for returning a name which is part of
-     audit_list_string.  */
-  char fname[SECURE_NAME_LIMIT];
-};
-
-/* 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;
-}
-
-/* Iterate through both audit_list_string and audit_list.  */
-static const char *
-audit_list_iter_next (struct audit_list_iter *iter)
-{
-  if (iter->audit_list_tail != NULL)
-    {
-      /* First iterate over audit_list_string.  */
-      while (*iter->audit_list_tail != '\0')
-	{
-	  /* Split audit list at colon.  */
-	  size_t len = strcspn (iter->audit_list_tail, ":");
-	  if (len > 0 && len < sizeof (iter->fname))
-	    {
-	      memcpy (iter->fname, iter->audit_list_tail, len);
-	      iter->fname[len] = '\0';
-	    }
-	  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.  */
-	}
-      /* 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;
-    }
-  if (iter->previous == audit_list)
-    /* Cyclic list wrap-around.  */
-    return NULL;
-  iter->previous = iter->previous->next;
-  return iter->previous->name;
-}
-
 #ifndef HAVE_INLINED_SYSCALLS
 /* Set nonzero during loading and initialization of executable and
    libraries, cleared before the executable's entry point runs.  This
@@ -861,6 +788,181 @@  handle_ld_preload (const char *preloadlist, struct link_map *main_map)
   return npreloads;
 }
 
+/* Load a single audit module.  */
+void
+handle_one_audit_module (const char *name, struct audit_ifaces **last_audit)
+{
+  int tls_idx = GL(dl_tls_max_dtv_idx);
+
+  /* Now it is time to determine the layout of the static TLS
+     block and allocate it for the initial thread.  Note that we
+     always allocate the static block, we never defer it even if
+     no DF_STATIC_TLS bit is set.  The reason is that we know
+     glibc will use the static model.  */
+  struct dlmopen_args dlmargs;
+  dlmargs.fname = name;
+  dlmargs.map = NULL;
+
+  const char *objname;
+  const char *err_str = NULL;
+  bool malloced;
+  (void) _dl_catch_error (&objname, &err_str, &malloced, dlmopen_doit,
+			  &dlmargs);
+  if (__glibc_unlikely (err_str != NULL))
+    {
+    not_loaded:
+      _dl_error_printf ("\
+ERROR: ld.so: object '%s' cannot be loaded as audit interface: %s; ignored.\n",
+			name, err_str);
+      if (malloced)
+	free ((char *) err_str);
+    }
+  else
+    {
+      struct lookup_args largs;
+      largs.name = "la_version";
+      largs.map = dlmargs.map;
+
+      /* Check whether the interface version matches.  */
+      (void) _dl_catch_error (&objname, &err_str, &malloced,
+			      lookup_doit, &largs);
+
+      unsigned int (*laversion) (unsigned int);
+      unsigned int lav;
+      if  (err_str == NULL
+	   && (laversion = largs.result) != NULL
+	   && (lav = laversion (LAV_CURRENT)) > 0
+	   && lav <= LAV_CURRENT)
+	{
+	  /* Allocate structure for the callback function pointers.
+	     This call can never fail.  */
+	  union
+	  {
+	    struct audit_ifaces ifaces;
+#define naudit_ifaces 8
+	    void (*fptr[naudit_ifaces]) (void);
+	  } *newp = malloc (sizeof (*newp));
+
+	  /* Names of the auditing interfaces.  All in one
+	     long string.  */
+	  static const char audit_iface_names[] =
+	    "la_activity\0"
+	    "la_objsearch\0"
+	    "la_objopen\0"
+	    "la_preinit\0"
+#if __ELF_NATIVE_CLASS == 32
+	    "la_symbind32\0"
+#elif __ELF_NATIVE_CLASS == 64
+	    "la_symbind64\0"
+#else
+# error "__ELF_NATIVE_CLASS must be defined"
+#endif
+#define STRING(s) __STRING (s)
+	    "la_" STRING (ARCH_LA_PLTENTER) "\0"
+	    "la_" STRING (ARCH_LA_PLTEXIT) "\0"
+	    "la_objclose\0";
+	  unsigned int cnt = 0;
+	  const char *cp = audit_iface_names;
+	  do
+	    {
+	      largs.name = cp;
+	      (void) _dl_catch_error (&objname, &err_str, &malloced,
+				      lookup_doit, &largs);
+
+	      /* Store the pointer.  */
+	      if (err_str == NULL && largs.result != NULL)
+		{
+		  newp->fptr[cnt] = largs.result;
+
+		  /* The dynamic linker link map is statically
+		     allocated, initialize the data now.   */
+		  GL(dl_rtld_map).l_audit[cnt].cookie
+		    = (intptr_t) &GL(dl_rtld_map);
+		}
+	      else
+		newp->fptr[cnt] = NULL;
+	      ++cnt;
+
+	      cp = (char *) rawmemchr (cp, '\0') + 1;
+	    }
+	  while (*cp != '\0');
+	  assert (cnt == naudit_ifaces);
+
+	  /* Now append the new auditing interface to the list.  */
+	  newp->ifaces.next = NULL;
+	  if (*last_audit == NULL)
+	    *last_audit = GLRO(dl_audit) = &newp->ifaces;
+	  else
+	    *last_audit = (*last_audit)->next = &newp->ifaces;
+	  ++GLRO(dl_naudit);
+
+	  /* Mark the DSO as being used for auditing.  */
+	  dlmargs.map->l_auditing = 1;
+	}
+      else
+	{
+	  /* We cannot use the DSO, it does not have the
+	     appropriate interfaces or it expects something
+	     more recent.  */
+#ifndef NDEBUG
+	  Lmid_t ns = dlmargs.map->l_ns;
+#endif
+	  _dl_close (dlmargs.map);
+
+	  /* Make sure the namespace has been cleared entirely.  */
+	  assert (GL(dl_ns)[ns]._ns_loaded == NULL);
+	  assert (GL(dl_ns)[ns]._ns_nloaded == 0);
+
+	  GL(dl_tls_max_dtv_idx) = tls_idx;
+	  goto not_loaded;
+	}
+    }
+}
+
+/* Process the audit modules in audit_list and audit_list_string.  */
+void
+handle_audit_modules (void)
+{
+  char fname[SECURE_PATH_LIMIT];
+  struct audit_ifaces *last_audit = NULL;
+
+  if (audit_list_string != NULL)
+    {
+      const char *p = audit_list_string;
+      while (*p != '\0')
+	{
+	  /* Split preload list at space/colon.  */
+	  size_t len = strcspn (p, ":");
+	  if (len > 0 && len < sizeof (fname))
+	    {
+	      memcpy (fname, p, len);
+	      fname[len] = '\0';
+	    }
+	  else
+	    fname[0] = '\0';
+
+	  /* Skip over the substring and the following delimiter.  */
+	  p += len;
+	  if (*p != '\0')
+	    ++p;
+
+	  if (dso_name_valid_for_suid (fname))
+	    handle_one_audit_module (fname, &last_audit);
+	}
+    }
+
+  if (audit_list != NULL)
+    {
+      struct audit_list *al = audit_list->next;
+      do
+	{
+	  handle_one_audit_module (al->name, &last_audit);
+	  al = al->next;
+	}
+      while (al != audit_list->next);
+    }
+}
+
 static void
 dl_main (const ElfW(Phdr) *phdr,
 	 ElfW(Word) phnum,
@@ -1387,10 +1489,6 @@  of this helper program; chances are you did not intend to run this program.\n\
   if (__glibc_unlikely (audit_list != NULL)
       || __glibc_unlikely (audit_list_string != NULL))
     {
-      struct audit_ifaces *last_audit = NULL;
-      struct audit_list_iter al_iter;
-      audit_list_iter_init (&al_iter);
-
       /* Since we start using the auditing DSOs right away we need to
 	 initialize the data structures now.  */
       tcbp = init_tls ();
@@ -1402,138 +1500,7 @@  of this helper program; chances are you did not intend to run this program.\n\
       security_init ();
       need_security_init = false;
 
-      while (true)
-	{
-	  const char *name = audit_list_iter_next (&al_iter);
-	  if (name == NULL)
-	    break;
-
-	  int tls_idx = GL(dl_tls_max_dtv_idx);
-
-	  /* Now it is time to determine the layout of the static TLS
-	     block and allocate it for the initial thread.  Note that we
-	     always allocate the static block, we never defer it even if
-	     no DF_STATIC_TLS bit is set.  The reason is that we know
-	     glibc will use the static model.  */
-	  struct dlmopen_args dlmargs;
-	  dlmargs.fname = name;
-	  dlmargs.map = NULL;
-
-	  const char *objname;
-	  const char *err_str = NULL;
-	  bool malloced;
-	  (void) _dl_catch_error (&objname, &err_str, &malloced, dlmopen_doit,
-				  &dlmargs);
-	  if (__glibc_unlikely (err_str != NULL))
-	    {
-	    not_loaded:
-	      _dl_error_printf ("\
-ERROR: ld.so: object '%s' cannot be loaded as audit interface: %s; ignored.\n",
-				name, err_str);
-	      if (malloced)
-		free ((char *) err_str);
-	    }
-	  else
-	    {
-	      struct lookup_args largs;
-	      largs.name = "la_version";
-	      largs.map = dlmargs.map;
-
-	      /* Check whether the interface version matches.  */
-	      (void) _dl_catch_error (&objname, &err_str, &malloced,
-				      lookup_doit, &largs);
-
-	      unsigned int (*laversion) (unsigned int);
-	      unsigned int lav;
-	      if  (err_str == NULL
-		   && (laversion = largs.result) != NULL
-		   && (lav = laversion (LAV_CURRENT)) > 0
-		   && lav <= LAV_CURRENT)
-		{
-		  /* Allocate structure for the callback function pointers.
-		     This call can never fail.  */
-		  union
-		  {
-		    struct audit_ifaces ifaces;
-#define naudit_ifaces 8
-		    void (*fptr[naudit_ifaces]) (void);
-		  } *newp = malloc (sizeof (*newp));
-
-		  /* Names of the auditing interfaces.  All in one
-		     long string.  */
-		  static const char audit_iface_names[] =
-		    "la_activity\0"
-		    "la_objsearch\0"
-		    "la_objopen\0"
-		    "la_preinit\0"
-#if __ELF_NATIVE_CLASS == 32
-		    "la_symbind32\0"
-#elif __ELF_NATIVE_CLASS == 64
-		    "la_symbind64\0"
-#else
-# error "__ELF_NATIVE_CLASS must be defined"
-#endif
-#define STRING(s) __STRING (s)
-		    "la_" STRING (ARCH_LA_PLTENTER) "\0"
-		    "la_" STRING (ARCH_LA_PLTEXIT) "\0"
-		    "la_objclose\0";
-		  unsigned int cnt = 0;
-		  const char *cp = audit_iface_names;
-		  do
-		    {
-		      largs.name = cp;
-		      (void) _dl_catch_error (&objname, &err_str, &malloced,
-					      lookup_doit, &largs);
-
-		      /* Store the pointer.  */
-		      if (err_str == NULL && largs.result != NULL)
-			{
-			  newp->fptr[cnt] = largs.result;
-
-			  /* The dynamic linker link map is statically
-			     allocated, initialize the data now.   */
-			  GL(dl_rtld_map).l_audit[cnt].cookie
-			    = (intptr_t) &GL(dl_rtld_map);
-			}
-		      else
-			newp->fptr[cnt] = NULL;
-		      ++cnt;
-
-		      cp = (char *) rawmemchr (cp, '\0') + 1;
-		    }
-		  while (*cp != '\0');
-		  assert (cnt == naudit_ifaces);
-
-		  /* Now append the new auditing interface to the list.  */
-		  newp->ifaces.next = NULL;
-		  if (last_audit == NULL)
-		    last_audit = GLRO(dl_audit) = &newp->ifaces;
-		  else
-		    last_audit = last_audit->next = &newp->ifaces;
-		  ++GLRO(dl_naudit);
-
-		  /* Mark the DSO as being used for auditing.  */
-		  dlmargs.map->l_auditing = 1;
-		}
-	      else
-		{
-		  /* We cannot use the DSO, it does not have the
-		     appropriate interfaces or it expects something
-		     more recent.  */
-#ifndef NDEBUG
-		  Lmid_t ns = dlmargs.map->l_ns;
-#endif
-		  _dl_close (dlmargs.map);
-
-		  /* Make sure the namespace has been cleared entirely.  */
-		  assert (GL(dl_ns)[ns]._ns_loaded == NULL);
-		  assert (GL(dl_ns)[ns]._ns_nloaded == 0);
-
-		  GL(dl_tls_max_dtv_idx) = tls_idx;
-		  goto not_loaded;
-		}
-	    }
-	}
+      handle_audit_modules ();
 
       /* If we have any auditing modules, announce that we already
 	 have two objects loaded.  */