Bug 18035 - pldd does no longer work, enters infinite loop
Summary: pldd does no longer work, enters infinite loop
Status: RESOLVED FIXED
Alias: None
Product: glibc
Classification: Unclassified
Component: dynamic-link (show other bugs)
Version: 2.19
: P2 normal
Target Milestone: 2.30
Assignee: Adhemerval Zanella
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-02-26 14:18 UTC by Florian Weimer
Modified: 2019-04-26 12:43 UTC (History)
5 users (show)

See Also:
Host:
Target:
Build:
Last reconfirmed:
fweimer: security-


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Florian Weimer 2015-02-26 14:18:47 UTC
This just hangs:

$ pldd $$
17928:	/usr/bin/bash

It loops around in pldd-xx.c, here (line numbers are from glibc 2.20 in Fedora 21):

201	    again:
202	      while (1)
203		{
204		  ssize_t n = pread64 (memfd, str, strsize, name_offset);
205		  if (n == -1)
206		    {
207		      error (0, 0, gettext ("cannot read object name"));
208		      return EXIT_FAILURE;
209		    }
210	
211		  if (memchr (str, '\0', n) != NULL)
212		    break;
213	
214		  str = extend_alloca (str, strsize, strsize * 2);
215		}
216	
217	      if (str[0] == '\0' && name_offset == m.l_name
218		  && m.l_libname != 0)
219		{
220		  /* Try the l_libname element.  */
221		  struct E(libname_list) ln;
222		  if (pread64 (memfd, &ln, sizeof (ln), m.l_libname) == sizeof (ln))
223		    {
224		      name_offset = ln.name;
225		      goto again;
226		    }
227		}

(I see a similar issue in master.)
Comment 1 Andreas Schwab 2015-02-26 17:56:16 UTC
Already broken in 2.19.
Comment 2 Carlos O'Donell 2016-07-29 20:06:10 UTC
I see no reason for keeping pldd.

This does exactly the same:

cat >> pldd.sh <<EOF
#!/bin/sh
gdb -ex "set confirm off" -ex "set height 0" -ex "info shared" -ex "quit" -p $1 | grep '^0x.*0x'
EOF

If we had a dynamic linker library we could use to reuse all of the code that goes into loading a shared library, then that would be fine.

As it stands today, pldd just isn't that useful. The fact that it's broken since 2.19 and nobody complained shows it's not used.

I would suggest we remove pldd rather than fixing whatever assumption it has that's wrong.
Comment 3 Yaakov Selkowitz 2016-08-29 22:22:34 UTC
Is https://lkml.org/lkml/2013/7/23/163 related?
Comment 4 Florian Weimer 2016-08-30 07:46:10 UTC
(In reply to Yaakov Selkowitz from comment #3)
> Is https://lkml.org/lkml/2013/7/23/163 related?

I don't think so.  The process attach is successful in the hangs I see (ptrace (PTRACE_ATTACH) returns 0), it really loops around pread64.
Comment 5 richard@wetafx.co.nz 2019-01-15 01:54:57 UTC
What is the current intention for addressing this bug request?
Comment 6 cvs-commit@gcc.gnu.org 2019-04-23 21:13:53 UTC
The master branch has been updated by Adhemerval Zanella <azanella@sourceware.org>:

https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=1a4c27355e146b6d8cc6487b998462c7fdd1048f

commit 1a4c27355e146b6d8cc6487b998462c7fdd1048f
Author: Adhemerval Zanella <adhemerval.zanella@linaro.org>
Date:   Thu Apr 11 18:12:00 2019 -0300

    elf: Fix pldd (BZ#18035)
    
    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.
Comment 7 Adhemerval Zanella 2019-04-23 21:14:44 UTC
Fixed on 2.30 (commit 1a4c27355e146b6d8cc6487b998462c7fdd1048f).
Comment 8 cvs-commit@gcc.gnu.org 2019-04-26 12:42:42 UTC
The release/2.29/master branch has been updated by Florian Weimer <fw@sourceware.org>:

https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=eaea1dfbe95a31c29adc259100569962cddb6f19

commit eaea1dfbe95a31c29adc259100569962cddb6f19
Author: Adhemerval Zanella <adhemerval.zanella@linaro.org>
Date:   Fri Apr 26 13:58:31 2019 +0200

    elf: Fix pldd (BZ#18035)
    
    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.
    
    (cherry picked from commit 1a4c27355e146b6d8cc6487b998462c7fdd1048f)
Comment 9 cvs-commit@gcc.gnu.org 2019-04-26 12:43:02 UTC
The release/2.28/master branch has been updated by Florian Weimer <fw@sourceware.org>:

https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=5cbb73004b635e762e20b447c2d93c307cb40f41

commit 5cbb73004b635e762e20b447c2d93c307cb40f41
Author: Adhemerval Zanella <adhemerval.zanella@linaro.org>
Date:   Fri Apr 26 14:08:20 2019 +0200

    elf: Fix pldd (BZ#18035)
    
    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.
    
    (cherry picked from commit 1a4c27355e146b6d8cc6487b998462c7fdd1048f)
    (Backported without the test case due to lack of test-in-container
    support.)
Comment 10 cvs-commit@gcc.gnu.org 2019-04-26 12:43:18 UTC
The release/2.27/master branch has been updated by Florian Weimer <fw@sourceware.org>:

https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=1961e5c72965a428e5ff18a49c4efdcb65991347

commit 1961e5c72965a428e5ff18a49c4efdcb65991347
Author: Adhemerval Zanella <adhemerval.zanella@linaro.org>
Date:   Fri Apr 26 14:06:31 2019 +0200

    elf: Fix pldd (BZ#18035)
    
    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.
    
    (cherry picked from commit 1a4c27355e146b6d8cc6487b998462c7fdd1048f)
    (Backported without the test case due to lack of test-in-container
    support.)