diff mbox series

[v2] elf: Fix pldd (BZ#18035)

Message ID 20190417144233.2470-1-adhemerval.zanella@linaro.org
State New
Headers show
Series [v2] elf: Fix pldd (BZ#18035) | expand

Commit Message

Adhemerval Zanella Netto April 17, 2019, 2:42 p.m. UTC
Changes from previous version:

  - Removal of more braces.
  - Added a comment where the old code was removed to warn against looking
    at l_libname.
  - Adjusted test comment.
  - Remove file artifact.

---

Since 9182aa67994 (Fix vDSO l_name for GDB's, BZ#387) the initial link_map
for executable itself and loader will have both l_name and l_libname->name
holding the same value due:

 elf/dl-object.c

 95   new->l_name = *realname ? realname : (char *) newname->name + libname_len - 1;

Since newname->name points to new->l_libname->name.

This leads to pldd to an infinite call at:

 elf/pldd-xx.c

203     again:
204       while (1)
205         {
206           ssize_t n = pread64 (memfd, tmpbuf.data, tmpbuf.length, name_offset);

228           /* Try the l_libname element.  */
229           struct E(libname_list) ln;
230           if (pread64 (memfd, &ln, sizeof (ln), m.l_libname) == sizeof (ln))
231             {
232               name_offset = ln.name;
233               goto again;
234             }

Since the value at ln.name (l_libname->name) will be the same as previously
read. The straightforward fix is just avoid the check and read the new list
entry.

I checked also against binaries issues with old loaders with fix for BZ#387,
and pldd could dump the shared objects.

Checked on x86_64-linux-gnu, i686-linux-gnu, aarch64-linux-gnu, and
powerpc64le-linux-gnu.

	[BZ #18035]
	* elf/Makefile (tests-container): Add tst-pldd.
	* elf/pldd-xx.c: Use _Static_assert in of pldd_assert.
	(E(find_maps)): Avoid use alloca, use default read file operations
	instead of explicit LFS names, and fix infinite	loop.
	* elf/pldd.c: Explicit set _FILE_OFFSET_BITS, cleanup headers.
	(get_process_info): Use _Static_assert instead of assert, use default
	directory operations instead of explicit LFS names, and free some
	leadek pointers.
	* elf/tst-pldd.c: New file.
	* elf/tst-pldd.root/tst-pldd.script: Likewise.
---
 elf/Makefile                      |   1 +
 elf/pldd-xx.c                     | 114 ++++++++++-------------------
 elf/pldd.c                        |  64 ++++++++--------
 elf/tst-pldd.c                    | 118 ++++++++++++++++++++++++++++++
 elf/tst-pldd.root/tst-pldd.script |   1 +
 5 files changed, 190 insertions(+), 108 deletions(-)
 create mode 100644 elf/tst-pldd.c
 create mode 100644 elf/tst-pldd.root/tst-pldd.script

-- 
2.17.1

Comments

Adhemerval Zanella Netto April 23, 2019, 5:35 p.m. UTC | #1
Ping.

On 17/04/2019 11:42, Adhemerval Zanella wrote:
> Changes from previous version:

> 

>   - Removal of more braces.

>   - Added a comment where the old code was removed to warn against looking

>     at l_libname.

>   - Adjusted test comment.

>   - Remove file artifact.

> 

> ---

> 

> Since 9182aa67994 (Fix vDSO l_name for GDB's, BZ#387) the initial link_map

> for executable itself and loader will have both l_name and l_libname->name

> holding the same value due:

> 

>  elf/dl-object.c

> 

>  95   new->l_name = *realname ? realname : (char *) newname->name + libname_len - 1;

> 

> Since newname->name points to new->l_libname->name.

> 

> This leads to pldd to an infinite call at:

> 

>  elf/pldd-xx.c

> 

> 203     again:

> 204       while (1)

> 205         {

> 206           ssize_t n = pread64 (memfd, tmpbuf.data, tmpbuf.length, name_offset);

> 

> 228           /* Try the l_libname element.  */

> 229           struct E(libname_list) ln;

> 230           if (pread64 (memfd, &ln, sizeof (ln), m.l_libname) == sizeof (ln))

> 231             {

> 232               name_offset = ln.name;

> 233               goto again;

> 234             }

> 

> Since the value at ln.name (l_libname->name) will be the same as previously

> read. The straightforward fix is just avoid the check and read the new list

> entry.

> 

> I checked also against binaries issues with old loaders with fix for BZ#387,

> and pldd could dump the shared objects.

> 

> Checked on x86_64-linux-gnu, i686-linux-gnu, aarch64-linux-gnu, and

> powerpc64le-linux-gnu.

> 

> 	[BZ #18035]

> 	* elf/Makefile (tests-container): Add tst-pldd.

> 	* elf/pldd-xx.c: Use _Static_assert in of pldd_assert.

> 	(E(find_maps)): Avoid use alloca, use default read file operations

> 	instead of explicit LFS names, and fix infinite	loop.

> 	* elf/pldd.c: Explicit set _FILE_OFFSET_BITS, cleanup headers.

> 	(get_process_info): Use _Static_assert instead of assert, use default

> 	directory operations instead of explicit LFS names, and free some

> 	leadek pointers.

> 	* elf/tst-pldd.c: New file.

> 	* elf/tst-pldd.root/tst-pldd.script: Likewise.

> ---

>  elf/Makefile                      |   1 +

>  elf/pldd-xx.c                     | 114 ++++++++++-------------------

>  elf/pldd.c                        |  64 ++++++++--------

>  elf/tst-pldd.c                    | 118 ++++++++++++++++++++++++++++++

>  elf/tst-pldd.root/tst-pldd.script |   1 +

>  5 files changed, 190 insertions(+), 108 deletions(-)

>  create mode 100644 elf/tst-pldd.c

>  create mode 100644 elf/tst-pldd.root/tst-pldd.script

> 

> diff --git a/elf/Makefile b/elf/Makefile

> index 310a37cc13..0b4a877880 100644

> --- a/elf/Makefile

> +++ b/elf/Makefile

> @@ -194,6 +194,7 @@ tests-internal += loadtest unload unload2 circleload1 \

>  	 tst-tls3 tst-tls6 tst-tls7 tst-tls8 tst-dlmopen2 \

>  	 tst-ptrguard1 tst-stackguard1 tst-libc_dlvsym \

>  	 tst-create_format1

> +tests-container += tst-pldd

>  ifeq ($(build-hardcoded-path-in-tests),yes)

>  tests += tst-dlopen-aout

>  tst-dlopen-aout-no-pie = yes

> diff --git a/elf/pldd-xx.c b/elf/pldd-xx.c

> index 547f840ee1..28a8659935 100644

> --- a/elf/pldd-xx.c

> +++ b/elf/pldd-xx.c

> @@ -23,10 +23,6 @@

>  #define EW_(e, w, t) EW__(e, w, _##t)

>  #define EW__(e, w, t) e##w##t

>  

> -#define pldd_assert(name, exp) \

> -  typedef int __assert_##name[((exp) != 0) - 1]

> -

> -

>  struct E(link_map)

>  {

>    EW(Addr) l_addr;

> @@ -39,12 +35,12 @@ struct E(link_map)

>    EW(Addr) l_libname;

>  };

>  #if CLASS == __ELF_NATIVE_CLASS

> -pldd_assert (l_addr, (offsetof (struct link_map, l_addr)

> -			== offsetof (struct E(link_map), l_addr)));

> -pldd_assert (l_name, (offsetof (struct link_map, l_name)

> -			== offsetof (struct E(link_map), l_name)));

> -pldd_assert (l_next, (offsetof (struct link_map, l_next)

> -			== offsetof (struct E(link_map), l_next)));

> +_Static_assert (offsetof (struct link_map, l_addr)

> +		== offsetof (struct E(link_map), l_addr), "l_addr");

> +_Static_assert (offsetof (struct link_map, l_name)

> +		== offsetof (struct E(link_map), l_name), "l_name");

> +_Static_assert (offsetof (struct link_map, l_next)

> +		== offsetof (struct E(link_map), l_next), "l_next");

>  #endif

>  

>  

> @@ -54,10 +50,10 @@ struct E(libname_list)

>    EW(Addr) next;

>  };

>  #if CLASS == __ELF_NATIVE_CLASS

> -pldd_assert (name, (offsetof (struct libname_list, name)

> -		      == offsetof (struct E(libname_list), name)));

> -pldd_assert (next, (offsetof (struct libname_list, next)

> -		      == offsetof (struct E(libname_list), next)));

> +_Static_assert (offsetof (struct libname_list, name)

> +		== offsetof (struct E(libname_list), name), "name");

> +_Static_assert (offsetof (struct libname_list, next)

> +		== offsetof (struct E(libname_list), next), "next");

>  #endif

>  

>  struct E(r_debug)

> @@ -69,16 +65,17 @@ struct E(r_debug)

>    EW(Addr) r_map;

>  };

>  #if CLASS == __ELF_NATIVE_CLASS

> -pldd_assert (r_version, (offsetof (struct r_debug, r_version)

> -			   == offsetof (struct E(r_debug), r_version)));

> -pldd_assert (r_map, (offsetof (struct r_debug, r_map)

> -		       == offsetof (struct E(r_debug), r_map)));

> +_Static_assert (offsetof (struct r_debug, r_version)

> +		== offsetof (struct E(r_debug), r_version), "r_version");

> +_Static_assert (offsetof (struct r_debug, r_map)

> +		== offsetof (struct E(r_debug), r_map), "r_map");

>  #endif

>  

>  

>  static int

>  

> -E(find_maps) (pid_t pid, void *auxv, size_t auxv_size)

> +E(find_maps) (const char *exe, int memfd, pid_t pid, void *auxv,

> +	      size_t auxv_size)

>  {

>    EW(Addr) phdr = 0;

>    unsigned int phnum = 0;

> @@ -104,12 +101,9 @@ E(find_maps) (pid_t pid, void *auxv, size_t auxv_size)

>    if (phdr == 0 || phnum == 0 || phent == 0)

>      error (EXIT_FAILURE, 0, gettext ("cannot find program header of process"));

>  

> -  EW(Phdr) *p = alloca (phnum * phent);

> -  if (pread64 (memfd, p, phnum * phent, phdr) != phnum * phent)

> -    {

> -      error (0, 0, gettext ("cannot read program header"));

> -      return EXIT_FAILURE;

> -    }

> +  EW(Phdr) *p = xmalloc (phnum * phent);

> +  if (pread (memfd, p, phnum * phent, phdr) != phnum * phent)

> +    error (EXIT_FAILURE, 0, gettext ("cannot read program header"));

>  

>    /* Determine the load offset.  We need this for interpreting the

>       other program header entries so we do this in a separate loop.

> @@ -129,24 +123,18 @@ E(find_maps) (pid_t pid, void *auxv, size_t auxv_size)

>      if (p[i].p_type == PT_DYNAMIC)

>        {

>  	EW(Dyn) *dyn = xmalloc (p[i].p_filesz);

> -	if (pread64 (memfd, dyn, p[i].p_filesz, offset + p[i].p_vaddr)

> +	if (pread (memfd, dyn, p[i].p_filesz, offset + p[i].p_vaddr)

>  	    != p[i].p_filesz)

> -	  {

> -	    error (0, 0, gettext ("cannot read dynamic section"));

> -	    return EXIT_FAILURE;

> -	  }

> +	  error (EXIT_FAILURE, 0, gettext ("cannot read dynamic section"));

>  

>  	/* Search for the DT_DEBUG entry.  */

>  	for (unsigned int j = 0; j < p[i].p_filesz / sizeof (EW(Dyn)); ++j)

>  	  if (dyn[j].d_tag == DT_DEBUG && dyn[j].d_un.d_ptr != 0)

>  	    {

>  	      struct E(r_debug) r;

> -	      if (pread64 (memfd, &r, sizeof (r), dyn[j].d_un.d_ptr)

> +	      if (pread (memfd, &r, sizeof (r), dyn[j].d_un.d_ptr)

>  		  != sizeof (r))

> -		{

> -		  error (0, 0, gettext ("cannot read r_debug"));

> -		  return EXIT_FAILURE;

> -		}

> +		error (EXIT_FAILURE, 0, gettext ("cannot read r_debug"));

>  

>  	      if (r.r_map != 0)

>  		{

> @@ -160,13 +148,10 @@ E(find_maps) (pid_t pid, void *auxv, size_t auxv_size)

>        }

>      else if (p[i].p_type == PT_INTERP)

>        {

> -	interp = alloca (p[i].p_filesz);

> -	if (pread64 (memfd, interp, p[i].p_filesz, offset + p[i].p_vaddr)

> +	interp = xmalloc (p[i].p_filesz);

> +	if (pread (memfd, interp, p[i].p_filesz, offset + p[i].p_vaddr)

>  	    != p[i].p_filesz)

> -	  {

> -	    error (0, 0, gettext ("cannot read program interpreter"));

> -	    return EXIT_FAILURE;

> -	  }

> +	  error (EXIT_FAILURE, 0, gettext ("cannot read program interpreter"));

>        }

>  

>    if (list == 0)

> @@ -174,14 +159,16 @@ E(find_maps) (pid_t pid, void *auxv, size_t auxv_size)

>        if (interp == NULL)

>  	{

>  	  // XXX check whether the executable itself is the loader

> -	  return EXIT_FAILURE;

> +	  exit (EXIT_FAILURE);

>  	}

>  

>        // XXX perhaps try finding ld.so and _r_debug in it

> -

> -      return EXIT_FAILURE;

> +      exit (EXIT_FAILURE);

>      }

>  

> +  free (p);

> +  free (interp);

> +

>    /* Print the PID and program name first.  */

>    printf ("%lu:\t%s\n", (unsigned long int) pid, exe);

>  

> @@ -192,47 +179,27 @@ E(find_maps) (pid_t pid, void *auxv, size_t auxv_size)

>    do

>      {

>        struct E(link_map) m;

> -      if (pread64 (memfd, &m, sizeof (m), list) != sizeof (m))

> -	{

> -	  error (0, 0, gettext ("cannot read link map"));

> -	  status = EXIT_FAILURE;

> -	  goto out;

> -	}

> +      if (pread (memfd, &m, sizeof (m), list) != sizeof (m))

> +	error (EXIT_FAILURE, 0, gettext ("cannot read link map"));

>  

>        EW(Addr) name_offset = m.l_name;

> -    again:

>        while (1)

>  	{

> -	  ssize_t n = pread64 (memfd, tmpbuf.data, tmpbuf.length, name_offset);

> +	  ssize_t n = pread (memfd, tmpbuf.data, tmpbuf.length, name_offset);

>  	  if (n == -1)

> -	    {

> -	      error (0, 0, gettext ("cannot read object name"));

> -	      status = EXIT_FAILURE;

> -	      goto out;

> -	    }

> +	    error (EXIT_FAILURE, 0, gettext ("cannot read object name"));

>  

>  	  if (memchr (tmpbuf.data, '\0', n) != NULL)

>  	    break;

>  

>  	  if (!scratch_buffer_grow (&tmpbuf))

> -	    {

> -	      error (0, 0, gettext ("cannot allocate buffer for object name"));

> -	      status = EXIT_FAILURE;

> -	      goto out;

> -	    }

> +	    error (EXIT_FAILURE, 0,

> +		   gettext ("cannot allocate buffer for object name"));

>  	}

>  

> -      if (((char *)tmpbuf.data)[0] == '\0' && name_offset == m.l_name

> -	  && m.l_libname != 0)

> -	{

> -	  /* Try the l_libname element.  */

> -	  struct E(libname_list) ln;

> -	  if (pread64 (memfd, &ln, sizeof (ln), m.l_libname) == sizeof (ln))

> -	    {

> -	      name_offset = ln.name;

> -	      goto again;

> -	    }

> -	}

> +      /* The m.l_name and m.l_libname.name for loader linkmap points to same

> +	 values (since BZ#387 fix).  Trying to use l_libname name as the

> +	 shared object name might lead to an infinite loop (BZ#18035).  */ 

>  

>        /* Skip over the executable.  */

>        if (((char *)tmpbuf.data)[0] != '\0')

> @@ -242,7 +209,6 @@ E(find_maps) (pid_t pid, void *auxv, size_t auxv_size)

>      }

>    while (list != 0);

>  

> - out:

>    scratch_buffer_free (&tmpbuf);

>    return status;

>  }

> diff --git a/elf/pldd.c b/elf/pldd.c

> index f3fac4e487..69629bd5d2 100644

> --- a/elf/pldd.c

> +++ b/elf/pldd.c

> @@ -17,23 +17,17 @@

>     License along with the GNU C Library; if not, see

>     <http://www.gnu.org/licenses/>.  */

>  

> -#include <alloca.h>

> +#define _FILE_OFFSET_BITS 64

> +

>  #include <argp.h>

> -#include <assert.h>

>  #include <dirent.h>

> -#include <elf.h>

> -#include <errno.h>

>  #include <error.h>

>  #include <fcntl.h>

>  #include <libintl.h>

> -#include <link.h>

> -#include <stddef.h>

>  #include <stdio.h>

>  #include <stdlib.h>

> -#include <string.h>

>  #include <unistd.h>

>  #include <sys/ptrace.h>

> -#include <sys/stat.h>

>  #include <sys/wait.h>

>  #include <scratch_buffer.h>

>  

> @@ -76,14 +70,9 @@ static struct argp argp =

>    options, parse_opt, args_doc, doc, NULL, more_help, NULL

>  };

>  

> -// File descriptor of /proc/*/mem file.

> -static int memfd;

> -

> -/* Name of the executable  */

> -static char *exe;

>  

>  /* Local functions.  */

> -static int get_process_info (int dfd, long int pid);

> +static int get_process_info (const char *exe, int dfd, long int pid);

>  static void wait_for_ptrace_stop (long int pid);

>  

>  

> @@ -102,8 +91,10 @@ main (int argc, char *argv[])

>        return 1;

>      }

>  

> -  assert (sizeof (pid_t) == sizeof (int)

> -	  || sizeof (pid_t) == sizeof (long int));

> +  _Static_assert (sizeof (pid_t) == sizeof (int)

> +		  || sizeof (pid_t) == sizeof (long int),

> +		  "sizeof (pid_t) != sizeof (int) or sizeof (long int)");

> +

>    char *endp;

>    errno = 0;

>    long int pid = strtol (argv[remaining], &endp, 10);

> @@ -119,25 +110,24 @@ main (int argc, char *argv[])

>    if (dfd == -1)

>      error (EXIT_FAILURE, errno, gettext ("cannot open %s"), buf);

>  

> -  struct scratch_buffer exebuf;

> -  scratch_buffer_init (&exebuf);

> +  /* Name of the executable  */

> +  struct scratch_buffer exe;

> +  scratch_buffer_init (&exe);

>    ssize_t nexe;

>    while ((nexe = readlinkat (dfd, "exe",

> -			     exebuf.data, exebuf.length)) == exebuf.length)

> +			     exe.data, exe.length)) == exe.length)

>      {

> -      if (!scratch_buffer_grow (&exebuf))

> +      if (!scratch_buffer_grow (&exe))

>  	{

>  	  nexe = -1;

>  	  break;

>  	}

>      }

>    if (nexe == -1)

> -    exe = (char *) "<program name undetermined>";

> +    /* Default stack allocation is at least 1024.  */

> +    snprintf (exe.data, exe.length, "<program name undetermined>");

>    else

> -    {

> -      exe = exebuf.data;

> -      exe[nexe] = '\0';

> -    }

> +    ((char*)exe.data)[nexe] = '\0';

>  

>    /* Stop all threads since otherwise the list of loaded modules might

>       change while we are reading it.  */

> @@ -155,8 +145,8 @@ main (int argc, char *argv[])

>      error (EXIT_FAILURE, errno, gettext ("cannot prepare reading %s/task"),

>  	   buf);

>  

> -  struct dirent64 *d;

> -  while ((d = readdir64 (dir)) != NULL)

> +  struct dirent *d;

> +  while ((d = readdir (dir)) != NULL)

>      {

>        if (! isdigit (d->d_name[0]))

>  	continue;

> @@ -182,7 +172,7 @@ main (int argc, char *argv[])

>  

>        wait_for_ptrace_stop (tid);

>  

> -      struct thread_list *newp = alloca (sizeof (*newp));

> +      struct thread_list *newp = xmalloc (sizeof (*newp));

>        newp->tid = tid;

>        newp->next = thread_list;

>        thread_list = newp;

> @@ -190,17 +180,22 @@ main (int argc, char *argv[])

>  

>    closedir (dir);

>  

> -  int status = get_process_info (dfd, pid);

> +  if (thread_list == NULL)

> +    error (EXIT_FAILURE, 0, gettext ("no valid %s/task entries"), buf);

> +

> +  int status = get_process_info (exe.data, dfd, pid);

>  

> -  assert (thread_list != NULL);

>    do

>      {

>        ptrace (PTRACE_DETACH, thread_list->tid, NULL, NULL);

> +      struct thread_list *prev = thread_list;

>        thread_list = thread_list->next;

> +      free (prev);

>      }

>    while (thread_list != NULL);

>  

>    close (dfd);

> +  scratch_buffer_free (&exe);

>  

>    return status;

>  }

> @@ -281,9 +276,10 @@ warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.\n\

>  

>  

>  static int

> -get_process_info (int dfd, long int pid)

> +get_process_info (const char *exe, int dfd, long int pid)

>  {

> -  memfd = openat (dfd, "mem", O_RDONLY);

> +  /* File descriptor of /proc/<pid>/mem file.  */

> +  int memfd = openat (dfd, "mem", O_RDONLY);

>    if (memfd == -1)

>      goto no_info;

>  

> @@ -333,9 +329,9 @@ get_process_info (int dfd, long int pid)

>  

>    int retval;

>    if (e_ident[EI_CLASS] == ELFCLASS32)

> -    retval = find_maps32 (pid, auxv, auxv_size);

> +    retval = find_maps32 (exe, memfd, pid, auxv, auxv_size);

>    else

> -    retval = find_maps64 (pid, auxv, auxv_size);

> +    retval = find_maps64 (exe, memfd, pid, auxv, auxv_size);

>  

>    free (auxv);

>    close (memfd);

> diff --git a/elf/tst-pldd.c b/elf/tst-pldd.c

> new file mode 100644

> index 0000000000..ed19cedd05

> --- /dev/null

> +++ b/elf/tst-pldd.c

> @@ -0,0 +1,118 @@

> +/* Basic tests for pldd program.

> +   Copyright (C) 2019 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 <stdio.h>

> +#include <string.h>

> +#include <unistd.h>

> +#include <stdint.h>

> +#include <libgen.h>

> +#include <stdbool.h>

> +

> +#include <array_length.h>

> +#include <gnu/lib-names.h>

> +

> +#include <support/subprocess.h>

> +#include <support/capture_subprocess.h>

> +#include <support/check.h>

> +

> +static void

> +target_process (void *arg)

> +{

> +  pause ();

> +}

> +

> +/* The test runs in a container because pldd does not support tracing

> +   a binary started by the loader iself (as with testrun.sh).  */

> +

> +static int

> +do_test (void)

> +{

> +  /* Create a copy of current test to check with pldd.  */

> +  struct support_subprocess target = support_subprocess (target_process, NULL);

> +

> +  /* Run 'pldd' on test subprocess.  */

> +  struct support_capture_subprocess pldd;

> +  {

> +    /* Three digits per byte plus null terminator.  */

> +    char pid[3 * sizeof (uint32_t) + 1];

> +    snprintf (pid, array_length (pid), "%d", target.pid);

> +

> +    const char prog[] = "/usr/bin/pldd";

> +

> +    pldd = support_capture_subprogram (prog,

> +      (char *const []) { (char *) prog, pid, NULL });

> +

> +    support_capture_subprocess_check (&pldd, "pldd", 0, sc_allow_stdout);

> +  }

> +

> +  /* Check 'pldd' output.  The test is expected to be linked against only

> +     loader and libc.  */

> +  {

> +    pid_t pid;

> +    char buffer[512];

> +#define STRINPUT(size) "%" # size "s"

> +

> +    FILE *out = fmemopen (pldd.out.buffer, pldd.out.length, "r");

> +    TEST_VERIFY (out != NULL);

> +

> +    /* First line is in the form of <pid>: <full path of executable>  */

> +    TEST_COMPARE (fscanf (out, "%u: " STRINPUT (512), &pid, buffer), 2);

> +

> +    TEST_COMPARE (pid, target.pid);

> +    TEST_COMPARE (strcmp (basename (buffer), "tst-pldd"), 0);

> +

> +    /* It expects only one loader and libc loaded by the program.  */

> +    bool interpreter_found = false, libc_found = false;

> +    while (fgets (buffer, array_length (buffer), out) != NULL)

> +      {

> +	/* Ignore vDSO.  */

> +        if (buffer[0] != '/')

> +	  continue;

> +

> +	/* Remove newline so baseline (buffer) can compare against the

> +	   LD_SO and LIBC_SO macros unmodified.  */

> +	if (buffer[strlen(buffer)-1] == '\n')

> +	  buffer[strlen(buffer)-1] = '\0';

> +

> +	if (strcmp (basename (buffer), LD_SO) == 0)

> +	  {

> +	    TEST_COMPARE (interpreter_found, false);

> +	    interpreter_found = true;

> +	    continue;

> +	  }

> +

> +	if (strcmp (basename (buffer), LIBC_SO) == 0)

> +	  {

> +	    TEST_COMPARE (libc_found, false);

> +	    libc_found = true;

> +	    continue;

> +	  }

> +      }

> +    TEST_COMPARE (interpreter_found, true);

> +    TEST_COMPARE (libc_found, true);

> +

> +    fclose (out);

> +  }

> +

> +  support_capture_subprocess_free (&pldd);

> +  support_process_terminate (&target);

> +

> +  return 0;

> +}

> +

> +#include <support/test-driver.c>

> diff --git a/elf/tst-pldd.root/tst-pldd.script b/elf/tst-pldd.root/tst-pldd.script

> new file mode 100644

> index 0000000000..5a197b2ed0

> --- /dev/null

> +++ b/elf/tst-pldd.root/tst-pldd.script

> @@ -0,0 +1 @@

> +cp $B/elf/pldd $I/bin/pldd

>
Carlos O'Donell April 23, 2019, 5:45 p.m. UTC | #2
On 4/17/19 10:42 AM, Adhemerval Zanella wrote:
> Changes from previous version:

> 

>    - Removal of more braces.

>    - Added a comment where the old code was removed to warn against looking

>      at l_libname.

>    - Adjusted test comment.

>    - Remove file artifact.

> 


If you remove elf/tst-pldd.root/tst-pldd.script, because it's not needed
because pldd is always installed as part of 'make install' in the testroot.root,
the OK for commit.

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



> ---


^^^ Should be mostly scissors and dashes for git am to exclude this part in a commit
review.

> 

> Since 9182aa67994 (Fix vDSO l_name for GDB's, BZ#387) the initial link_map

> for executable itself and loader will have both l_name and l_libname->name

> holding the same value due:

> 

>   elf/dl-object.c

> 

>   95   new->l_name = *realname ? realname : (char *) newname->name + libname_len - 1;

> 

> Since newname->name points to new->l_libname->name.

> 

> This leads to pldd to an infinite call at:

> 

>   elf/pldd-xx.c

> 

> 203     again:

> 204       while (1)

> 205         {

> 206           ssize_t n = pread64 (memfd, tmpbuf.data, tmpbuf.length, name_offset);

> 

> 228           /* Try the l_libname element.  */

> 229           struct E(libname_list) ln;

> 230           if (pread64 (memfd, &ln, sizeof (ln), m.l_libname) == sizeof (ln))

> 231             {

> 232               name_offset = ln.name;

> 233               goto again;

> 234             }

> 

> Since the value at ln.name (l_libname->name) will be the same as previously

> read. The straightforward fix is just avoid the check and read the new list

> entry.

> 

> I checked also against binaries issues with old loaders with fix for BZ#387,

> and pldd could dump the shared objects.

> 

> Checked on x86_64-linux-gnu, i686-linux-gnu, aarch64-linux-gnu, and

> powerpc64le-linux-gnu.

> 

> 	[BZ #18035]

> 	* elf/Makefile (tests-container): Add tst-pldd.

> 	* elf/pldd-xx.c: Use _Static_assert in of pldd_assert.

> 	(E(find_maps)): Avoid use alloca, use default read file operations

> 	instead of explicit LFS names, and fix infinite	loop.

> 	* elf/pldd.c: Explicit set _FILE_OFFSET_BITS, cleanup headers.

> 	(get_process_info): Use _Static_assert instead of assert, use default

> 	directory operations instead of explicit LFS names, and free some

> 	leadek pointers.

> 	* elf/tst-pldd.c: New file.

> 	* elf/tst-pldd.root/tst-pldd.script: Likewise.


OK.

> ---

>   elf/Makefile                      |   1 +

>   elf/pldd-xx.c                     | 114 ++++++++++-------------------

>   elf/pldd.c                        |  64 ++++++++--------

>   elf/tst-pldd.c                    | 118 ++++++++++++++++++++++++++++++

>   elf/tst-pldd.root/tst-pldd.script |   1 +

>   5 files changed, 190 insertions(+), 108 deletions(-)

>   create mode 100644 elf/tst-pldd.c

>   create mode 100644 elf/tst-pldd.root/tst-pldd.script

> 

> diff --git a/elf/Makefile b/elf/Makefile

> index 310a37cc13..0b4a877880 100644

> --- a/elf/Makefile

> +++ b/elf/Makefile

> @@ -194,6 +194,7 @@ tests-internal += loadtest unload unload2 circleload1 \

>   	 tst-tls3 tst-tls6 tst-tls7 tst-tls8 tst-dlmopen2 \

>   	 tst-ptrguard1 tst-stackguard1 tst-libc_dlvsym \

>   	 tst-create_format1

> +tests-container += tst-pldd


OK.

>   ifeq ($(build-hardcoded-path-in-tests),yes)

>   tests += tst-dlopen-aout

>   tst-dlopen-aout-no-pie = yes

> diff --git a/elf/pldd-xx.c b/elf/pldd-xx.c

> index 547f840ee1..28a8659935 100644

> --- a/elf/pldd-xx.c

> +++ b/elf/pldd-xx.c

> @@ -23,10 +23,6 @@

>   #define EW_(e, w, t) EW__(e, w, _##t)

>   #define EW__(e, w, t) e##w##t

>   

> -#define pldd_assert(name, exp) \

> -  typedef int __assert_##name[((exp) != 0) - 1]

> -

> -


OK.

>   struct E(link_map)

>   {

>     EW(Addr) l_addr;

> @@ -39,12 +35,12 @@ struct E(link_map)

>     EW(Addr) l_libname;

>   };

>   #if CLASS == __ELF_NATIVE_CLASS

> -pldd_assert (l_addr, (offsetof (struct link_map, l_addr)

> -			== offsetof (struct E(link_map), l_addr)));

> -pldd_assert (l_name, (offsetof (struct link_map, l_name)

> -			== offsetof (struct E(link_map), l_name)));

> -pldd_assert (l_next, (offsetof (struct link_map, l_next)

> -			== offsetof (struct E(link_map), l_next)));

> +_Static_assert (offsetof (struct link_map, l_addr)

> +		== offsetof (struct E(link_map), l_addr), "l_addr");

> +_Static_assert (offsetof (struct link_map, l_name)

> +		== offsetof (struct E(link_map), l_name), "l_name");

> +_Static_assert (offsetof (struct link_map, l_next)

> +		== offsetof (struct E(link_map), l_next), "l_next");


OK.

>   #endif

>   

>   

> @@ -54,10 +50,10 @@ struct E(libname_list)

>     EW(Addr) next;

>   };

>   #if CLASS == __ELF_NATIVE_CLASS

> -pldd_assert (name, (offsetof (struct libname_list, name)

> -		      == offsetof (struct E(libname_list), name)));

> -pldd_assert (next, (offsetof (struct libname_list, next)

> -		      == offsetof (struct E(libname_list), next)));

> +_Static_assert (offsetof (struct libname_list, name)

> +		== offsetof (struct E(libname_list), name), "name");

> +_Static_assert (offsetof (struct libname_list, next)

> +		== offsetof (struct E(libname_list), next), "next");


OK.

>   #endif

>   

>   struct E(r_debug)

> @@ -69,16 +65,17 @@ struct E(r_debug)

>     EW(Addr) r_map;

>   };

>   #if CLASS == __ELF_NATIVE_CLASS

> -pldd_assert (r_version, (offsetof (struct r_debug, r_version)

> -			   == offsetof (struct E(r_debug), r_version)));

> -pldd_assert (r_map, (offsetof (struct r_debug, r_map)

> -		       == offsetof (struct E(r_debug), r_map)));

> +_Static_assert (offsetof (struct r_debug, r_version)

> +		== offsetof (struct E(r_debug), r_version), "r_version");

> +_Static_assert (offsetof (struct r_debug, r_map)

> +		== offsetof (struct E(r_debug), r_map), "r_map");


OK.

>   #endif

>   

>   

>   static int

>   

> -E(find_maps) (pid_t pid, void *auxv, size_t auxv_size)

> +E(find_maps) (const char *exe, int memfd, pid_t pid, void *auxv,

> +	      size_t auxv_size)


OK.

>   {

>     EW(Addr) phdr = 0;

>     unsigned int phnum = 0;

> @@ -104,12 +101,9 @@ E(find_maps) (pid_t pid, void *auxv, size_t auxv_size)

>     if (phdr == 0 || phnum == 0 || phent == 0)

>       error (EXIT_FAILURE, 0, gettext ("cannot find program header of process"));

>   

> -  EW(Phdr) *p = alloca (phnum * phent);

> -  if (pread64 (memfd, p, phnum * phent, phdr) != phnum * phent)

> -    {

> -      error (0, 0, gettext ("cannot read program header"));

> -      return EXIT_FAILURE;

> -    }

> +  EW(Phdr) *p = xmalloc (phnum * phent);

> +  if (pread (memfd, p, phnum * phent, phdr) != phnum * phent)

> +    error (EXIT_FAILURE, 0, gettext ("cannot read program header"));


OK.

>   

>     /* Determine the load offset.  We need this for interpreting the

>        other program header entries so we do this in a separate loop.

> @@ -129,24 +123,18 @@ E(find_maps) (pid_t pid, void *auxv, size_t auxv_size)

>       if (p[i].p_type == PT_DYNAMIC)

>         {

>   	EW(Dyn) *dyn = xmalloc (p[i].p_filesz);

> -	if (pread64 (memfd, dyn, p[i].p_filesz, offset + p[i].p_vaddr)

> +	if (pread (memfd, dyn, p[i].p_filesz, offset + p[i].p_vaddr)


OK.

>   	    != p[i].p_filesz)

> -	  {

> -	    error (0, 0, gettext ("cannot read dynamic section"));

> -	    return EXIT_FAILURE;

> -	  }

> +	  error (EXIT_FAILURE, 0, gettext ("cannot read dynamic section"));


OK.

>   

>   	/* Search for the DT_DEBUG entry.  */

>   	for (unsigned int j = 0; j < p[i].p_filesz / sizeof (EW(Dyn)); ++j)

>   	  if (dyn[j].d_tag == DT_DEBUG && dyn[j].d_un.d_ptr != 0)

>   	    {

>   	      struct E(r_debug) r;

> -	      if (pread64 (memfd, &r, sizeof (r), dyn[j].d_un.d_ptr)

> +	      if (pread (memfd, &r, sizeof (r), dyn[j].d_un.d_ptr)


OK.

>   		  != sizeof (r))

> -		{

> -		  error (0, 0, gettext ("cannot read r_debug"));

> -		  return EXIT_FAILURE;

> -		}

> +		error (EXIT_FAILURE, 0, gettext ("cannot read r_debug"));


OK.

>   

>   	      if (r.r_map != 0)

>   		{

> @@ -160,13 +148,10 @@ E(find_maps) (pid_t pid, void *auxv, size_t auxv_size)

>         }

>       else if (p[i].p_type == PT_INTERP)

>         {

> -	interp = alloca (p[i].p_filesz);

> -	if (pread64 (memfd, interp, p[i].p_filesz, offset + p[i].p_vaddr)

> +	interp = xmalloc (p[i].p_filesz);

> +	if (pread (memfd, interp, p[i].p_filesz, offset + p[i].p_vaddr)


OK.

>   	    != p[i].p_filesz)

> -	  {

> -	    error (0, 0, gettext ("cannot read program interpreter"));

> -	    return EXIT_FAILURE;

> -	  }

> +	  error (EXIT_FAILURE, 0, gettext ("cannot read program interpreter"));


OK.

>         }

>   

>     if (list == 0)

> @@ -174,14 +159,16 @@ E(find_maps) (pid_t pid, void *auxv, size_t auxv_size)

>         if (interp == NULL)

>   	{

>   	  // XXX check whether the executable itself is the loader

> -	  return EXIT_FAILURE;

> +	  exit (EXIT_FAILURE);


OK.

>   	}

>   

>         // XXX perhaps try finding ld.so and _r_debug in it

> -

> -      return EXIT_FAILURE;

> +      exit (EXIT_FAILURE);


OK.

>       }

>   

> +  free (p);

> +  free (interp);


OK.

> +

>     /* Print the PID and program name first.  */

>     printf ("%lu:\t%s\n", (unsigned long int) pid, exe);

>   

> @@ -192,47 +179,27 @@ E(find_maps) (pid_t pid, void *auxv, size_t auxv_size)

>     do

>       {

>         struct E(link_map) m;

> -      if (pread64 (memfd, &m, sizeof (m), list) != sizeof (m))

> -	{

> -	  error (0, 0, gettext ("cannot read link map"));

> -	  status = EXIT_FAILURE;

> -	  goto out;

> -	}

> +      if (pread (memfd, &m, sizeof (m), list) != sizeof (m))

> +	error (EXIT_FAILURE, 0, gettext ("cannot read link map"));


OK.

>   

>         EW(Addr) name_offset = m.l_name;

> -    again:


OK.

>         while (1)

>   	{

> -	  ssize_t n = pread64 (memfd, tmpbuf.data, tmpbuf.length, name_offset);

> +	  ssize_t n = pread (memfd, tmpbuf.data, tmpbuf.length, name_offset);


OK.

>   	  if (n == -1)

> -	    {

> -	      error (0, 0, gettext ("cannot read object name"));

> -	      status = EXIT_FAILURE;

> -	      goto out;

> -	    }

> +	    error (EXIT_FAILURE, 0, gettext ("cannot read object name"));


OK.

>   

>   	  if (memchr (tmpbuf.data, '\0', n) != NULL)

>   	    break;

>   

>   	  if (!scratch_buffer_grow (&tmpbuf))

> -	    {

> -	      error (0, 0, gettext ("cannot allocate buffer for object name"));

> -	      status = EXIT_FAILURE;

> -	      goto out;

> -	    }

> +	    error (EXIT_FAILURE, 0,

> +		   gettext ("cannot allocate buffer for object name"));


OK.

>   	}

>   

> -      if (((char *)tmpbuf.data)[0] == '\0' && name_offset == m.l_name

> -	  && m.l_libname != 0)

> -	{

> -	  /* Try the l_libname element.  */

> -	  struct E(libname_list) ln;

> -	  if (pread64 (memfd, &ln, sizeof (ln), m.l_libname) == sizeof (ln))

> -	    {

> -	      name_offset = ln.name;

> -	      goto again;

> -	    }

> -	}

> +      /* The m.l_name and m.l_libname.name for loader linkmap points to same

> +	 values (since BZ#387 fix).  Trying to use l_libname name as the

> +	 shared object name might lead to an infinite loop (BZ#18035).  */


OK. Thanks for this! It's a nugget of gold for future maintainers.
This also simplifies the heuristics.

>   

>         /* Skip over the executable.  */

>         if (((char *)tmpbuf.data)[0] != '\0')

> @@ -242,7 +209,6 @@ E(find_maps) (pid_t pid, void *auxv, size_t auxv_size)

>       }

>     while (list != 0);

>   

> - out:


OK.

>     scratch_buffer_free (&tmpbuf);

>     return status;

>   }

> diff --git a/elf/pldd.c b/elf/pldd.c

> index f3fac4e487..69629bd5d2 100644

> --- a/elf/pldd.c

> +++ b/elf/pldd.c

> @@ -17,23 +17,17 @@

>      License along with the GNU C Library; if not, see

>      <http://www.gnu.org/licenses/>.  */

>   

> -#include <alloca.h>

> +#define _FILE_OFFSET_BITS 64

> +


OK.

>   #include <argp.h>

> -#include <assert.h>

>   #include <dirent.h>

> -#include <elf.h>

> -#include <errno.h>

>   #include <error.h>

>   #include <fcntl.h>

>   #include <libintl.h>

> -#include <link.h>

> -#include <stddef.h>

>   #include <stdio.h>

>   #include <stdlib.h>

> -#include <string.h>

>   #include <unistd.h>

>   #include <sys/ptrace.h>

> -#include <sys/stat.h>

>   #include <sys/wait.h>

>   #include <scratch_buffer.h>

>   

> @@ -76,14 +70,9 @@ static struct argp argp =

>     options, parse_opt, args_doc, doc, NULL, more_help, NULL

>   };

>   

> -// File descriptor of /proc/*/mem file.

> -static int memfd;

> -

> -/* Name of the executable  */

> -static char *exe;


OK.

>   

>   /* Local functions.  */

> -static int get_process_info (int dfd, long int pid);

> +static int get_process_info (const char *exe, int dfd, long int pid);


OK.

>   static void wait_for_ptrace_stop (long int pid);

>   

>   

> @@ -102,8 +91,10 @@ main (int argc, char *argv[])

>         return 1;

>       }

>   

> -  assert (sizeof (pid_t) == sizeof (int)

> -	  || sizeof (pid_t) == sizeof (long int));

> +  _Static_assert (sizeof (pid_t) == sizeof (int)

> +		  || sizeof (pid_t) == sizeof (long int),

> +		  "sizeof (pid_t) != sizeof (int) or sizeof (long int)");


OK.

> +

>     char *endp;

>     errno = 0;

>     long int pid = strtol (argv[remaining], &endp, 10);

> @@ -119,25 +110,24 @@ main (int argc, char *argv[])

>     if (dfd == -1)

>       error (EXIT_FAILURE, errno, gettext ("cannot open %s"), buf);

>   

> -  struct scratch_buffer exebuf;

> -  scratch_buffer_init (&exebuf);

> +  /* Name of the executable  */

> +  struct scratch_buffer exe;

> +  scratch_buffer_init (&exe);


OK.

>     ssize_t nexe;

>     while ((nexe = readlinkat (dfd, "exe",

> -			     exebuf.data, exebuf.length)) == exebuf.length)

> +			     exe.data, exe.length)) == exe.length)

>       {

> -      if (!scratch_buffer_grow (&exebuf))

> +      if (!scratch_buffer_grow (&exe))

>   	{

>   	  nexe = -1;

>   	  break;

>   	}

>       }

>     if (nexe == -1)

> -    exe = (char *) "<program name undetermined>";

> +    /* Default stack allocation is at least 1024.  */

> +    snprintf (exe.data, exe.length, "<program name undetermined>");


OK.

>     else

> -    {

> -      exe = exebuf.data;

> -      exe[nexe] = '\0';

> -    }

> +    ((char*)exe.data)[nexe] = '\0';


OK.

>   

>     /* Stop all threads since otherwise the list of loaded modules might

>        change while we are reading it.  */

> @@ -155,8 +145,8 @@ main (int argc, char *argv[])

>       error (EXIT_FAILURE, errno, gettext ("cannot prepare reading %s/task"),

>   	   buf);

>   

> -  struct dirent64 *d;

> -  while ((d = readdir64 (dir)) != NULL)

> +  struct dirent *d;

> +  while ((d = readdir (dir)) != NULL)


OK.

>       {

>         if (! isdigit (d->d_name[0]))

>   	continue;

> @@ -182,7 +172,7 @@ main (int argc, char *argv[])

>   

>         wait_for_ptrace_stop (tid);

>   

> -      struct thread_list *newp = alloca (sizeof (*newp));

> +      struct thread_list *newp = xmalloc (sizeof (*newp));


OK.

>         newp->tid = tid;

>         newp->next = thread_list;

>         thread_list = newp;

> @@ -190,17 +180,22 @@ main (int argc, char *argv[])

>   

>     closedir (dir);

>   

> -  int status = get_process_info (dfd, pid);

> +  if (thread_list == NULL)

> +    error (EXIT_FAILURE, 0, gettext ("no valid %s/task entries"), buf);

> +

> +  int status = get_process_info (exe.data, dfd, pid);


OK.

>   

> -  assert (thread_list != NULL);

>     do

>       {

>         ptrace (PTRACE_DETACH, thread_list->tid, NULL, NULL);

> +      struct thread_list *prev = thread_list;

>         thread_list = thread_list->next;

> +      free (prev);


OK.

>       }

>     while (thread_list != NULL);

>   

>     close (dfd);

> +  scratch_buffer_free (&exe);


OK.

>   

>     return status;

>   }

> @@ -281,9 +276,10 @@ warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.\n\

>   

>   

>   static int

> -get_process_info (int dfd, long int pid)

> +get_process_info (const char *exe, int dfd, long int pid)


OK.

>   {

> -  memfd = openat (dfd, "mem", O_RDONLY);

> +  /* File descriptor of /proc/<pid>/mem file.  */

> +  int memfd = openat (dfd, "mem", O_RDONLY);


OK.

>     if (memfd == -1)

>       goto no_info;

>   

> @@ -333,9 +329,9 @@ get_process_info (int dfd, long int pid)

>   

>     int retval;

>     if (e_ident[EI_CLASS] == ELFCLASS32)

> -    retval = find_maps32 (pid, auxv, auxv_size);

> +    retval = find_maps32 (exe, memfd, pid, auxv, auxv_size);

>     else

> -    retval = find_maps64 (pid, auxv, auxv_size);

> +    retval = find_maps64 (exe, memfd, pid, auxv, auxv_size);


OK.

>   

>     free (auxv);

>     close (memfd);

> diff --git a/elf/tst-pldd.c b/elf/tst-pldd.c

> new file mode 100644

> index 0000000000..ed19cedd05

> --- /dev/null

> +++ b/elf/tst-pldd.c

> @@ -0,0 +1,118 @@

> +/* Basic tests for pldd program.


OK.

> +   Copyright (C) 2019 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 <stdio.h>

> +#include <string.h>

> +#include <unistd.h>

> +#include <stdint.h>

> +#include <libgen.h>

> +#include <stdbool.h>

> +

> +#include <array_length.h>

> +#include <gnu/lib-names.h>

> +

> +#include <support/subprocess.h>

> +#include <support/capture_subprocess.h>

> +#include <support/check.h>

> +

> +static void

> +target_process (void *arg)

> +{

> +  pause ();

> +}

> +

> +/* The test runs in a container because pldd does not support tracing

> +   a binary started by the loader iself (as with testrun.sh).  */

> +

> +static int

> +do_test (void)

> +{

> +  /* Create a copy of current test to check with pldd.  */

> +  struct support_subprocess target = support_subprocess (target_process, NULL);

> +

> +  /* Run 'pldd' on test subprocess.  */

> +  struct support_capture_subprocess pldd;

> +  {

> +    /* Three digits per byte plus null terminator.  */

> +    char pid[3 * sizeof (uint32_t) + 1];

> +    snprintf (pid, array_length (pid), "%d", target.pid);

> +

> +    const char prog[] = "/usr/bin/pldd";

> +

> +    pldd = support_capture_subprogram (prog,

> +      (char *const []) { (char *) prog, pid, NULL });

> +

> +    support_capture_subprocess_check (&pldd, "pldd", 0, sc_allow_stdout);

> +  }

> +

> +  /* Check 'pldd' output.  The test is expected to be linked against only

> +     loader and libc.  */

> +  {

> +    pid_t pid;

> +    char buffer[512];

> +#define STRINPUT(size) "%" # size "s"

> +

> +    FILE *out = fmemopen (pldd.out.buffer, pldd.out.length, "r");

> +    TEST_VERIFY (out != NULL);

> +

> +    /* First line is in the form of <pid>: <full path of executable>  */

> +    TEST_COMPARE (fscanf (out, "%u: " STRINPUT (512), &pid, buffer), 2);

> +

> +    TEST_COMPARE (pid, target.pid);

> +    TEST_COMPARE (strcmp (basename (buffer), "tst-pldd"), 0);

> +

> +    /* It expects only one loader and libc loaded by the program.  */

> +    bool interpreter_found = false, libc_found = false;

> +    while (fgets (buffer, array_length (buffer), out) != NULL)

> +      {

> +	/* Ignore vDSO.  */

> +        if (buffer[0] != '/')

> +	  continue;

> +

> +	/* Remove newline so baseline (buffer) can compare against the

> +	   LD_SO and LIBC_SO macros unmodified.  */

> +	if (buffer[strlen(buffer)-1] == '\n')

> +	  buffer[strlen(buffer)-1] = '\0';

> +

> +	if (strcmp (basename (buffer), LD_SO) == 0)

> +	  {

> +	    TEST_COMPARE (interpreter_found, false);

> +	    interpreter_found = true;

> +	    continue;

> +	  }

> +

> +	if (strcmp (basename (buffer), LIBC_SO) == 0)

> +	  {

> +	    TEST_COMPARE (libc_found, false);

> +	    libc_found = true;

> +	    continue;

> +	  }

> +      }

> +    TEST_COMPARE (interpreter_found, true);

> +    TEST_COMPARE (libc_found, true);

> +

> +    fclose (out);

> +  }

> +

> +  support_capture_subprocess_free (&pldd);

> +  support_process_terminate (&target);

> +

> +  return 0;

> +}

> +

> +#include <support/test-driver.c>

> diff --git a/elf/tst-pldd.root/tst-pldd.script b/elf/tst-pldd.root/tst-pldd.script

> new file mode 100644

> index 0000000000..5a197b2ed0

> --- /dev/null

> +++ b/elf/tst-pldd.root/tst-pldd.script

> @@ -0,0 +1 @@

> +cp $B/elf/pldd $I/bin/pldd


This is not required. pldd is already installed in the testroot.root by the
test-in-container setup process. You don't need a *.root directory for this test.

> 



-- 
Cheers,
Carlos.
Rafal Luzynski April 24, 2019, 11:14 p.m. UTC | #3
17.04.2019 16:42 Adhemerval Zanella <adhemerval.zanella@linaro.org> wrote:
> [...]

> 	[BZ #18035]

> 	* elf/Makefile (tests-container): Add tst-pldd.

> 	* elf/pldd-xx.c: Use _Static_assert in of pldd_assert.

> 	(E(find_maps)): Avoid use alloca, use default read file operations

> 	instead of explicit LFS names, and fix infinite	loop.

> 	* elf/pldd.c: Explicit set _FILE_OFFSET_BITS, cleanup headers.

> 	(get_process_info): Use _Static_assert instead of assert, use default

> 	directory operations instead of explicit LFS names, and free some

> 	leadek pointers.

> 	* elf/tst-pldd.c: New file.


This test (elf/tst-pldd.c) always times out for me, even with some large
timeout factors.  Having read the description of the bug and the patch
I believe it's not a simple timeout but a permanent hang instead.  Also
please see below:

> [...]

> diff --git a/elf/tst-pldd.c b/elf/tst-pldd.c

> new file mode 100644

> index 0000000000..ed19cedd05

> --- /dev/null

> +++ b/elf/tst-pldd.c

> @@ -0,0 +1,118 @@

> + [...]

> +static int

> +do_test (void)

> +{

> +  /* Create a copy of current test to check with pldd.  */

> +  struct support_subprocess target = support_subprocess (target_process,

> NULL);

> +

> +  /* Run 'pldd' on test subprocess.  */

> +  struct support_capture_subprocess pldd;

> +  {

> +    /* Three digits per byte plus null terminator.  */

> +    char pid[3 * sizeof (uint32_t) + 1];

> +    snprintf (pid, array_length (pid), "%d", target.pid);

> +

> +    const char prog[] = "/usr/bin/pldd";


My guess is that the reason is that it launches and tests the system
installed /usr/bin/pldd which most likely contains the bug which is fixed
by this patch.  Would you consider replacing "/usr/bin/pldd" with "./pldd"
or whatever path is required to use the correct pldd?

Thanks and regards,

Rafal
Florian Weimer April 25, 2019, 4:51 a.m. UTC | #4
* Rafal Luzynski:

> 17.04.2019 16:42 Adhemerval Zanella <adhemerval.zanella@linaro.org> wrote:

>> [...]

>> 	[BZ #18035]

>> 	* elf/Makefile (tests-container): Add tst-pldd.


>> +    const char prog[] = "/usr/bin/pldd";

>

> My guess is that the reason is that it launches and tests the system

> installed /usr/bin/pldd which most likely contains the bug which is fixed

> by this patch.  Would you consider replacing "/usr/bin/pldd" with "./pldd"

> or whatever path is required to use the correct pldd?


Do you run the test using the test-in-container framework?
It's supposed to run in a chroot.

As discussed before, the makefile dependencies for the chroot generation
are incorrect/incomplete, so perhaps pldd in the chroot was not updated
properly.

Thanks,
Florian
Rafal Luzynski April 25, 2019, 9:09 a.m. UTC | #5
25.04.2019 06:51 Florian Weimer <fweimer@redhat.com> wrote:
> [...]

> Do you run the test using the test-in-container framework?


No.

> It's supposed to run in a chroot.


Sounds reasonable that it would work fine in a chroot environment.
What about "make check", is it obsolete?  Maybe this test should
be skipped if executed outside the test-in-container framework?

Regards,

Rafal
Florian Weimer April 25, 2019, 9:17 a.m. UTC | #6
* Rafal Luzynski:

> 25.04.2019 06:51 Florian Weimer <fweimer@redhat.com> wrote:

>> [...]

>> Do you run the test using the test-in-container framework?

>

> No.


Well, that explains it.

>> It's supposed to run in a chroot.

>

> Sounds reasonable that it would work fine in a chroot environment.

> What about "make check", is it obsolete?  Maybe this test should

> be skipped if executed outside the test-in-container framework?


“make check” is supposed to take care of all that, and run these tests
in a chroot if it's possible to create one.

Thanks,
Florian
Adhemerval Zanella Netto April 25, 2019, 12:09 p.m. UTC | #7
On 25/04/2019 06:17, Florian Weimer wrote:
> * Rafal Luzynski:

> 

>> 25.04.2019 06:51 Florian Weimer <fweimer@redhat.com> wrote:

>>> [...]

>>> Do you run the test using the test-in-container framework?

>>

>> No.

> 

> Well, that explains it.

> 

>>> It's supposed to run in a chroot.

>>

>> Sounds reasonable that it would work fine in a chroot environment.

>> What about "make check", is it obsolete?  Maybe this test should

>> be skipped if executed outside the test-in-container framework?

> 

> “make check” is supposed to take care of all that, and run these tests

> in a chroot if it's possible to create one.

> 


Are you seeing hangouts while running with test-in-container framework 
as well? 

> As discussed before, the makefile dependencies for the chroot generation

> are incorrect/incomplete, so perhaps pldd in the chroot was not updated

> properly.


I think I missed this discussion, what is missing here?
Florian Weimer April 25, 2019, 12:22 p.m. UTC | #8
* Adhemerval Zanella:

>> As discussed before, the makefile dependencies for the chroot generation

>> are incorrect/incomplete, so perhaps pldd in the chroot was not updated

>> properly.

>

> I think I missed this discussion, what is missing here?


Maybe I misunderstood things, but this is what I took away from this
discussion:

  How to debug test-in-container failures?
  <https://sourceware.org/ml/libc-alpha/2019-02/msg00267.html>

Thanks,
Florian
H.J. Lu April 30, 2019, 4:27 p.m. UTC | #9
On Thu, Apr 25, 2019 at 5:22 AM Florian Weimer <fweimer@redhat.com> wrote:
>

> * Adhemerval Zanella:

>

> >> As discussed before, the makefile dependencies for the chroot generation

> >> are incorrect/incomplete, so perhaps pldd in the chroot was not updated

> >> properly.

> >

> > I think I missed this discussion, what is missing here?

>

> Maybe I misunderstood things, but this is what I took away from this

> discussion:

>

>   How to debug test-in-container failures?

>   <https://sourceware.org/ml/libc-alpha/2019-02/msg00267.html>

>


I opened:

https://sourceware.org/bugzilla/show_bug.cgi?id=24506

-- 
H.J.
Rafal Luzynski May 8, 2019, 10:42 p.m. UTC | #10
25.04.2019 11:17 Florian Weimer <fweimer@redhat.com> wrote:
> 

> * Rafal Luzynski:

> 

> > 25.04.2019 06:51 Florian Weimer <fweimer@redhat.com> wrote:

> >> [...]

> >> Do you run the test using the test-in-container framework?

> >

> > No.

> 

> Well, that explains it.


Looking at your backports [1] [2] "(Backported without the test case..."
I understand that glibc development is possible only on rather new system
so I am moving to a newer machine.  This works good in Fedora 30.

Regards,

Rafal


[1] https://sourceware.org/ml/libc-stable/2019-04/msg00010.html
[2] https://sourceware.org/ml/libc-stable/2019-04/msg00011.html
diff mbox series

Patch

diff --git a/elf/Makefile b/elf/Makefile
index 310a37cc13..0b4a877880 100644
--- a/elf/Makefile
+++ b/elf/Makefile
@@ -194,6 +194,7 @@  tests-internal += loadtest unload unload2 circleload1 \
 	 tst-tls3 tst-tls6 tst-tls7 tst-tls8 tst-dlmopen2 \
 	 tst-ptrguard1 tst-stackguard1 tst-libc_dlvsym \
 	 tst-create_format1
+tests-container += tst-pldd
 ifeq ($(build-hardcoded-path-in-tests),yes)
 tests += tst-dlopen-aout
 tst-dlopen-aout-no-pie = yes
diff --git a/elf/pldd-xx.c b/elf/pldd-xx.c
index 547f840ee1..28a8659935 100644
--- a/elf/pldd-xx.c
+++ b/elf/pldd-xx.c
@@ -23,10 +23,6 @@ 
 #define EW_(e, w, t) EW__(e, w, _##t)
 #define EW__(e, w, t) e##w##t
 
-#define pldd_assert(name, exp) \
-  typedef int __assert_##name[((exp) != 0) - 1]
-
-
 struct E(link_map)
 {
   EW(Addr) l_addr;
@@ -39,12 +35,12 @@  struct E(link_map)
   EW(Addr) l_libname;
 };
 #if CLASS == __ELF_NATIVE_CLASS
-pldd_assert (l_addr, (offsetof (struct link_map, l_addr)
-			== offsetof (struct E(link_map), l_addr)));
-pldd_assert (l_name, (offsetof (struct link_map, l_name)
-			== offsetof (struct E(link_map), l_name)));
-pldd_assert (l_next, (offsetof (struct link_map, l_next)
-			== offsetof (struct E(link_map), l_next)));
+_Static_assert (offsetof (struct link_map, l_addr)
+		== offsetof (struct E(link_map), l_addr), "l_addr");
+_Static_assert (offsetof (struct link_map, l_name)
+		== offsetof (struct E(link_map), l_name), "l_name");
+_Static_assert (offsetof (struct link_map, l_next)
+		== offsetof (struct E(link_map), l_next), "l_next");
 #endif
 
 
@@ -54,10 +50,10 @@  struct E(libname_list)
   EW(Addr) next;
 };
 #if CLASS == __ELF_NATIVE_CLASS
-pldd_assert (name, (offsetof (struct libname_list, name)
-		      == offsetof (struct E(libname_list), name)));
-pldd_assert (next, (offsetof (struct libname_list, next)
-		      == offsetof (struct E(libname_list), next)));
+_Static_assert (offsetof (struct libname_list, name)
+		== offsetof (struct E(libname_list), name), "name");
+_Static_assert (offsetof (struct libname_list, next)
+		== offsetof (struct E(libname_list), next), "next");
 #endif
 
 struct E(r_debug)
@@ -69,16 +65,17 @@  struct E(r_debug)
   EW(Addr) r_map;
 };
 #if CLASS == __ELF_NATIVE_CLASS
-pldd_assert (r_version, (offsetof (struct r_debug, r_version)
-			   == offsetof (struct E(r_debug), r_version)));
-pldd_assert (r_map, (offsetof (struct r_debug, r_map)
-		       == offsetof (struct E(r_debug), r_map)));
+_Static_assert (offsetof (struct r_debug, r_version)
+		== offsetof (struct E(r_debug), r_version), "r_version");
+_Static_assert (offsetof (struct r_debug, r_map)
+		== offsetof (struct E(r_debug), r_map), "r_map");
 #endif
 
 
 static int
 
-E(find_maps) (pid_t pid, void *auxv, size_t auxv_size)
+E(find_maps) (const char *exe, int memfd, pid_t pid, void *auxv,
+	      size_t auxv_size)
 {
   EW(Addr) phdr = 0;
   unsigned int phnum = 0;
@@ -104,12 +101,9 @@  E(find_maps) (pid_t pid, void *auxv, size_t auxv_size)
   if (phdr == 0 || phnum == 0 || phent == 0)
     error (EXIT_FAILURE, 0, gettext ("cannot find program header of process"));
 
-  EW(Phdr) *p = alloca (phnum * phent);
-  if (pread64 (memfd, p, phnum * phent, phdr) != phnum * phent)
-    {
-      error (0, 0, gettext ("cannot read program header"));
-      return EXIT_FAILURE;
-    }
+  EW(Phdr) *p = xmalloc (phnum * phent);
+  if (pread (memfd, p, phnum * phent, phdr) != phnum * phent)
+    error (EXIT_FAILURE, 0, gettext ("cannot read program header"));
 
   /* Determine the load offset.  We need this for interpreting the
      other program header entries so we do this in a separate loop.
@@ -129,24 +123,18 @@  E(find_maps) (pid_t pid, void *auxv, size_t auxv_size)
     if (p[i].p_type == PT_DYNAMIC)
       {
 	EW(Dyn) *dyn = xmalloc (p[i].p_filesz);
-	if (pread64 (memfd, dyn, p[i].p_filesz, offset + p[i].p_vaddr)
+	if (pread (memfd, dyn, p[i].p_filesz, offset + p[i].p_vaddr)
 	    != p[i].p_filesz)
-	  {
-	    error (0, 0, gettext ("cannot read dynamic section"));
-	    return EXIT_FAILURE;
-	  }
+	  error (EXIT_FAILURE, 0, gettext ("cannot read dynamic section"));
 
 	/* Search for the DT_DEBUG entry.  */
 	for (unsigned int j = 0; j < p[i].p_filesz / sizeof (EW(Dyn)); ++j)
 	  if (dyn[j].d_tag == DT_DEBUG && dyn[j].d_un.d_ptr != 0)
 	    {
 	      struct E(r_debug) r;
-	      if (pread64 (memfd, &r, sizeof (r), dyn[j].d_un.d_ptr)
+	      if (pread (memfd, &r, sizeof (r), dyn[j].d_un.d_ptr)
 		  != sizeof (r))
-		{
-		  error (0, 0, gettext ("cannot read r_debug"));
-		  return EXIT_FAILURE;
-		}
+		error (EXIT_FAILURE, 0, gettext ("cannot read r_debug"));
 
 	      if (r.r_map != 0)
 		{
@@ -160,13 +148,10 @@  E(find_maps) (pid_t pid, void *auxv, size_t auxv_size)
       }
     else if (p[i].p_type == PT_INTERP)
       {
-	interp = alloca (p[i].p_filesz);
-	if (pread64 (memfd, interp, p[i].p_filesz, offset + p[i].p_vaddr)
+	interp = xmalloc (p[i].p_filesz);
+	if (pread (memfd, interp, p[i].p_filesz, offset + p[i].p_vaddr)
 	    != p[i].p_filesz)
-	  {
-	    error (0, 0, gettext ("cannot read program interpreter"));
-	    return EXIT_FAILURE;
-	  }
+	  error (EXIT_FAILURE, 0, gettext ("cannot read program interpreter"));
       }
 
   if (list == 0)
@@ -174,14 +159,16 @@  E(find_maps) (pid_t pid, void *auxv, size_t auxv_size)
       if (interp == NULL)
 	{
 	  // XXX check whether the executable itself is the loader
-	  return EXIT_FAILURE;
+	  exit (EXIT_FAILURE);
 	}
 
       // XXX perhaps try finding ld.so and _r_debug in it
-
-      return EXIT_FAILURE;
+      exit (EXIT_FAILURE);
     }
 
+  free (p);
+  free (interp);
+
   /* Print the PID and program name first.  */
   printf ("%lu:\t%s\n", (unsigned long int) pid, exe);
 
@@ -192,47 +179,27 @@  E(find_maps) (pid_t pid, void *auxv, size_t auxv_size)
   do
     {
       struct E(link_map) m;
-      if (pread64 (memfd, &m, sizeof (m), list) != sizeof (m))
-	{
-	  error (0, 0, gettext ("cannot read link map"));
-	  status = EXIT_FAILURE;
-	  goto out;
-	}
+      if (pread (memfd, &m, sizeof (m), list) != sizeof (m))
+	error (EXIT_FAILURE, 0, gettext ("cannot read link map"));
 
       EW(Addr) name_offset = m.l_name;
-    again:
       while (1)
 	{
-	  ssize_t n = pread64 (memfd, tmpbuf.data, tmpbuf.length, name_offset);
+	  ssize_t n = pread (memfd, tmpbuf.data, tmpbuf.length, name_offset);
 	  if (n == -1)
-	    {
-	      error (0, 0, gettext ("cannot read object name"));
-	      status = EXIT_FAILURE;
-	      goto out;
-	    }
+	    error (EXIT_FAILURE, 0, gettext ("cannot read object name"));
 
 	  if (memchr (tmpbuf.data, '\0', n) != NULL)
 	    break;
 
 	  if (!scratch_buffer_grow (&tmpbuf))
-	    {
-	      error (0, 0, gettext ("cannot allocate buffer for object name"));
-	      status = EXIT_FAILURE;
-	      goto out;
-	    }
+	    error (EXIT_FAILURE, 0,
+		   gettext ("cannot allocate buffer for object name"));
 	}
 
-      if (((char *)tmpbuf.data)[0] == '\0' && name_offset == m.l_name
-	  && m.l_libname != 0)
-	{
-	  /* Try the l_libname element.  */
-	  struct E(libname_list) ln;
-	  if (pread64 (memfd, &ln, sizeof (ln), m.l_libname) == sizeof (ln))
-	    {
-	      name_offset = ln.name;
-	      goto again;
-	    }
-	}
+      /* The m.l_name and m.l_libname.name for loader linkmap points to same
+	 values (since BZ#387 fix).  Trying to use l_libname name as the
+	 shared object name might lead to an infinite loop (BZ#18035).  */ 
 
       /* Skip over the executable.  */
       if (((char *)tmpbuf.data)[0] != '\0')
@@ -242,7 +209,6 @@  E(find_maps) (pid_t pid, void *auxv, size_t auxv_size)
     }
   while (list != 0);
 
- out:
   scratch_buffer_free (&tmpbuf);
   return status;
 }
diff --git a/elf/pldd.c b/elf/pldd.c
index f3fac4e487..69629bd5d2 100644
--- a/elf/pldd.c
+++ b/elf/pldd.c
@@ -17,23 +17,17 @@ 
    License along with the GNU C Library; if not, see
    <http://www.gnu.org/licenses/>.  */
 
-#include <alloca.h>
+#define _FILE_OFFSET_BITS 64
+
 #include <argp.h>
-#include <assert.h>
 #include <dirent.h>
-#include <elf.h>
-#include <errno.h>
 #include <error.h>
 #include <fcntl.h>
 #include <libintl.h>
-#include <link.h>
-#include <stddef.h>
 #include <stdio.h>
 #include <stdlib.h>
-#include <string.h>
 #include <unistd.h>
 #include <sys/ptrace.h>
-#include <sys/stat.h>
 #include <sys/wait.h>
 #include <scratch_buffer.h>
 
@@ -76,14 +70,9 @@  static struct argp argp =
   options, parse_opt, args_doc, doc, NULL, more_help, NULL
 };
 
-// File descriptor of /proc/*/mem file.
-static int memfd;
-
-/* Name of the executable  */
-static char *exe;
 
 /* Local functions.  */
-static int get_process_info (int dfd, long int pid);
+static int get_process_info (const char *exe, int dfd, long int pid);
 static void wait_for_ptrace_stop (long int pid);
 
 
@@ -102,8 +91,10 @@  main (int argc, char *argv[])
       return 1;
     }
 
-  assert (sizeof (pid_t) == sizeof (int)
-	  || sizeof (pid_t) == sizeof (long int));
+  _Static_assert (sizeof (pid_t) == sizeof (int)
+		  || sizeof (pid_t) == sizeof (long int),
+		  "sizeof (pid_t) != sizeof (int) or sizeof (long int)");
+
   char *endp;
   errno = 0;
   long int pid = strtol (argv[remaining], &endp, 10);
@@ -119,25 +110,24 @@  main (int argc, char *argv[])
   if (dfd == -1)
     error (EXIT_FAILURE, errno, gettext ("cannot open %s"), buf);
 
-  struct scratch_buffer exebuf;
-  scratch_buffer_init (&exebuf);
+  /* Name of the executable  */
+  struct scratch_buffer exe;
+  scratch_buffer_init (&exe);
   ssize_t nexe;
   while ((nexe = readlinkat (dfd, "exe",
-			     exebuf.data, exebuf.length)) == exebuf.length)
+			     exe.data, exe.length)) == exe.length)
     {
-      if (!scratch_buffer_grow (&exebuf))
+      if (!scratch_buffer_grow (&exe))
 	{
 	  nexe = -1;
 	  break;
 	}
     }
   if (nexe == -1)
-    exe = (char *) "<program name undetermined>";
+    /* Default stack allocation is at least 1024.  */
+    snprintf (exe.data, exe.length, "<program name undetermined>");
   else
-    {
-      exe = exebuf.data;
-      exe[nexe] = '\0';
-    }
+    ((char*)exe.data)[nexe] = '\0';
 
   /* Stop all threads since otherwise the list of loaded modules might
      change while we are reading it.  */
@@ -155,8 +145,8 @@  main (int argc, char *argv[])
     error (EXIT_FAILURE, errno, gettext ("cannot prepare reading %s/task"),
 	   buf);
 
-  struct dirent64 *d;
-  while ((d = readdir64 (dir)) != NULL)
+  struct dirent *d;
+  while ((d = readdir (dir)) != NULL)
     {
       if (! isdigit (d->d_name[0]))
 	continue;
@@ -182,7 +172,7 @@  main (int argc, char *argv[])
 
       wait_for_ptrace_stop (tid);
 
-      struct thread_list *newp = alloca (sizeof (*newp));
+      struct thread_list *newp = xmalloc (sizeof (*newp));
       newp->tid = tid;
       newp->next = thread_list;
       thread_list = newp;
@@ -190,17 +180,22 @@  main (int argc, char *argv[])
 
   closedir (dir);
 
-  int status = get_process_info (dfd, pid);
+  if (thread_list == NULL)
+    error (EXIT_FAILURE, 0, gettext ("no valid %s/task entries"), buf);
+
+  int status = get_process_info (exe.data, dfd, pid);
 
-  assert (thread_list != NULL);
   do
     {
       ptrace (PTRACE_DETACH, thread_list->tid, NULL, NULL);
+      struct thread_list *prev = thread_list;
       thread_list = thread_list->next;
+      free (prev);
     }
   while (thread_list != NULL);
 
   close (dfd);
+  scratch_buffer_free (&exe);
 
   return status;
 }
@@ -281,9 +276,10 @@  warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.\n\
 
 
 static int
-get_process_info (int dfd, long int pid)
+get_process_info (const char *exe, int dfd, long int pid)
 {
-  memfd = openat (dfd, "mem", O_RDONLY);
+  /* File descriptor of /proc/<pid>/mem file.  */
+  int memfd = openat (dfd, "mem", O_RDONLY);
   if (memfd == -1)
     goto no_info;
 
@@ -333,9 +329,9 @@  get_process_info (int dfd, long int pid)
 
   int retval;
   if (e_ident[EI_CLASS] == ELFCLASS32)
-    retval = find_maps32 (pid, auxv, auxv_size);
+    retval = find_maps32 (exe, memfd, pid, auxv, auxv_size);
   else
-    retval = find_maps64 (pid, auxv, auxv_size);
+    retval = find_maps64 (exe, memfd, pid, auxv, auxv_size);
 
   free (auxv);
   close (memfd);
diff --git a/elf/tst-pldd.c b/elf/tst-pldd.c
new file mode 100644
index 0000000000..ed19cedd05
--- /dev/null
+++ b/elf/tst-pldd.c
@@ -0,0 +1,118 @@ 
+/* Basic tests for pldd program.
+   Copyright (C) 2019 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 <stdio.h>
+#include <string.h>
+#include <unistd.h>
+#include <stdint.h>
+#include <libgen.h>
+#include <stdbool.h>
+
+#include <array_length.h>
+#include <gnu/lib-names.h>
+
+#include <support/subprocess.h>
+#include <support/capture_subprocess.h>
+#include <support/check.h>
+
+static void
+target_process (void *arg)
+{
+  pause ();
+}
+
+/* The test runs in a container because pldd does not support tracing
+   a binary started by the loader iself (as with testrun.sh).  */
+
+static int
+do_test (void)
+{
+  /* Create a copy of current test to check with pldd.  */
+  struct support_subprocess target = support_subprocess (target_process, NULL);
+
+  /* Run 'pldd' on test subprocess.  */
+  struct support_capture_subprocess pldd;
+  {
+    /* Three digits per byte plus null terminator.  */
+    char pid[3 * sizeof (uint32_t) + 1];
+    snprintf (pid, array_length (pid), "%d", target.pid);
+
+    const char prog[] = "/usr/bin/pldd";
+
+    pldd = support_capture_subprogram (prog,
+      (char *const []) { (char *) prog, pid, NULL });
+
+    support_capture_subprocess_check (&pldd, "pldd", 0, sc_allow_stdout);
+  }
+
+  /* Check 'pldd' output.  The test is expected to be linked against only
+     loader and libc.  */
+  {
+    pid_t pid;
+    char buffer[512];
+#define STRINPUT(size) "%" # size "s"
+
+    FILE *out = fmemopen (pldd.out.buffer, pldd.out.length, "r");
+    TEST_VERIFY (out != NULL);
+
+    /* First line is in the form of <pid>: <full path of executable>  */
+    TEST_COMPARE (fscanf (out, "%u: " STRINPUT (512), &pid, buffer), 2);
+
+    TEST_COMPARE (pid, target.pid);
+    TEST_COMPARE (strcmp (basename (buffer), "tst-pldd"), 0);
+
+    /* It expects only one loader and libc loaded by the program.  */
+    bool interpreter_found = false, libc_found = false;
+    while (fgets (buffer, array_length (buffer), out) != NULL)
+      {
+	/* Ignore vDSO.  */
+        if (buffer[0] != '/')
+	  continue;
+
+	/* Remove newline so baseline (buffer) can compare against the
+	   LD_SO and LIBC_SO macros unmodified.  */
+	if (buffer[strlen(buffer)-1] == '\n')
+	  buffer[strlen(buffer)-1] = '\0';
+
+	if (strcmp (basename (buffer), LD_SO) == 0)
+	  {
+	    TEST_COMPARE (interpreter_found, false);
+	    interpreter_found = true;
+	    continue;
+	  }
+
+	if (strcmp (basename (buffer), LIBC_SO) == 0)
+	  {
+	    TEST_COMPARE (libc_found, false);
+	    libc_found = true;
+	    continue;
+	  }
+      }
+    TEST_COMPARE (interpreter_found, true);
+    TEST_COMPARE (libc_found, true);
+
+    fclose (out);
+  }
+
+  support_capture_subprocess_free (&pldd);
+  support_process_terminate (&target);
+
+  return 0;
+}
+
+#include <support/test-driver.c>
diff --git a/elf/tst-pldd.root/tst-pldd.script b/elf/tst-pldd.root/tst-pldd.script
new file mode 100644
index 0000000000..5a197b2ed0
--- /dev/null
+++ b/elf/tst-pldd.root/tst-pldd.script
@@ -0,0 +1 @@ 
+cp $B/elf/pldd $I/bin/pldd