[PATCH] linux_nat_target::xfer_partial: Fallback to ptrace

[Keith Seitz]

On 7/21/22 13:07, Pedro Alves wrote:
> 
> Appologies for the delay, there was something here that I wanted to think about, and then it
> just fell through the cracks.

I have quite a few cracks myself through which things fall!

>> On 6/3/22 08:18, Keith Seitz via Gdb-patches wrote:
>>> Pedro Alves wrote:
>>> diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c
>>> index 3b5400896bc..571c97137c2 100644
>>> --- a/gdb/linux-nat.c
>>> +++ b/gdb/linux-nat.c
>>> @@ -3706,8 +3706,12 @@ linux_nat_target::xfer_partial (enum target_object object,
>>>          if (addr_bit < (sizeof (ULONGEST) * HOST_CHAR_BIT))
>>>        offset &= ((ULONGEST) 1 << addr_bit) - 1;
>>>    -      return linux_proc_xfer_memory_partial (readbuf, writebuf,
>>> -                         offset, len, xfered_len);
>>> +      enum target_xfer_status xfer
>>> +    = linux_proc_xfer_memory_partial (readbuf, writebuf,
>>> +                      offset, len, xfered_len);
>>> +      if (xfer != TARGET_XFER_E_IO || readbuf != nullptr)
>>> +    return xfer;
>>> +      /* Fallthrough to ptrace.  /proc/pid/mem wasn't writable before Linux 2.6.39.  */
>>>        }
>>>        return inf_ptrace_target::xfer_partial (object, annex, readbuf, writebuf,
>>
> So I was wondering about mitigating this by only falling back to ptrace if writing to
> /proc/pid/mem doesn't really work.  Checking the kernel version itself seems a bit
> fragile, so I thought we could make gdb probe once at started up whether writing
> to itself via /proc/self/mem works.  It turns out that actually works.  With this, you'd
> just add an extra proc_mem_file_is_writable() check in your patch before falling
> back, or even, skip straight to ptrace if !proc_mem_file_is_writable().
> 
> WDYT?

I think this is quite clever, and an obvious step up in reliability. Thanks
for doing this for me.

>  From 56622b9cadff4b62a0b05861015ce06cf9d6e8f2 Mon Sep 17 00:00:00 2001
> From: Pedro Alves <pedro@palves.net>
> Date: Thu, 21 Jul 2022 19:11:16 +0100
> Subject: [PATCH] gdb/linux-nat: Check whether /proc/pid/mem is writable
> 
> Probe whether /proc/pid/mem is writable, by using it to write to a GDB
> variable.

[snip]

I've taken your patch and updated mine (now trivial):

diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c
index 0a93ab5..95732a6 100644
--- a/gdb/linux-nat.c
+++ b/gdb/linux-nat.c
@@ -3710,8 +3712,10 @@ enum target_xfer_status
        if (addr_bit < (sizeof (ULONGEST) * HOST_CHAR_BIT))
         offset &= ((ULONGEST) 1 << addr_bit) - 1;
  
-      return linux_proc_xfer_memory_partial (readbuf, writebuf,
-                                            offset, len, xfered_len);
+      if (proc_mem_file_is_writable ())
+       return linux_proc_xfer_memory_partial (readbuf, writebuf,
+                                              offset, len, xfered_len);
+      /* Fallthrough to ptrace  */
      }
  
    return inf_ptrace_target::xfer_partial (object, annex, readbuf, writebuf,

I've re-run this through all my testing, and all looks good.

However, I suspect you already knew that. :-)

If/when you push your patch, and there are no further concerns, I will push mine,
with your approval.

Thank you for your follow-up!
Keith