* [PATCH v2] elf: Fix pldd (BZ#18035)
@ 2019-04-17 14:43 Adhemerval Zanella
2019-04-23 17:40 ` Adhemerval Zanella
` (2 more replies)
0 siblings, 3 replies; 11+ messages in thread
From: Adhemerval Zanella @ 2019-04-17 14:43 UTC (permalink / raw)
To: libc-alpha; +Cc: Carlos O'Donell
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
--
2.17.1
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2] elf: Fix pldd (BZ#18035)
2019-04-17 14:43 [PATCH v2] elf: Fix pldd (BZ#18035) Adhemerval Zanella
@ 2019-04-23 17:40 ` Adhemerval Zanella
2019-04-23 18:21 ` Carlos O'Donell
2019-04-25 0:41 ` Rafal Luzynski
2 siblings, 0 replies; 11+ messages in thread
From: Adhemerval Zanella @ 2019-04-23 17:40 UTC (permalink / raw)
To: libc-alpha; +Cc: Carlos O'Donell
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
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2] elf: Fix pldd (BZ#18035)
2019-04-17 14:43 [PATCH v2] elf: Fix pldd (BZ#18035) Adhemerval Zanella
2019-04-23 17:40 ` Adhemerval Zanella
@ 2019-04-23 18:21 ` Carlos O'Donell
2019-04-25 0:41 ` Rafal Luzynski
2 siblings, 0 replies; 11+ messages in thread
From: Carlos O'Donell @ 2019-04-23 18:21 UTC (permalink / raw)
To: Adhemerval Zanella, libc-alpha
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.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2] elf: Fix pldd (BZ#18035)
2019-04-17 14:43 [PATCH v2] elf: Fix pldd (BZ#18035) Adhemerval Zanella
2019-04-23 17:40 ` Adhemerval Zanella
2019-04-23 18:21 ` Carlos O'Donell
@ 2019-04-25 0:41 ` Rafal Luzynski
2019-04-25 8:52 ` Florian Weimer
2 siblings, 1 reply; 11+ messages in thread
From: Rafal Luzynski @ 2019-04-25 0:41 UTC (permalink / raw)
To: Adhemerval Zanella, libc-alpha; +Cc: Carlos O'Donell
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
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2] elf: Fix pldd (BZ#18035)
2019-04-25 0:41 ` Rafal Luzynski
@ 2019-04-25 8:52 ` Florian Weimer
2019-04-25 9:17 ` Rafal Luzynski
0 siblings, 1 reply; 11+ messages in thread
From: Florian Weimer @ 2019-04-25 8:52 UTC (permalink / raw)
To: Rafal Luzynski; +Cc: Adhemerval Zanella, libc-alpha, Carlos O'Donell
* 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
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2] elf: Fix pldd (BZ#18035)
2019-04-25 8:52 ` Florian Weimer
@ 2019-04-25 9:17 ` Rafal Luzynski
2019-04-25 12:09 ` Florian Weimer
0 siblings, 1 reply; 11+ messages in thread
From: Rafal Luzynski @ 2019-04-25 9:17 UTC (permalink / raw)
To: Florian Weimer; +Cc: Adhemerval Zanella, libc-alpha, Carlos O'Donell
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
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2] elf: Fix pldd (BZ#18035)
2019-04-25 9:17 ` Rafal Luzynski
@ 2019-04-25 12:09 ` Florian Weimer
2019-04-25 12:20 ` Adhemerval Zanella
2019-05-08 22:46 ` Rafal Luzynski
0 siblings, 2 replies; 11+ messages in thread
From: Florian Weimer @ 2019-04-25 12:09 UTC (permalink / raw)
To: Rafal Luzynski; +Cc: Adhemerval Zanella, libc-alpha, Carlos O'Donell
* 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
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2] elf: Fix pldd (BZ#18035)
2019-04-25 12:09 ` Florian Weimer
@ 2019-04-25 12:20 ` Adhemerval Zanella
2019-04-25 12:30 ` Florian Weimer
2019-05-08 22:46 ` Rafal Luzynski
1 sibling, 1 reply; 11+ messages in thread
From: Adhemerval Zanella @ 2019-04-25 12:20 UTC (permalink / raw)
To: Florian Weimer, Rafal Luzynski; +Cc: libc-alpha, Carlos O'Donell
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?
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2] elf: Fix pldd (BZ#18035)
2019-04-25 12:20 ` Adhemerval Zanella
@ 2019-04-25 12:30 ` Florian Weimer
2019-04-30 17:41 ` H.J. Lu
0 siblings, 1 reply; 11+ messages in thread
From: Florian Weimer @ 2019-04-25 12:30 UTC (permalink / raw)
To: Adhemerval Zanella; +Cc: Rafal Luzynski, libc-alpha, Carlos O'Donell
* 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
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2] elf: Fix pldd (BZ#18035)
2019-04-25 12:30 ` Florian Weimer
@ 2019-04-30 17:41 ` H.J. Lu
0 siblings, 0 replies; 11+ messages in thread
From: H.J. Lu @ 2019-04-30 17:41 UTC (permalink / raw)
To: Florian Weimer
Cc: Adhemerval Zanella, Rafal Luzynski, GNU C Library, Carlos O'Donell
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.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2] elf: Fix pldd (BZ#18035)
2019-04-25 12:09 ` Florian Weimer
2019-04-25 12:20 ` Adhemerval Zanella
@ 2019-05-08 22:46 ` Rafal Luzynski
1 sibling, 0 replies; 11+ messages in thread
From: Rafal Luzynski @ 2019-05-08 22:46 UTC (permalink / raw)
To: Florian Weimer; +Cc: Adhemerval Zanella, libc-alpha, Carlos O'Donell
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
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2019-05-08 22:46 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-04-17 14:43 [PATCH v2] elf: Fix pldd (BZ#18035) Adhemerval Zanella
2019-04-23 17:40 ` Adhemerval Zanella
2019-04-23 18:21 ` Carlos O'Donell
2019-04-25 0:41 ` Rafal Luzynski
2019-04-25 8:52 ` Florian Weimer
2019-04-25 9:17 ` Rafal Luzynski
2019-04-25 12:09 ` Florian Weimer
2019-04-25 12:20 ` Adhemerval Zanella
2019-04-25 12:30 ` Florian Weimer
2019-04-30 17:41 ` H.J. Lu
2019-05-08 22:46 ` Rafal Luzynski
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).