From patchwork Wed Apr 17 14:42:33 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Adhemerval Zanella X-Patchwork-Id: 32320 Received: (qmail 68722 invoked by alias); 17 Apr 2019 14:42:45 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 68714 invoked by uid 89); 17 Apr 2019 14:42:45 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-19.7 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KAM_SHORT, RCVD_IN_DNSWL_NONE, SPF_PASS autolearn=ham version=3.3.1 spammy= X-HELO: mail-vk1-f176.google.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=from:to:cc:subject:date:message-id; bh=7xIbSrZSl6xG4gSTGc7wgIf4IPtZqJKTg+CTIkaprlk=; b=pa+6VtlkkNSFAPKnYYFq9JoWJikDSiAZYkH2YsfIkWAp7Bw6b9/rkFiaIRcm8nayWg 45Fyjrn+aNKCB59YnEI9lTdJb2g+CoTdznrAiypircUrfwXyHGQ+kzirE4/52bK6RFdP KrvfX2InRtpeD02v+OXbRfgdXawQ88OELgBm40+ILp9ZPv0kqwj4DnKodZbW+ccaeMdd +2bPcwcZ2kwzbdV6LGoeEX68pYCnUtgZpYAa0dbIVZPaXBT6bDUBcTL1eUMCE9PiNRCm 7EPrKJNo6fxGRdj09CoQLToWTR4fqkuzk+Ro2FLvPTwEnMLa4S6FcT8u0p0bBCsYJgpt 0q3Q== Return-Path: From: Adhemerval Zanella To: libc-alpha@sourceware.org Cc: Carlos O'Donell Subject: [PATCH v2] elf: Fix pldd (BZ#18035) Date: Wed, 17 Apr 2019 11:42:33 -0300 Message-Id: <20190417144233.2470-1-adhemerval.zanella@linaro.org> 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. Reviewed-by: Carlos O'Donell --- 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 . */ -#include +#define _FILE_OFFSET_BITS 64 + #include -#include #include -#include -#include #include #include #include -#include -#include #include #include -#include #include #include -#include #include #include @@ -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 *) ""; + /* Default stack allocation is at least 1024. */ + snprintf (exe.data, exe.length, ""); 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//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 + . */ + +#include +#include +#include +#include +#include +#include + +#include +#include + +#include +#include +#include + +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 : */ + 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 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