[0/5] gdbserver: Follow-up on linux_get_auxv using PID parameter

  Hello,

Back in February, I pushed commit 43e5fbd8b788 ("gdbserver: Add PID
parameter to linux_get_auxv and linux_get_hwcap"), and mentioned that
there was no change in gdbserver behaviour. Pedro corrected me,
mentioning that before the patch gdbserver would read the auxv from
/proc/<current_thread's LWP>/auxv, while after the patch it would read
it from /proc/<current_thread's PID>/auxv. This causes trouble in case
the inferior's main thread exits.

I created a testcase exercising this scenario (the last patch in this
series) and confirmed that both GDB and gdbserver have this problem (GDB
wasn't changed by my patch). The problem is largely mitigated by GDB's
auxv cache though, and is very hard to hit because one of the first
things that GDB does when starting/attaching to an inferior is read the
auxv and cache it. It will only be a problem if the cache is invalidated
by one of the cache-clearing events ("inferior_exit",
"inferior_appeared", "executable_changed").

In the discussion about my patch there were also some questions about
other race conditions in this area. I created a test program to
experiment and these are the results:

Q1: What happens exactly if GDB/gdbserver tries to read
    /proc/<thread's PID>/auxv after the main thread exits?
A1: If GDB/gdbserver is root, then open() will succeed but read() will
    return 0, indicating an empty file. If GDB/gdbserver isn't root,
    then open() will fail with errno = EACCES ("Permission denied") even
    if it's running as the same user as the inferior.

Q2: What happens if an inferior thread exits after GDB/gdbserver opens its
    /proc/<thread's LWP>/auxv file but before it has a chance to read it?
A2: The read() call will return 0, indicating an empty file.

Patch 3 fixes the problem for gdbserver, and patch 4 fixes it for GDB.

Patches 1 and 2 implement a couple of suggestions made by Pedro in the
same thread.

Regression tested on native and remote aarch64-linux and x86_64-linux.

Thiago Jung Bauermann (5):
  gdbserver: Use current_process in handle_qxfer_auxv
  gdbserver: Use the PID of the current process instead of the current
    thread
  gdbserver/linux: Read auxv from any thread of the process
  gdb/linux-nat: Read auxv from any thread of the process
  gdb/testsuite: Add test that reads auxv in a multi-threaded inferior

 gdb/auxv.c                         | 16 +++++++
 gdb/linux-nat.c                    | 40 +++++++++++++++++-
 gdb/nat/linux-procfs.c             | 67 ++++++++++++++++++++++++++++++
 gdb/nat/linux-procfs.h             |  7 ++++
 gdb/testsuite/gdb.base/auxv.exp    | 56 -------------------------
 gdb/testsuite/gdb.threads/auxv.c   | 62 +++++++++++++++++++++++++++
 gdb/testsuite/gdb.threads/auxv.exp | 30 +++++++++++++
 gdb/testsuite/lib/gdb.exp          | 62 +++++++++++++++++++++++++++
 gdbserver/linux-aarch64-low.cc     | 10 ++---
 gdbserver/linux-arm-low.cc         |  6 +--
 gdbserver/linux-low.cc             | 21 ++--------
 gdbserver/linux-ppc-low.cc         |  6 +--
 gdbserver/mem-break.cc             |  2 +-
 gdbserver/regcache.cc              |  2 +-
 gdbserver/server.cc                |  7 ++--
 gdbserver/tracepoint.cc            |  2 +-
 gdbserver/win32-i386-low.cc        |  4 +-
 17 files changed, 305 insertions(+), 95 deletions(-)
 create mode 100644 gdb/testsuite/gdb.threads/auxv.c
 create mode 100644 gdb/testsuite/gdb.threads/auxv.exp


base-commit: 66f76c545b293f8b89fef0f996a3a48fa59fae61
  

On 3/30/23 23:44, Thiago Jung Bauermann via Gdb-patches wrote:
> Hello,
> 
> Back in February, I pushed commit 43e5fbd8b788 ("gdbserver: Add PID
> parameter to linux_get_auxv and linux_get_hwcap"), and mentioned that
> there was no change in gdbserver behaviour. Pedro corrected me,
> mentioning that before the patch gdbserver would read the auxv from
> /proc/<current_thread's LWP>/auxv, while after the patch it would read
> it from /proc/<current_thread's PID>/auxv. This causes trouble in case
> the inferior's main thread exits.
> 
> I created a testcase exercising this scenario (the last patch in this
> series) and confirmed that both GDB and gdbserver have this problem (GDB
> wasn't changed by my patch). The problem is largely mitigated by GDB's
> auxv cache though, and is very hard to hit because one of the first
> things that GDB does when starting/attaching to an inferior is read the
> auxv and cache it. It will only be a problem if the cache is invalidated
> by one of the cache-clearing events ("inferior_exit",
> "inferior_appeared", "executable_changed").

If it helps, you can add maintenance commands to force clearing and
fetching the auxv.

> 
> In the discussion about my patch there were also some questions about
> other race conditions in this area. I created a test program to
> experiment and these are the results:
> 
> Q1: What happens exactly if GDB/gdbserver tries to read
>     /proc/<thread's PID>/auxv after the main thread exits?
> A1: If GDB/gdbserver is root, then open() will succeed but read() will
>     return 0, indicating an empty file. If GDB/gdbserver isn't root,
>     then open() will fail with errno = EACCES ("Permission denied") even
>     if it's running as the same user as the inferior.

Crazy!

> Q2: What happens if an inferior thread exits after GDB/gdbserver opens its
>     /proc/<thread's LWP>/auxv file but before it has a chance to read it?
> A2: The read() call will return 0, indicating an empty file.

Even if the thread is ptraced (which is the case, we are debugging it)
and we haven't consumed the thread exit event from the kernel?

I suppose an even more corner case is: a first partial read succeeds,
the thread exits, the following read returns 0.  Is it even possible to
detect this happened?

Simon