elf: Ignore LD_AUDIT interfaces if la_version returns 0 [BZ #24122]

Message ID 87imy2w6om.fsf@oldenburg2.str.redhat.com
State New
Headers show
Series
  • elf: Ignore LD_AUDIT interfaces if la_version returns 0 [BZ #24122]
Related show

Commit Message

Florian Weimer Feb. 2, 2019, 3:28 p.m.
This is a repost of the early patch for glibc 2.30.

I updated some comments, replaced one “unsigned” with “unsigned int”,
and introduced the early_audit_modules_notification function.

Thanks,
Florian

-----
This change moves the audit module loading and early notification into
separate functions out of dl_main.

It restores the bug fix from commit
8e889c5da3c5981c5a46a93fec02de40131ac5a6  ("elf: Fix LD_AUDIT for
modules with invalid version (BZ#24122)") which was reverted in commit
83e6b59625f45db1eee93e5684091f740c52a083  ("[elf] Revert 8e889c5da3
(BZ#24122)").

The actual bug fix is the separate error message for the case when
la_version is zero.  The dynamic linker error message (which is NULL
in this case) is no longer used.  Based on the intended use of version
zero (ignore this module due to explicit request), the message is only
printed if debugging is enabled.

2019-02-02  Florian Weimer  <fweimer@redhat.com>

	[BZ #24122]
	* elf/rtld.c (unload_audit_module): New function.
	(report_audit_module_load_error): Likewise.
	(load_audit_module): Likewise.  Extracted from dl_main.  Call
	_dl_close if the laversion symbol cannot be found.  Use early
	returns for error handling.  Check for a zero return value from
	la_version.  Remove spurious comment about static TLS
	initialization.  Remove (void) casts of function results.
	(load_audit_modules): New function.  Extracted from dl_main.
	(early_audit_modules_notification): Likewise.
	(dl_main): Call load_audit_modules and
	early_audit_modules_notification.

Comments

Adhemerval Zanella Feb. 4, 2019, 1:16 p.m. | #1
On 02/02/2019 13:28, Florian Weimer wrote:
> This is a repost of the early patch for glibc 2.30.
> 
> I updated some comments, replaced one “unsigned” with “unsigned int”,
> and introduced the early_audit_modules_notification function.
> 
> Thanks,
> Florian
> 
> -----
> This change moves the audit module loading and early notification into
> separate functions out of dl_main.
> 
> It restores the bug fix from commit
> 8e889c5da3c5981c5a46a93fec02de40131ac5a6  ("elf: Fix LD_AUDIT for
> modules with invalid version (BZ#24122)") which was reverted in commit
> 83e6b59625f45db1eee93e5684091f740c52a083  ("[elf] Revert 8e889c5da3
> (BZ#24122)").
> 
> The actual bug fix is the separate error message for the case when
> la_version is zero.  The dynamic linker error message (which is NULL
> in this case) is no longer used.  Based on the intended use of version
> zero (ignore this module due to explicit request), the message is only
> printed if debugging is enabled.
> 
> 2019-02-02  Florian Weimer  <fweimer@redhat.com>
> 
> 	[BZ #24122]
> 	* elf/rtld.c (unload_audit_module): New function.
> 	(report_audit_module_load_error): Likewise.
> 	(load_audit_module): Likewise.  Extracted from dl_main.  Call
> 	_dl_close if the laversion symbol cannot be found.  Use early
> 	returns for error handling.  Check for a zero return value from
> 	la_version.  Remove spurious comment about static TLS
> 	initialization.  Remove (void) casts of function results.
> 	(load_audit_modules): New function.  Extracted from dl_main.
> 	(early_audit_modules_notification): Likewise.
> 	(dl_main): Call load_audit_modules and
> 	early_audit_modules_notification.

I think we should reinstate the testcase from original commit 
8e889c5da3c5981c5a46a93fec02de40131ac5a6 along with this change. It aligns
with our policy of fixing bug reports with a regression test.

> 
> diff --git a/elf/rtld.c b/elf/rtld.c
> index 5d97f41b7b..3babe0185b 100644
> --- a/elf/rtld.c
> +++ b/elf/rtld.c
> @@ -863,6 +863,205 @@ handle_ld_preload (const char *preloadlist, struct link_map *main_map)
>    return npreloads;
>  }
>  
> +/* Called if the audit DSO cannot be used: if it does not have the
> +   appropriate interfaces, or it expects a more recent version library
> +   version than what the dynamic linker provides.  */
> +static void
> +unload_audit_module (struct link_map *map, int original_tls_idx)
> +{
> +#ifndef NDEBUG
> +  Lmid_t ns = map->l_ns;
> +#endif
> +  _dl_close (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) = original_tls_idx;
> +}

Ok.

> +
> +/* Called to print an error message if loading of an audit module
> +   failed.  */
> +static void
> +report_audit_module_load_error (const char *name, const char *err_str,
> +				bool malloced)
> +{
> +  _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);
> +}

Ok.

> +
> +/* Load one audit module.  */
> +static void
> +load_audit_module (const char *name, struct audit_ifaces **last_audit)
> +{
> +  int original_tls_idx = GL(dl_tls_max_dtv_idx);
> +
> +  struct dlmopen_args dlmargs;
> +  dlmargs.fname = name;
> +  dlmargs.map = NULL;

Maybe a simple initialization as 'struct dlmopen_args dlmargs = { name, NULL }'?

> +
> +  const char *objname;
> +  const char *err_str = NULL;
> +  bool malloced;
> +  _dl_catch_error (&objname, &err_str, &malloced, dlmopen_doit, &dlmargs);
> +  if (__glibc_unlikely (err_str != NULL))
> +    {
> +      report_audit_module_load_error (name, err_str, malloced);
> +      return;
> +    }
> +
> +  struct lookup_args largs;
> +  largs.name = "la_version";
> +  largs.map = dlmargs.map;
> +  _dl_catch_error (&objname, &err_str, &malloced, lookup_doit, &largs);
> +  if (__glibc_likely (err_str != NULL))
> +    {
> +      unload_audit_module (dlmargs.map, original_tls_idx);
> +      report_audit_module_load_error (name, err_str, malloced);
> +      return;
> +    }
> +
> +  unsigned int (*laversion) (unsigned int) = largs.result;
> +  assert (laversion != NULL);

I think we need to add a comment why we assert on a NULL 'laversion'
symbol, maybe by adding this means a corrupted audit object.

> +  unsigned int lav = laversion (LAV_CURRENT);
> +  if (lav == 0)

Maybe __glibc_unlikely here?

> +    {
> +      /* Only print an error message if debugging because this can
> +	 happen deliberately.  */
> +      if (GLRO(dl_debug_mask) & DL_DEBUG_FILES)
> +	_dl_debug_printf ("\
> +file=%s [%lu]; audit interface function la_version returned zero; ignored.\n",
> +			  dlmargs.map->l_name, dlmargs.map->l_ns);
> +      unload_audit_module (dlmargs.map, original_tls_idx);
> +      return;
> +    }
> +
> +  if (lav > LAV_CURRENT)
> +    {
> +      _dl_debug_printf ("\
> +ERROR: audit interface '%s' requires version %d (maximum supported version %d); ignored.\n",
> +			name, lav, LAV_CURRENT);
> +      unload_audit_module (dlmargs.map, original_tls_idx);
> +      return;
> +    }
> +
> +  /* Allocate structure for the callback function pointers.  This call
> +     can never fail.  */
> +  enum { naudit_ifaces = 8 };
> +  union
> +  {
> +    struct audit_ifaces ifaces;
> +    void (*fptr[naudit_ifaces]) (void);
> +  } *newp = malloc (sizeof (*newp));

If this call should never fail, should we also assert for the malloc ?

> +
> +  /* 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);

Do we need the cast here?

> +
> +      /* 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;

Same as before.

> +    }
> +  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;
> +}
> +
> +/* Load all audit modules.  */
> +static void
> +load_audit_modules (void)
> +{
> +  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);
> +      if (name == NULL)
> +	break;
> +      load_audit_module (name, &last_audit);
> +    }
> +}
> +
> +/* If we have any auditing modules, announce that we already have two
> +   objects loaded: the main program and the dynamic linker itself.  */
> +static void
> +early_audit_modules_notification (struct link_map *main_map)
> +{
> +  if (__glibc_likely (GLRO(dl_naudit) == 0))
> +    return;
> +
> +  struct link_map *ls[2] = { main_map, &GL(dl_rtld_map) };
> +
> +  for (unsigned int outer = 0; outer < 2; ++outer)

Maybe size_t here?

> +    {
> +      struct audit_ifaces *afct = GLRO(dl_audit);
> +      for (unsigned int cnt = 0; cnt < GLRO(dl_naudit); ++cnt)
> +	{
> +	  if (afct->objopen != NULL)
> +	    {
> +	      ls[outer]->l_audit[cnt].bindflags
> +		= afct->objopen (ls[outer], LM_ID_BASE,
> +				 &ls[outer]->l_audit[cnt].cookie);
> +
> +	      ls[outer]->l_audit_any_plt
> +		|= ls[outer]->l_audit[cnt].bindflags != 0;
> +	    }
> +
> +	  afct = afct->next;
> +	}
> +    }
> +}
> +

Ok.

>  static void
>  dl_main (const ElfW(Phdr) *phdr,
>  	 ElfW(Word) phnum,
> @@ -1395,10 +1594,6 @@ ERROR: '%s': cannot process note segment.\n", _dl_argv[0]);
>    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 ();
> @@ -1410,164 +1605,8 @@ ERROR: '%s': cannot process note segment.\n", _dl_argv[0]);
>        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;
> -		}
> -	    }
> -	}
> -
> -      /* If we have any auditing modules, announce that we already
> -	 have two objects loaded.  */
> -      if (__glibc_unlikely (GLRO(dl_naudit) > 0))
> -	{
> -	  struct link_map *ls[2] = { main_map, &GL(dl_rtld_map) };
> -
> -	  for (unsigned int outer = 0; outer < 2; ++outer)
> -	    {
> -	      struct audit_ifaces *afct = GLRO(dl_audit);
> -	      for (unsigned int cnt = 0; cnt < GLRO(dl_naudit); ++cnt)
> -		{
> -		  if (afct->objopen != NULL)
> -		    {
> -		      ls[outer]->l_audit[cnt].bindflags
> -			= afct->objopen (ls[outer], LM_ID_BASE,
> -					 &ls[outer]->l_audit[cnt].cookie);
> -
> -		      ls[outer]->l_audit_any_plt
> -			|= ls[outer]->l_audit[cnt].bindflags != 0;
> -		    }
> -
> -		  afct = afct->next;
> -		}
> -	    }
> -	}
> +      load_audit_modules ();
> +      early_audit_modules_notification (main_map);
>      }
>  
>    /* Keep track of the currently loaded modules to count how many
> 


Ok.
Florian Weimer Feb. 4, 2019, 1:51 p.m. | #2
* Adhemerval Zanella:

>> The actual bug fix is the separate error message for the case when
>> la_version is zero.  The dynamic linker error message (which is NULL
>> in this case) is no longer used.  Based on the intended use of version
>> zero (ignore this module due to explicit request), the message is only
>> printed if debugging is enabled.
>> 
>> 2019-02-02  Florian Weimer  <fweimer@redhat.com>
>> 
>> 	[BZ #24122]
>> 	* elf/rtld.c (unload_audit_module): New function.
>> 	(report_audit_module_load_error): Likewise.
>> 	(load_audit_module): Likewise.  Extracted from dl_main.  Call
>> 	_dl_close if the laversion symbol cannot be found.  Use early
>> 	returns for error handling.  Check for a zero return value from
>> 	la_version.  Remove spurious comment about static TLS
>> 	initialization.  Remove (void) casts of function results.
>> 	(load_audit_modules): New function.  Extracted from dl_main.
>> 	(early_audit_modules_notification): Likewise.
>> 	(dl_main): Call load_audit_modules and
>> 	early_audit_modules_notification.
>
> I think we should reinstate the testcase from original commit 
> 8e889c5da3c5981c5a46a93fec02de40131ac5a6 along with this change. It aligns
> with our policy of fixing bug reports with a regression test.

Absolutely.  I already posted that as a separate patch.  I want to
retain your authorship, and since it is a completely separate change, a
separate commit seems reasonable to me.

>> +/* Load one audit module.  */
>> +static void
>> +load_audit_module (const char *name, struct audit_ifaces **last_audit)
>> +{
>> +  int original_tls_idx = GL(dl_tls_max_dtv_idx);
>> +
>> +  struct dlmopen_args dlmargs;
>> +  dlmargs.fname = name;
>> +  dlmargs.map = NULL;
>
> Maybe a simple initialization as 'struct dlmopen_args dlmargs = { name, NULL }'?

It's what was there before. 8-/

>> +  const char *objname;
>> +  const char *err_str = NULL;
>> +  bool malloced;
>> +  _dl_catch_error (&objname, &err_str, &malloced, dlmopen_doit, &dlmargs);
>> +  if (__glibc_unlikely (err_str != NULL))
>> +    {
>> +      report_audit_module_load_error (name, err_str, malloced);
>> +      return;
>> +    }
>> +
>> +  struct lookup_args largs;
>> +  largs.name = "la_version";
>> +  largs.map = dlmargs.map;
>> +  _dl_catch_error (&objname, &err_str, &malloced, lookup_doit, &largs);
>> +  if (__glibc_likely (err_str != NULL))
>> +    {
>> +      unload_audit_module (dlmargs.map, original_tls_idx);
>> +      report_audit_module_load_error (name, err_str, malloced);
>> +      return;
>> +    }
>> +
>> +  unsigned int (*laversion) (unsigned int) = largs.result;
>> +  assert (laversion != NULL);
>
> I think we need to add a comment why we assert on a NULL 'laversion'
> symbol, maybe by adding this means a corrupted audit object.

I can add:

/* A null symbol indicates that something is very wrong with the loaded
   object because defined symbols are supposed to have a valid, non-null
   address.  */

>> +  unsigned int lav = laversion (LAV_CURRENT);
>> +  if (lav == 0)
>
> Maybe __glibc_unlikely here?

We don't really know what's unlikely or not.  Since audit modules are
used only rarely, I don't want to worry about it.


>> +  /* Allocate structure for the callback function pointers.  This call
>> +     can never fail.  */
>> +  enum { naudit_ifaces = 8 };
>> +  union
>> +  {
>> +    struct audit_ifaces ifaces;
>> +    void (*fptr[naudit_ifaces]) (void);
>> +  } *newp = malloc (sizeof (*newp));
>
> If this call should never fail, should we also assert for the malloc ?

I think we use _dl_fatal_printf elsewhere.  Again, this is a
pre-existing issue. 8-/

>
>> +
>> +  /* 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);
>
> Do we need the cast here?

I think we should switch to the new error handling framework
(_dl_catch_exception) and not worry about it until then.

>> +
>> +      /* 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;
>
> Same as before.

Yeah, an unrelated, pre-existing issue.

>> +/* If we have any auditing modules, announce that we already have two
>> +   objects loaded: the main program and the dynamic linker itself.  */
>> +static void
>> +early_audit_modules_notification (struct link_map *main_map)
>> +{
>> +  if (__glibc_likely (GLRO(dl_naudit) == 0))
>> +    return;
>> +
>> +  struct link_map *ls[2] = { main_map, &GL(dl_rtld_map) };
>> +
>> +  for (unsigned int outer = 0; outer < 2; ++outer)
>
> Maybe size_t here?

Despite outer < 2?

I can unroll the loop in the caller, like this:

  if (__glibc_likely (GLRO(dl_naudit) > 0)
    {
      early_audit_modules_notification (main_map);
      early_audit_modules_notification (&GL(dl_rtld_map));
    }

That actually looks quite reasonable.  What do you think?

Thanks,
Florian
Adhemerval Zanella Feb. 4, 2019, 2:29 p.m. | #3
On 04/02/2019 11:51, Florian Weimer wrote:
> * Adhemerval Zanella:
> 
>>> The actual bug fix is the separate error message for the case when
>>> la_version is zero.  The dynamic linker error message (which is NULL
>>> in this case) is no longer used.  Based on the intended use of version
>>> zero (ignore this module due to explicit request), the message is only
>>> printed if debugging is enabled.
>>>
>>> 2019-02-02  Florian Weimer  <fweimer@redhat.com>
>>>
>>> 	[BZ #24122]
>>> 	* elf/rtld.c (unload_audit_module): New function.
>>> 	(report_audit_module_load_error): Likewise.
>>> 	(load_audit_module): Likewise.  Extracted from dl_main.  Call
>>> 	_dl_close if the laversion symbol cannot be found.  Use early
>>> 	returns for error handling.  Check for a zero return value from
>>> 	la_version.  Remove spurious comment about static TLS
>>> 	initialization.  Remove (void) casts of function results.
>>> 	(load_audit_modules): New function.  Extracted from dl_main.
>>> 	(early_audit_modules_notification): Likewise.
>>> 	(dl_main): Call load_audit_modules and
>>> 	early_audit_modules_notification.
>>
>> I think we should reinstate the testcase from original commit 
>> 8e889c5da3c5981c5a46a93fec02de40131ac5a6 along with this change. It aligns
>> with our policy of fixing bug reports with a regression test.
> 
> Absolutely.  I already posted that as a separate patch.  I want to
> retain your authorship, and since it is a completely separate change, a
> separate commit seems reasonable to me.

I don't mind, just add me on CL and put reviewed-by: on commit message.

> 
>>> +/* Load one audit module.  */
>>> +static void
>>> +load_audit_module (const char *name, struct audit_ifaces **last_audit)
>>> +{
>>> +  int original_tls_idx = GL(dl_tls_max_dtv_idx);
>>> +
>>> +  struct dlmopen_args dlmargs;
>>> +  dlmargs.fname = name;
>>> +  dlmargs.map = NULL;
>>
>> Maybe a simple initialization as 'struct dlmopen_args dlmargs = { name, NULL }'?
> 
> It's what was there before. 8-/
> 
>>> +  const char *objname;
>>> +  const char *err_str = NULL;
>>> +  bool malloced;
>>> +  _dl_catch_error (&objname, &err_str, &malloced, dlmopen_doit, &dlmargs);
>>> +  if (__glibc_unlikely (err_str != NULL))
>>> +    {
>>> +      report_audit_module_load_error (name, err_str, malloced);
>>> +      return;
>>> +    }
>>> +
>>> +  struct lookup_args largs;
>>> +  largs.name = "la_version";
>>> +  largs.map = dlmargs.map;
>>> +  _dl_catch_error (&objname, &err_str, &malloced, lookup_doit, &largs);
>>> +  if (__glibc_likely (err_str != NULL))
>>> +    {
>>> +      unload_audit_module (dlmargs.map, original_tls_idx);
>>> +      report_audit_module_load_error (name, err_str, malloced);
>>> +      return;
>>> +    }
>>> +
>>> +  unsigned int (*laversion) (unsigned int) = largs.result;
>>> +  assert (laversion != NULL);
>>
>> I think we need to add a comment why we assert on a NULL 'laversion'
>> symbol, maybe by adding this means a corrupted audit object.
> 
> I can add:
> 
> /* A null symbol indicates that something is very wrong with the loaded
>    object because defined symbols are supposed to have a valid, non-null
>    address.  */

Ok.

> 
>>> +  unsigned int lav = laversion (LAV_CURRENT);
>>> +  if (lav == 0)
>>
>> Maybe __glibc_unlikely here?
> 
> We don't really know what's unlikely or not.  Since audit modules are
> used only rarely, I don't want to worry about it.

Ok.

> 
> 
>>> +  /* Allocate structure for the callback function pointers.  This call
>>> +     can never fail.  */
>>> +  enum { naudit_ifaces = 8 };
>>> +  union
>>> +  {
>>> +    struct audit_ifaces ifaces;
>>> +    void (*fptr[naudit_ifaces]) (void);
>>> +  } *newp = malloc (sizeof (*newp));
>>
>> If this call should never fail, should we also assert for the malloc ?
> 
> I think we use _dl_fatal_printf elsewhere.  Again, this is a
> pre-existing issue. 8-/

Ok, we can live with it.

> 
>>
>>> +
>>> +  /* 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);
>>
>> Do we need the cast here?
> 
> I think we should switch to the new error handling framework
> (_dl_catch_exception) and not worry about it until then.

Alright, I think since you refactoring the code it would be good to
not propagate unnecessary code conventions like that. 

> 
>>> +
>>> +      /* 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;
>>
>> Same as before.
> 
> Yeah, an unrelated, pre-existing issue.

Ok.

> 
>>> +/* If we have any auditing modules, announce that we already have two
>>> +   objects loaded: the main program and the dynamic linker itself.  */
>>> +static void
>>> +early_audit_modules_notification (struct link_map *main_map)
>>> +{
>>> +  if (__glibc_likely (GLRO(dl_naudit) == 0))
>>> +    return;
>>> +
>>> +  struct link_map *ls[2] = { main_map, &GL(dl_rtld_map) };
>>> +
>>> +  for (unsigned int outer = 0; outer < 2; ++outer)
>>
>> Maybe size_t here?
> 
> Despite outer < 2?
> 
> I can unroll the loop in the caller, like this:
> 
>   if (__glibc_likely (GLRO(dl_naudit) > 0)
>     {
>       early_audit_modules_notification (main_map);
>       early_audit_modules_notification (&GL(dl_rtld_map));
>     }
> 
> That actually looks quite reasonable.  What do you think?

Look better to me, thanks.

> 
> Thanks,
> Florian
>
Florian Weimer Feb. 6, 2019, 1:52 p.m. | #4
* Adhemerval Zanella:

> Alright, I think since you refactoring the code it would be good to
> not propagate unnecessary code conventions like that.

I think the patch below addresses the minor quirks you noted.  I also
removed the awkward loop and moved the audit module notification into
load_audit_modules.

Thanks,
Florian

elf: Ignore LD_AUDIT interfaces if la_version returns 0 [BZ #24122]

This change moves the audit module loading and early notification into
separate functions out of dl_main.

It restores the bug fix from commit
8e889c5da3c5981c5a46a93fec02de40131ac5a6  ("elf: Fix LD_AUDIT for
modules with invalid version (BZ#24122)") which was reverted in commit
83e6b59625f45db1eee93e5684091f740c52a083  ("[elf] Revert 8e889c5da3
(BZ#24122)").

The actual bug fix is the separate error message for the case when
la_version is zero.  The dynamic linker error message (which is NULL
in this case) is no longer used.  Based on the intended use of version
zero (ignore this module due to explicit request), the message is only
printed if debugging is enabled.

2019-02-02  Florian Weimer  <fweimer@redhat.com>

	[BZ #24122]
	* elf/rtld.c (unload_audit_module): New function.
	(report_audit_module_load_error): Likewise.
	(load_audit_module): Likewise.  Extracted from dl_main.  Call
	_dl_close if the laversion symbol cannot be found.  Use early
	returns for error handling.  Add malloc error check.  Check for a
	zero return value from la_version.  Remove spurious comment about
	static TLS initialization.  Remove useless casts.
	(notify_audit_modules_of_loaded_object): New function.  Extracted
	from dl_main.
	(load_audit_module): Likewise.
	(dl_main): Call load_audit_modules.

diff --git a/elf/rtld.c b/elf/rtld.c
index 5a90e78ed6..2e3ccc2819 100644
--- a/elf/rtld.c
+++ b/elf/rtld.c
@@ -866,6 +866,205 @@ handle_preload_list (const char *preloadlist, struct link_map *main_map,
   return npreloads;
 }
 
+/* Called if the audit DSO cannot be used: if it does not have the
+   appropriate interfaces, or it expects a more recent version library
+   version than what the dynamic linker provides.  */
+static void
+unload_audit_module (struct link_map *map, int original_tls_idx)
+{
+#ifndef NDEBUG
+  Lmid_t ns = map->l_ns;
+#endif
+  _dl_close (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) = original_tls_idx;
+}
+
+/* Called to print an error message if loading of an audit module
+   failed.  */
+static void
+report_audit_module_load_error (const char *name, const char *err_str,
+				bool malloced)
+{
+  _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);
+}
+
+/* Load one audit module.  */
+static void
+load_audit_module (const char *name, struct audit_ifaces **last_audit)
+{
+  int original_tls_idx = GL(dl_tls_max_dtv_idx);
+
+  struct dlmopen_args dlmargs;
+  dlmargs.fname = name;
+  dlmargs.map = NULL;
+
+  const char *objname;
+  const char *err_str = NULL;
+  bool malloced;
+  _dl_catch_error (&objname, &err_str, &malloced, dlmopen_doit, &dlmargs);
+  if (__glibc_unlikely (err_str != NULL))
+    {
+      report_audit_module_load_error (name, err_str, malloced);
+      return;
+    }
+
+  struct lookup_args largs;
+  largs.name = "la_version";
+  largs.map = dlmargs.map;
+  _dl_catch_error (&objname, &err_str, &malloced, lookup_doit, &largs);
+  if (__glibc_likely (err_str != NULL))
+    {
+      unload_audit_module (dlmargs.map, original_tls_idx);
+      report_audit_module_load_error (name, err_str, malloced);
+      return;
+    }
+
+  unsigned int (*laversion) (unsigned int) = largs.result;
+
+ /* A null symbol indicates that something is very wrong with the
+    loaded object because defined symbols are supposed to have a
+    valid, non-null address.  */
+  assert (laversion != NULL);
+
+  unsigned int lav = laversion (LAV_CURRENT);
+  if (lav == 0)
+    {
+      /* Only print an error message if debugging because this can
+	 happen deliberately.  */
+      if (GLRO(dl_debug_mask) & DL_DEBUG_FILES)
+	_dl_debug_printf ("\
+file=%s [%lu]; audit interface function la_version returned zero; ignored.\n",
+			  dlmargs.map->l_name, dlmargs.map->l_ns);
+      unload_audit_module (dlmargs.map, original_tls_idx);
+      return;
+    }
+
+  if (lav > LAV_CURRENT)
+    {
+      _dl_debug_printf ("\
+ERROR: audit interface '%s' requires version %d (maximum supported version %d); ignored.\n",
+			name, lav, LAV_CURRENT);
+      unload_audit_module (dlmargs.map, original_tls_idx);
+      return;
+    }
+
+  enum { naudit_ifaces = 8 };
+  union
+  {
+    struct audit_ifaces ifaces;
+    void (*fptr[naudit_ifaces]) (void);
+  } *newp = malloc (sizeof (*newp));
+  if (newp == NULL)
+    _dl_fatal_printf ("Out of memory while loading audit modules\n");
+
+  /* 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;
+      _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 = 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;
+}
+
+/* Notify the the audit modules that the object MAP has already been
+   loaded.  */
+static void
+notify_audit_modules_of_loaded_object (struct link_map *map)
+{
+  struct audit_ifaces *afct = GLRO(dl_audit);
+  for (unsigned int cnt = 0; cnt < GLRO(dl_naudit); ++cnt)
+    {
+      if (afct->objopen != NULL)
+	{
+	  map->l_audit[cnt].bindflags
+	    = afct->objopen (map, LM_ID_BASE, &map->l_audit[cnt].cookie);
+	  map->l_audit_any_plt |= map->l_audit[cnt].bindflags != 0;
+	}
+
+      afct = afct->next;
+    }
+}
+
+/* Load all audit modules.  */
+static void
+load_audit_modules (struct link_map *main_map)
+{
+  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);
+      if (name == NULL)
+	break;
+      load_audit_module (name, &last_audit);
+    }
+
+  /* Notify audit modules of the initially loaded modules (the main
+     program and the dynamic linker itself).  */
+  if (GLRO(dl_naudit) > 0)
+    {
+      notify_audit_modules_of_loaded_object (main_map);
+      notify_audit_modules_of_loaded_object (&GL(dl_rtld_map));
+    }
+}
+
 static void
 dl_main (const ElfW(Phdr) *phdr,
 	 ElfW(Word) phnum,
@@ -1406,10 +1605,6 @@ ERROR: '%s': cannot process note segment.\n", _dl_argv[0]);
   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 ();
@@ -1421,164 +1616,7 @@ ERROR: '%s': cannot process note segment.\n", _dl_argv[0]);
       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;
-		}
-	    }
-	}
-
-      /* If we have any auditing modules, announce that we already
-	 have two objects loaded.  */
-      if (__glibc_unlikely (GLRO(dl_naudit) > 0))
-	{
-	  struct link_map *ls[2] = { main_map, &GL(dl_rtld_map) };
-
-	  for (unsigned int outer = 0; outer < 2; ++outer)
-	    {
-	      struct audit_ifaces *afct = GLRO(dl_audit);
-	      for (unsigned int cnt = 0; cnt < GLRO(dl_naudit); ++cnt)
-		{
-		  if (afct->objopen != NULL)
-		    {
-		      ls[outer]->l_audit[cnt].bindflags
-			= afct->objopen (ls[outer], LM_ID_BASE,
-					 &ls[outer]->l_audit[cnt].cookie);
-
-		      ls[outer]->l_audit_any_plt
-			|= ls[outer]->l_audit[cnt].bindflags != 0;
-		    }
-
-		  afct = afct->next;
-		}
-	    }
-	}
+      load_audit_modules (main_map);
     }
 
   /* Keep track of the currently loaded modules to count how many
Adhemerval Zanella Feb. 12, 2019, 12:14 p.m. | #5
On 06/02/2019 11:52, Florian Weimer wrote:
> * Adhemerval Zanella:
> 
>> Alright, I think since you refactoring the code it would be good to
>> not propagate unnecessary code conventions like that.
> 
> I think the patch below addresses the minor quirks you noted.  I also
> removed the awkward loop and moved the audit module notification into
> load_audit_modules.
> 
> Thanks,
> Florian
> 
> elf: Ignore LD_AUDIT interfaces if la_version returns 0 [BZ #24122]
> 
> This change moves the audit module loading and early notification into
> separate functions out of dl_main.
> 
> It restores the bug fix from commit
> 8e889c5da3c5981c5a46a93fec02de40131ac5a6  ("elf: Fix LD_AUDIT for
> modules with invalid version (BZ#24122)") which was reverted in commit
> 83e6b59625f45db1eee93e5684091f740c52a083  ("[elf] Revert 8e889c5da3
> (BZ#24122)").
> 
> The actual bug fix is the separate error message for the case when
> la_version is zero.  The dynamic linker error message (which is NULL
> in this case) is no longer used.  Based on the intended use of version
> zero (ignore this module due to explicit request), the message is only
> printed if debugging is enabled.
> 
> 2019-02-02  Florian Weimer  <fweimer@redhat.com>
> 
> 	[BZ #24122]
> 	* elf/rtld.c (unload_audit_module): New function.
> 	(report_audit_module_load_error): Likewise.
> 	(load_audit_module): Likewise.  Extracted from dl_main.  Call
> 	_dl_close if the laversion symbol cannot be found.  Use early
> 	returns for error handling.  Add malloc error check.  Check for a
> 	zero return value from la_version.  Remove spurious comment about
> 	static TLS initialization.  Remove useless casts.
> 	(notify_audit_modules_of_loaded_object): New function.  Extracted
> 	from dl_main.
> 	(load_audit_module): Likewise.
> 	(dl_main): Call load_audit_modules.

LGTM, thanks.

> 
> diff --git a/elf/rtld.c b/elf/rtld.c
> index 5a90e78ed6..2e3ccc2819 100644
> --- a/elf/rtld.c
> +++ b/elf/rtld.c
> @@ -866,6 +866,205 @@ handle_preload_list (const char *preloadlist, struct link_map *main_map,
>    return npreloads;
>  }
>  
> +/* Called if the audit DSO cannot be used: if it does not have the
> +   appropriate interfaces, or it expects a more recent version library
> +   version than what the dynamic linker provides.  */
> +static void
> +unload_audit_module (struct link_map *map, int original_tls_idx)
> +{
> +#ifndef NDEBUG
> +  Lmid_t ns = map->l_ns;
> +#endif
> +  _dl_close (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) = original_tls_idx;
> +}
> +
> +/* Called to print an error message if loading of an audit module
> +   failed.  */
> +static void
> +report_audit_module_load_error (const char *name, const char *err_str,
> +				bool malloced)
> +{
> +  _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);
> +}
> +
> +/* Load one audit module.  */
> +static void
> +load_audit_module (const char *name, struct audit_ifaces **last_audit)
> +{
> +  int original_tls_idx = GL(dl_tls_max_dtv_idx);
> +
> +  struct dlmopen_args dlmargs;
> +  dlmargs.fname = name;
> +  dlmargs.map = NULL;
> +
> +  const char *objname;
> +  const char *err_str = NULL;
> +  bool malloced;
> +  _dl_catch_error (&objname, &err_str, &malloced, dlmopen_doit, &dlmargs);
> +  if (__glibc_unlikely (err_str != NULL))
> +    {
> +      report_audit_module_load_error (name, err_str, malloced);
> +      return;
> +    }
> +
> +  struct lookup_args largs;
> +  largs.name = "la_version";
> +  largs.map = dlmargs.map;
> +  _dl_catch_error (&objname, &err_str, &malloced, lookup_doit, &largs);
> +  if (__glibc_likely (err_str != NULL))
> +    {
> +      unload_audit_module (dlmargs.map, original_tls_idx);
> +      report_audit_module_load_error (name, err_str, malloced);
> +      return;
> +    }
> +
> +  unsigned int (*laversion) (unsigned int) = largs.result;
> +
> + /* A null symbol indicates that something is very wrong with the
> +    loaded object because defined symbols are supposed to have a
> +    valid, non-null address.  */
> +  assert (laversion != NULL);
> +
> +  unsigned int lav = laversion (LAV_CURRENT);
> +  if (lav == 0)
> +    {
> +      /* Only print an error message if debugging because this can
> +	 happen deliberately.  */
> +      if (GLRO(dl_debug_mask) & DL_DEBUG_FILES)
> +	_dl_debug_printf ("\
> +file=%s [%lu]; audit interface function la_version returned zero; ignored.\n",
> +			  dlmargs.map->l_name, dlmargs.map->l_ns);
> +      unload_audit_module (dlmargs.map, original_tls_idx);
> +      return;
> +    }
> +
> +  if (lav > LAV_CURRENT)
> +    {
> +      _dl_debug_printf ("\
> +ERROR: audit interface '%s' requires version %d (maximum supported version %d); ignored.\n",
> +			name, lav, LAV_CURRENT);
> +      unload_audit_module (dlmargs.map, original_tls_idx);
> +      return;
> +    }
> +
> +  enum { naudit_ifaces = 8 };
> +  union
> +  {
> +    struct audit_ifaces ifaces;
> +    void (*fptr[naudit_ifaces]) (void);
> +  } *newp = malloc (sizeof (*newp));
> +  if (newp == NULL)
> +    _dl_fatal_printf ("Out of memory while loading audit modules\n");
> +
> +  /* 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;
> +      _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 = 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;
> +}
> +
> +/* Notify the the audit modules that the object MAP has already been
> +   loaded.  */
> +static void
> +notify_audit_modules_of_loaded_object (struct link_map *map)
> +{
> +  struct audit_ifaces *afct = GLRO(dl_audit);
> +  for (unsigned int cnt = 0; cnt < GLRO(dl_naudit); ++cnt)
> +    {
> +      if (afct->objopen != NULL)
> +	{
> +	  map->l_audit[cnt].bindflags
> +	    = afct->objopen (map, LM_ID_BASE, &map->l_audit[cnt].cookie);
> +	  map->l_audit_any_plt |= map->l_audit[cnt].bindflags != 0;
> +	}
> +
> +      afct = afct->next;
> +    }
> +}
> +
> +/* Load all audit modules.  */
> +static void
> +load_audit_modules (struct link_map *main_map)
> +{
> +  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);
> +      if (name == NULL)
> +	break;
> +      load_audit_module (name, &last_audit);
> +    }
> +
> +  /* Notify audit modules of the initially loaded modules (the main
> +     program and the dynamic linker itself).  */
> +  if (GLRO(dl_naudit) > 0)
> +    {
> +      notify_audit_modules_of_loaded_object (main_map);
> +      notify_audit_modules_of_loaded_object (&GL(dl_rtld_map));
> +    }
> +}
> +
>  static void
>  dl_main (const ElfW(Phdr) *phdr,
>  	 ElfW(Word) phnum,
> @@ -1406,10 +1605,6 @@ ERROR: '%s': cannot process note segment.\n", _dl_argv[0]);
>    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 ();
> @@ -1421,164 +1616,7 @@ ERROR: '%s': cannot process note segment.\n", _dl_argv[0]);
>        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;
> -		}
> -	    }
> -	}
> -
> -      /* If we have any auditing modules, announce that we already
> -	 have two objects loaded.  */
> -      if (__glibc_unlikely (GLRO(dl_naudit) > 0))
> -	{
> -	  struct link_map *ls[2] = { main_map, &GL(dl_rtld_map) };
> -
> -	  for (unsigned int outer = 0; outer < 2; ++outer)
> -	    {
> -	      struct audit_ifaces *afct = GLRO(dl_audit);
> -	      for (unsigned int cnt = 0; cnt < GLRO(dl_naudit); ++cnt)
> -		{
> -		  if (afct->objopen != NULL)
> -		    {
> -		      ls[outer]->l_audit[cnt].bindflags
> -			= afct->objopen (ls[outer], LM_ID_BASE,
> -					 &ls[outer]->l_audit[cnt].cookie);
> -
> -		      ls[outer]->l_audit_any_plt
> -			|= ls[outer]->l_audit[cnt].bindflags != 0;
> -		    }
> -
> -		  afct = afct->next;
> -		}
> -	    }
> -	}
> +      load_audit_modules (main_map);
>      }
>  
>    /* Keep track of the currently loaded modules to count how many
>

Patch

diff --git a/elf/rtld.c b/elf/rtld.c
index 5d97f41b7b..3babe0185b 100644
--- a/elf/rtld.c
+++ b/elf/rtld.c
@@ -863,6 +863,205 @@  handle_ld_preload (const char *preloadlist, struct link_map *main_map)
   return npreloads;
 }
 
+/* Called if the audit DSO cannot be used: if it does not have the
+   appropriate interfaces, or it expects a more recent version library
+   version than what the dynamic linker provides.  */
+static void
+unload_audit_module (struct link_map *map, int original_tls_idx)
+{
+#ifndef NDEBUG
+  Lmid_t ns = map->l_ns;
+#endif
+  _dl_close (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) = original_tls_idx;
+}
+
+/* Called to print an error message if loading of an audit module
+   failed.  */
+static void
+report_audit_module_load_error (const char *name, const char *err_str,
+				bool malloced)
+{
+  _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);
+}
+
+/* Load one audit module.  */
+static void
+load_audit_module (const char *name, struct audit_ifaces **last_audit)
+{
+  int original_tls_idx = GL(dl_tls_max_dtv_idx);
+
+  struct dlmopen_args dlmargs;
+  dlmargs.fname = name;
+  dlmargs.map = NULL;
+
+  const char *objname;
+  const char *err_str = NULL;
+  bool malloced;
+  _dl_catch_error (&objname, &err_str, &malloced, dlmopen_doit, &dlmargs);
+  if (__glibc_unlikely (err_str != NULL))
+    {
+      report_audit_module_load_error (name, err_str, malloced);
+      return;
+    }
+
+  struct lookup_args largs;
+  largs.name = "la_version";
+  largs.map = dlmargs.map;
+  _dl_catch_error (&objname, &err_str, &malloced, lookup_doit, &largs);
+  if (__glibc_likely (err_str != NULL))
+    {
+      unload_audit_module (dlmargs.map, original_tls_idx);
+      report_audit_module_load_error (name, err_str, malloced);
+      return;
+    }
+
+  unsigned int (*laversion) (unsigned int) = largs.result;
+  assert (laversion != NULL);
+  unsigned int lav = laversion (LAV_CURRENT);
+  if (lav == 0)
+    {
+      /* Only print an error message if debugging because this can
+	 happen deliberately.  */
+      if (GLRO(dl_debug_mask) & DL_DEBUG_FILES)
+	_dl_debug_printf ("\
+file=%s [%lu]; audit interface function la_version returned zero; ignored.\n",
+			  dlmargs.map->l_name, dlmargs.map->l_ns);
+      unload_audit_module (dlmargs.map, original_tls_idx);
+      return;
+    }
+
+  if (lav > LAV_CURRENT)
+    {
+      _dl_debug_printf ("\
+ERROR: audit interface '%s' requires version %d (maximum supported version %d); ignored.\n",
+			name, lav, LAV_CURRENT);
+      unload_audit_module (dlmargs.map, original_tls_idx);
+      return;
+    }
+
+  /* Allocate structure for the callback function pointers.  This call
+     can never fail.  */
+  enum { naudit_ifaces = 8 };
+  union
+  {
+    struct audit_ifaces ifaces;
+    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;
+}
+
+/* Load all audit modules.  */
+static void
+load_audit_modules (void)
+{
+  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);
+      if (name == NULL)
+	break;
+      load_audit_module (name, &last_audit);
+    }
+}
+
+/* If we have any auditing modules, announce that we already have two
+   objects loaded: the main program and the dynamic linker itself.  */
+static void
+early_audit_modules_notification (struct link_map *main_map)
+{
+  if (__glibc_likely (GLRO(dl_naudit) == 0))
+    return;
+
+  struct link_map *ls[2] = { main_map, &GL(dl_rtld_map) };
+
+  for (unsigned int outer = 0; outer < 2; ++outer)
+    {
+      struct audit_ifaces *afct = GLRO(dl_audit);
+      for (unsigned int cnt = 0; cnt < GLRO(dl_naudit); ++cnt)
+	{
+	  if (afct->objopen != NULL)
+	    {
+	      ls[outer]->l_audit[cnt].bindflags
+		= afct->objopen (ls[outer], LM_ID_BASE,
+				 &ls[outer]->l_audit[cnt].cookie);
+
+	      ls[outer]->l_audit_any_plt
+		|= ls[outer]->l_audit[cnt].bindflags != 0;
+	    }
+
+	  afct = afct->next;
+	}
+    }
+}
+
 static void
 dl_main (const ElfW(Phdr) *phdr,
 	 ElfW(Word) phnum,
@@ -1395,10 +1594,6 @@  ERROR: '%s': cannot process note segment.\n", _dl_argv[0]);
   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 ();
@@ -1410,164 +1605,8 @@  ERROR: '%s': cannot process note segment.\n", _dl_argv[0]);
       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;
-		}
-	    }
-	}
-
-      /* If we have any auditing modules, announce that we already
-	 have two objects loaded.  */
-      if (__glibc_unlikely (GLRO(dl_naudit) > 0))
-	{
-	  struct link_map *ls[2] = { main_map, &GL(dl_rtld_map) };
-
-	  for (unsigned int outer = 0; outer < 2; ++outer)
-	    {
-	      struct audit_ifaces *afct = GLRO(dl_audit);
-	      for (unsigned int cnt = 0; cnt < GLRO(dl_naudit); ++cnt)
-		{
-		  if (afct->objopen != NULL)
-		    {
-		      ls[outer]->l_audit[cnt].bindflags
-			= afct->objopen (ls[outer], LM_ID_BASE,
-					 &ls[outer]->l_audit[cnt].cookie);
-
-		      ls[outer]->l_audit_any_plt
-			|= ls[outer]->l_audit[cnt].bindflags != 0;
-		    }
-
-		  afct = afct->next;
-		}
-	    }
-	}
+      load_audit_modules ();
+      early_audit_modules_notification (main_map);
     }
 
   /* Keep track of the currently loaded modules to count how many