[3/3] linux-nat: Exploit /proc/<pid>/mem for writing

Message ID 1488816060-20776-4-git-send-email-arnez@linux.vnet.ibm.com
State New, archived
Headers

Commit Message

Andreas Arnez March 6, 2017, 4 p.m. UTC
  So far linux_proc_xfer_partial refused to handle write requests.  This is
still based on the assumption that the Linux kernel does not support
writes to /proc/<pid>/mem.  That used to be true, but has changed with
Linux 2.6.39 released in May 2011.

This patch lifts this restriction and now exploits /proc/<pid>/mem for
writing to inferior memory as well, if possible.

gdb/ChangeLog:

	* linux-nat.c (linux_proc_xfer_partial): Handle write operations
	as well.
---
 gdb/linux-nat.c | 32 ++++++++++++++++----------------
 1 file changed, 16 insertions(+), 16 deletions(-)
  

Comments

Pedro Alves March 13, 2017, 8:05 p.m. UTC | #1
On 03/06/2017 04:00 PM, Andreas Arnez wrote:
> So far linux_proc_xfer_partial refused to handle write requests.  This is
> still based on the assumption that the Linux kernel does not support
> writes to /proc/<pid>/mem.  That used to be true, but has changed with
> Linux 2.6.39 released in May 2011.

Hey, I had not noticed that.  Awesome.

(There's also process_vm_readv / process_vm_writev.)

> This patch lifts this restriction and now exploits /proc/<pid>/mem for
> writing to inferior memory as well, if possible.
> 
> gdb/ChangeLog:
> 
> 	* linux-nat.c (linux_proc_xfer_partial): Handle write operations
> 	as well.
> ---
>  gdb/linux-nat.c | 32 ++++++++++++++++----------------
>  1 file changed, 16 insertions(+), 16 deletions(-)
> 
> diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c
> index c58ed83..73ef2d4 100644
> --- a/gdb/linux-nat.c
> +++ b/gdb/linux-nat.c
> @@ -3978,10 +3978,9 @@ linux_child_pid_to_exec_file (struct target_ops *self, int pid)
>    return linux_proc_pid_to_exec_file (pid);
>  }
>  
> -/* Implement the to_xfer_partial interface for memory reads using the /proc
> -   filesystem.  Because we can use a single read() call for /proc, this
> -   can be much more efficient than banging away at PTRACE_PEEKTEXT,
> -   but it doesn't support writes.  */
> +/* Implement the to_xfer_partial target method using /proc/<pid>/mem.
> +   Because we can use a single read/write call, this can be much more
> +   efficient than banging away at PTRACE_PEEKTEXT.  */
>  
>  static enum target_xfer_status
>  linux_proc_xfer_partial (struct target_ops *ops, enum target_object object,
> @@ -3993,7 +3992,7 @@ linux_proc_xfer_partial (struct target_ops *ops, enum target_object object,
>    int fd;
>    char filename[64];
>  
> -  if (object != TARGET_OBJECT_MEMORY || !readbuf)
> +  if (object != TARGET_OBJECT_MEMORY)
>      return TARGET_XFER_EOF;
>  
>    /* Don't bother for one word.  */
> @@ -4004,26 +4003,27 @@ linux_proc_xfer_partial (struct target_ops *ops, enum target_object object,
>       thread.  That requires some juggling, but is even faster.  */
>    xsnprintf (filename, sizeof filename, "/proc/%d/mem",
>  	     ptid_get_pid (inferior_ptid));
> -  fd = gdb_open_cloexec (filename, O_RDONLY | O_LARGEFILE, 0);
> +  fd = gdb_open_cloexec (filename, ((readbuf ? O_RDONLY : O_WRONLY)
> +				    | O_LARGEFILE), 0);
>    if (fd == -1)
>      return TARGET_XFER_EOF;
>  
> -  /* If pread64 is available, use it.  It's faster if the kernel
> -     supports it (only one syscall), and it's 64-bit safe even on
> -     32-bit platforms (for instance, SPARC debugging a SPARC64
> -     application).  */
> +  /* Use pread64/pwrite64 if available, since they save a syscall and can
> +     handle 64-bit offsets even on 32-bit platforms (for instance, SPARC
> +     debugging a SPARC64 application).  */
>  #ifdef HAVE_PREAD64
> -  if (pread64 (fd, readbuf, len, offset) != len)
> +  ret = (readbuf ? pread64 (fd, readbuf, len, offset)
> +	 : pwrite64 (fd, writebuf, len, offset));
>  #else
> -  if (lseek (fd, offset, SEEK_SET) == -1 || read (fd, readbuf, len) != len)
> +  ret = lseek (fd, offset, SEEK_SET);
> +  if (ret != -1)
> +    ret = (readbuf ? read (fd, readbuf, len)
> +	   : write (fd, writebuf, len));
>  #endif
> -    ret = 0;
> -  else
> -    ret = len;
>  
>    close (fd);
>  
> -  if (ret == 0)
> +  if (ret == -1 || ret == 0)
>      return TARGET_XFER_EOF;

Are we sure we can't see partial reads/writes here?
I.e., seems like we lose the "read/pread64 (fd, readbuf, len) != len" 
checks?

Thanks,
Pedro Alves
  
Pedro Alves March 13, 2017, 8:08 p.m. UTC | #2
On 03/13/2017 08:05 PM, Pedro Alves wrote:

> Are we sure we can't see partial reads/writes here?
> I.e., seems like we lose the "read/pread64 (fd, readbuf, len) != len" 
> checks?

Gah, nevermind.  I had it the other way around.
What you have handles partial reads/writes better than
what was there.

So patch is OK.

Thanks,
Pedro Alves
  
Andreas Arnez March 14, 2017, 9:53 a.m. UTC | #3
On Mon, Mar 13 2017, Pedro Alves wrote:

> On 03/06/2017 04:00 PM, Andreas Arnez wrote:
>> So far linux_proc_xfer_partial refused to handle write requests.  This is
>> still based on the assumption that the Linux kernel does not support
>> writes to /proc/<pid>/mem.  That used to be true, but has changed with
>> Linux 2.6.39 released in May 2011.
>
> Hey, I had not noticed that.  Awesome.
>
> (There's also process_vm_readv / process_vm_writev.)

Right.  This reminds me that I've started a patch for exploiting
process_vm_readv/writev two years ago, but then abandoned it.  There was
some problem with it, but I don't recall the details.  Maybe I can dig
it out and try again...

--
Andreas
  
Andreas Arnez March 14, 2017, 11:23 a.m. UTC | #4
On Mon, Mar 13 2017, Pedro Alves wrote:

> On 03/13/2017 08:05 PM, Pedro Alves wrote:
>
>> Are we sure we can't see partial reads/writes here?
>> I.e., seems like we lose the "read/pread64 (fd, readbuf, len) != len" 
>> checks?
>
> Gah, nevermind.  I had it the other way around.
> What you have handles partial reads/writes better than
> what was there.
>
> So patch is OK.

Thanks, pushed.

--
Andreas
  

Patch

diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c
index c58ed83..73ef2d4 100644
--- a/gdb/linux-nat.c
+++ b/gdb/linux-nat.c
@@ -3978,10 +3978,9 @@  linux_child_pid_to_exec_file (struct target_ops *self, int pid)
   return linux_proc_pid_to_exec_file (pid);
 }
 
-/* Implement the to_xfer_partial interface for memory reads using the /proc
-   filesystem.  Because we can use a single read() call for /proc, this
-   can be much more efficient than banging away at PTRACE_PEEKTEXT,
-   but it doesn't support writes.  */
+/* Implement the to_xfer_partial target method using /proc/<pid>/mem.
+   Because we can use a single read/write call, this can be much more
+   efficient than banging away at PTRACE_PEEKTEXT.  */
 
 static enum target_xfer_status
 linux_proc_xfer_partial (struct target_ops *ops, enum target_object object,
@@ -3993,7 +3992,7 @@  linux_proc_xfer_partial (struct target_ops *ops, enum target_object object,
   int fd;
   char filename[64];
 
-  if (object != TARGET_OBJECT_MEMORY || !readbuf)
+  if (object != TARGET_OBJECT_MEMORY)
     return TARGET_XFER_EOF;
 
   /* Don't bother for one word.  */
@@ -4004,26 +4003,27 @@  linux_proc_xfer_partial (struct target_ops *ops, enum target_object object,
      thread.  That requires some juggling, but is even faster.  */
   xsnprintf (filename, sizeof filename, "/proc/%d/mem",
 	     ptid_get_pid (inferior_ptid));
-  fd = gdb_open_cloexec (filename, O_RDONLY | O_LARGEFILE, 0);
+  fd = gdb_open_cloexec (filename, ((readbuf ? O_RDONLY : O_WRONLY)
+				    | O_LARGEFILE), 0);
   if (fd == -1)
     return TARGET_XFER_EOF;
 
-  /* If pread64 is available, use it.  It's faster if the kernel
-     supports it (only one syscall), and it's 64-bit safe even on
-     32-bit platforms (for instance, SPARC debugging a SPARC64
-     application).  */
+  /* Use pread64/pwrite64 if available, since they save a syscall and can
+     handle 64-bit offsets even on 32-bit platforms (for instance, SPARC
+     debugging a SPARC64 application).  */
 #ifdef HAVE_PREAD64
-  if (pread64 (fd, readbuf, len, offset) != len)
+  ret = (readbuf ? pread64 (fd, readbuf, len, offset)
+	 : pwrite64 (fd, writebuf, len, offset));
 #else
-  if (lseek (fd, offset, SEEK_SET) == -1 || read (fd, readbuf, len) != len)
+  ret = lseek (fd, offset, SEEK_SET);
+  if (ret != -1)
+    ret = (readbuf ? read (fd, readbuf, len)
+	   : write (fd, writebuf, len));
 #endif
-    ret = 0;
-  else
-    ret = len;
 
   close (fd);
 
-  if (ret == 0)
+  if (ret == -1 || ret == 0)
     return TARGET_XFER_EOF;
   else
     {