Re: [patch] Fix BZ #16634 -- assert in ld.so when dlopen("a.out"...) is called repeatedly.

[Siddhesh Poyarekar <siddhesh at redhat dot com>]

On Fri, Mar 14, 2014 at 02:54:05PM -0700, Paul Pluzhnikov wrote:
> Greetings,
> 
> Attached patch fixes BZ #16634 by moving sanity check for dlopen()ing
> a.out before we call _dl_next_tls_modid() for it.
> 
> Tested on Linux/x86_64; no new failures.

A more detailed description please, assuming that it's going to be
used for the commit log.

> 
> --
> 
> 2014-03-14  Paul Pluzhnikov  <ppluzhnikov@google.com>
> 
> 	BZ #16634

In square brackets.

>         * elf/dl-load.c (open_verify): Add mode parameter.
>         Error early when ET_EXEC and mode does not have __RTLD_OPENEXEC.
>         (open_path): Change from boolean 'secure' to complete flag 'mode'
>         (_dl_map_object): Adjust.
> 
> 
> diff --git a/elf/dl-load.c b/elf/dl-load.c
> index 8ebc128..3f90b92 100644
> --- a/elf/dl-load.c
> +++ b/elf/dl-load.c
> @@ -1667,7 +1667,7 @@ print_search_path (struct r_search_path_elem **list,
>     user might want to know about this.  */
>  static int
>  open_verify (const char *name, struct filebuf *fbp, struct link_map *loader,
> -	     int whatcode, bool *found_other_class, bool free_name)
> +	     int whatcode, int mode, bool *found_other_class, bool free_name)
>  {
>    /* This is the expected ELF header.  */
>  #define ELF32_CLASS ELFCLASS32
> @@ -1843,6 +1843,13 @@ open_verify (const char *name, struct filebuf *fbp, struct link_map *loader,
>  	  errstring = N_("only ET_DYN and ET_EXEC can be loaded");
>  	  goto call_lose;
>  	}
> +      else if (__glibc_unlikely (ehdr->e_type == ET_EXEC
> +				 && (mode & __RTLD_OPENEXEC) == 0))
> +	{
> +	  /* For BZ #16634, return early.  */

Likewise, please explain the condition instead of just quoting the bz
number.

> +	  errstring = N_("cannot dynamically load executable");
> +	  goto call_lose;
> +	}
>        else if (__builtin_expect (ehdr->e_phentsize, sizeof (ElfW(Phdr)))
>  	       != sizeof (ElfW(Phdr)))
>  	{
> @@ -1928,7 +1935,7 @@ open_verify (const char *name, struct filebuf *fbp, struct link_map *loader,
>     if MAY_FREE_DIRS is true.  */
>  
>  static int
> -open_path (const char *name, size_t namelen, int secure,
> +open_path (const char *name, size_t namelen, int mode,
>  	   struct r_search_path_struct *sps, char **realname,
>  	   struct filebuf *fbp, struct link_map *loader, int whatcode,
>  	   bool *found_other_class)
> @@ -1980,8 +1987,8 @@ open_path (const char *name, size_t namelen, int secure,
>  	  if (__glibc_unlikely (GLRO(dl_debug_mask) & DL_DEBUG_LIBS))
>  	    _dl_debug_printf ("  trying file=%s\n", buf);
>  
> -	  fd = open_verify (buf, fbp, loader, whatcode, found_other_class,
> -			    false);
> +	  fd = open_verify (buf, fbp, loader, whatcode, mode,
> +			    found_other_class, false);
>  	  if (this_dir->status[cnt] == unknown)
>  	    {
>  	      if (fd != -1)
> @@ -2010,7 +2017,7 @@ open_path (const char *name, size_t namelen, int secure,
>  	  /* Remember whether we found any existing directory.  */
>  	  here_any |= this_dir->status[cnt] != nonexisting;
>  
> -	  if (fd != -1 && __builtin_expect (secure, 0)
> +	  if (fd != -1 && __builtin_expect (mode & __RTLD_SECURE, 0)

Use __glibc_unlikely.


Siddhesh

Attachment: pgppQPi3IEXa4.pgp
Description: PGP signature