diff mbox series

[v2] elf: fix pldd on targets that map objects above 2^63 [BZ #34641]

Message ID 20260917095647.284940-1-stian@itx.no (mailing list archive)
State New
Delegated to: Florian Weimer
Headers
Series [v2] elf: fix pldd on targets that map objects above 2^63 [BZ #34641] |

Checks

Context Check Description
redhat-pt-bot/TryBot-apply_patch success Patch applied to master at the time it was sent
redhat-pt-bot/TryBot-32bit success Build for i686
linaro-tcwg-bot/tcwg_glibc_build--master-aarch64 success Build passed
linaro-tcwg-bot/tcwg_glibc_build--master-arm success Build passed
linaro-tcwg-bot/tcwg_glibc_check--master-aarch64 success Test passed
linaro-tcwg-bot/tcwg_glibc_check--master-arm success Test passed

Commit Message

Stian Halseth Sept. 17, 2026, 9:56 a.m. UTC 
  pldd reads target's memory with pread on /proc/PID/mem.  The offset
argument of pread is signed, and ksys_pread64 rejects a negative
position. On sparc64 the dynamic linker and the shared objects are
mapped at 0xfff8000100000000 and above, so every such read fails with
EINVAL and pldd exits with "cannot read r_debug".  elf/tst-pldd fails
as a result. This is not visible on x86_64, where objects are mapped
low.

Read the memory with lseek and read instead. lseek on /proc/PID/mem
accepts the high offset that pread rejects, and read has no such limit.
gdb had the same bug and fixed it the same way, avoiding pread for high
addresses (PR gdb/30525, commit 31a56a22c45d).

Tested on sparc64: pldd fails on every process before this change and
lists them correctly after it. Full make check shows no test that
passed before failing after.

Signed-off-by: Stian Halseth <stian@itx.no>
---
v2: use lseek and read instead of pread with a process_vm_readv
    fallback, per Adhemerval's review. Drops the process_vm_readv
    availability concern across ABIs, and is simpler: one path, no
    fallback, and the helper is defined once in pldd.c for both classes.

 elf/pldd-xx.c | 12 ++++++------
 elf/pldd.c    | 10 ++++++++++
 2 files changed, 16 insertions(+), 6 deletions(-)

Comments

Florian Weimer Sept. 21, 2026, 1:23 p.m. UTC | #1 
* Stian Halseth:

> pldd reads target's memory with pread on /proc/PID/mem.  The offset
> argument of pread is signed, and ksys_pread64 rejects a negative
> position. On sparc64 the dynamic linker and the shared objects are
> mapped at 0xfff8000100000000 and above, so every such read fails with
> EINVAL and pldd exits with "cannot read r_debug".  elf/tst-pldd fails
> as a result. This is not visible on x86_64, where objects are mapped
> low.
>
> Read the memory with lseek and read instead. lseek on /proc/PID/mem
> accepts the high offset that pread rejects, and read has no such limit.
> gdb had the same bug and fixed it the same way, avoiding pread for high
> addresses (PR gdb/30525, commit 31a56a22c45d).
>
> Tested on sparc64: pldd fails on every process before this change and
> lists them correctly after it. Full make check shows no test that
> passed before failing after.
>
> Signed-off-by: Stian Halseth <stian@itx.no>

Should we still take this patch given that it's a kernel bug?

Thanks,
Florian
Andreas Schwab Sept. 21, 2026, 1:38 p.m. UTC | #2 
On Sep 21 2026, Florian Weimer wrote:

> Should we still take this patch given that it's a kernel bug?

It needs to be fixed together with 34645, because lseek can spuriously
return -1 dependening on the offset.
Adhemerval Zanella Netto Sept. 21, 2026, 1:56 p.m. UTC | #3 
On 21/09/26 10:38, Andreas Schwab wrote:
> On Sep 21 2026, Florian Weimer wrote:
> 
>> Should we still take this patch given that it's a kernel bug?
> 
> It needs to be fixed together with 34645, because lseek can spuriously
> return -1 dependening on the offset.
> 

I forgot about this issue, I will send a patch today.
diff mbox series

Patch

diff --git a/elf/pldd-xx.c b/elf/pldd-xx.c
index 29bbb7307..5e75550a5 100644
--- a/elf/pldd-xx.c
+++ b/elf/pldd-xx.c
@@ -103,7 +103,7 @@  E(find_maps) (const char *exe, int memfd, pid_t pid, void *auxv,
     error (EXIT_FAILURE, 0, gettext ("cannot find program header of process"));
 
   EW(Phdr) *p = xmalloc (phnum * phent);
-  if (pread (memfd, p, phnum * phent, phdr) != phnum * phent)
+  if (read_mem (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
@@ -124,7 +124,7 @@  E(find_maps) (const char *exe, int memfd, pid_t pid, void *auxv,
     if (p[i].p_type == PT_DYNAMIC)
       {
 	EW(Dyn) *dyn = xmalloc (p[i].p_filesz);
-	if (pread (memfd, dyn, p[i].p_filesz, offset + p[i].p_vaddr)
+	if (read_mem (memfd, dyn, p[i].p_filesz, offset + p[i].p_vaddr)
 	    != p[i].p_filesz)
 	  error (EXIT_FAILURE, 0, gettext ("cannot read dynamic section"));
 
@@ -136,7 +136,7 @@  E(find_maps) (const char *exe, int memfd, pid_t pid, void *auxv,
 	    if (off != 0)
 	      {
 		struct E(r_debug) r;
-		if (pread (memfd, &r, sizeof (r), off)
+		if (read_mem (memfd, &r, sizeof (r), off)
 		    != sizeof (r))
 		  error (EXIT_FAILURE, 0, gettext ("cannot read r_debug"));
 
@@ -154,7 +154,7 @@  E(find_maps) (const char *exe, int memfd, pid_t pid, void *auxv,
     else if (p[i].p_type == PT_INTERP)
       {
 	interp = xmalloc (p[i].p_filesz);
-	if (pread (memfd, interp, p[i].p_filesz, offset + p[i].p_vaddr)
+	if (read_mem (memfd, interp, p[i].p_filesz, offset + p[i].p_vaddr)
 	    != p[i].p_filesz)
 	  error (EXIT_FAILURE, 0, gettext ("cannot read program interpreter"));
       }
@@ -184,13 +184,13 @@  E(find_maps) (const char *exe, int memfd, pid_t pid, void *auxv,
   do
     {
       struct E(link_map) m;
-      if (pread (memfd, &m, sizeof (m), list) != sizeof (m))
+      if (read_mem (memfd, &m, sizeof (m), list) != sizeof (m))
 	error (EXIT_FAILURE, 0, gettext ("cannot read link map"));
 
       EW(Addr) name_offset = m.l_name;
       while (1)
 	{
-	  ssize_t n = pread (memfd, tmpbuf.data, tmpbuf.length, name_offset);
+	  ssize_t n = read_mem (memfd, tmpbuf.data, tmpbuf.length, name_offset);
 	  if (n == -1)
 	    error (EXIT_FAILURE, 0, gettext ("cannot read object name"));
 
diff --git a/elf/pldd.c b/elf/pldd.c
index 62e660c3f..d784c24d7 100644
--- a/elf/pldd.c
+++ b/elf/pldd.c
@@ -24,6 +24,7 @@ 
 #include <fcntl.h>
 #include <libintl.h>
 #include <stdio.h>
+#include <stdint.h>
 #include <stdlib.h>
 #include <unistd.h>
 #include <sys/ptrace.h>
@@ -268,6 +269,15 @@  warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.\n\
 }
 
 
+/* Read LEN bytes at ADDR from the target's MEMFD into BUF.  */
+static ssize_t
+read_mem (int memfd, void *buf, size_t len, uint64_t addr)
+{
+  if (lseek (memfd, addr, SEEK_SET) == (off_t) -1)
+    return -1;
+  return read (memfd, buf, len);
+}
+
 #define CLASS 32
 #include "pldd-xx.c"
 #define CLASS 64