gdb: only use /proc/PID/exe for local f/s with no sysroot

This commit works around a problem introduced by commit:

  commit e58beedf2c8a1e0c78e0f57aeab3934de9416bfc
  Date:   Tue Jan 23 16:00:59 2024 +0000

      gdb: attach to a process when the executable has been deleted

The above commit extended GDB for Linux, so that, of the executable
for a process had been deleted, GDB would instead try to use
/proc/PID/exe as the executable.

This worked by updating linux_proc_pid_to_exec_file to introduce the
/proc/PID/exe fallback.  However, the result of
linux_proc_pid_to_exec_file is then passed to exec_file_find to
actually find the executable, and exec_file_find, will take into
account the sysroot.  In addition, if GDB is attaching to a process in
a different MNT and/or PID namespace then the executable lookup is
done within that namespace.

This all means two things:

  1. Just because linux_proc_pid_to_exec_file cannot see the
     executable doesn't mean that GDB is actually going to fail to
     find the executable, and

  2. returning /proc/PID/exe isn't useful if we know GDB is then going
     to look for this within a sysroot, or within some other
     namespace (where PIDs might be different).

There was an initial attempt to fix this issue here:

  https://inbox.sourceware.org/gdb-patches/20250511141517.2455092-4-kilger@sec.in.tum.de/

This proposal addresses the issue in PR gdb/32955, which is all about
the namespace side of the problem.  The fix in this original proposal
is to check the MNT namespace inside linux_proc_pid_to_exec_file, and
for the namespace problem this is fine.  But we should also consider
the sysroot problem.

And for the sysroot problem, the fix cannot fully live inside
linux_proc_pid_to_exec_file, as linux_proc_pid_to_exec_file is shared
between GDB and gdbserver, and gdbserver has no sysroot.

And so, I propose a slightly bigger change.

Now, linux_proc_pid_to_exec_file takes a flag which indicates if
GDB (or gdbserver) will look for the inferior executable in the
local file system, where local means the same file system as GDB (or
gdbserver) is running in.

This local file system check is true if:

  1. The MNT namespace of the inferior is the same as for GDB, and

  2. for GDB only, the sysroot must either be empty, or 'target:'.

If the local file system check is false then GDB (or gdbserver) is
going to look elsewhere for the inferior executable, and so, falling
back to /proc/PID/exe should not be done, as GDB will end up looking
for this file in the sysroot, or within the alternative MNT
namespace (which in also likely to be a different PID namespace).

Now this is all a bit of a shame really.  It would be nice if
linux_proc_pid_to_exec_file could return /proc/PID/exe in such a way
that exec_file_find would know that the file should NOT be looked for
in the sysroot, or in the alternative namespace.  But fixing that
problem would be a much bigger change, so for now lets just disable
the /proc/PID/exe fallback for cases where it might not work.

For testing, the sysroot case is now tested.

I don't believe we have any alternative namespace testing.  It would
certainly be interesting to add some, but I'm not proposing any with
this patch, so the code for checking the MNT namespace has been tested
manually by me, but isn't covered by a new test I'm adding here.

Author of the original fix is listed as co-author here.  Credit for
identifying the original problem, and proposing a solution belongs to
them.

Co-Authored-By: Fabian Kilger <kilger@sec.in.tum.de>
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=32955

diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c
index 8f5f978074f017e0a043dc60dddbad1fe7163ea3..59f416f6118615a4622d4736b5bb387a32b773bb 100644 (file)
--- a/gdb/linux-nat.c
+++ b/gdb/linux-nat.c
@@ -2129,7 +2129,7 @@ linux_handle_extended_wait (struct lwp_info *lp, int status)
       open_proc_mem_file (lp->ptid);
 
       ourstatus->set_execd
-       (make_unique_xstrdup (linux_proc_pid_to_exec_file (pid)));
+       (make_unique_xstrdup (linux_target->pid_to_exec_file (pid)));
 
       /* The thread that execed must have been resumed, but, when a
         thread execs, it changes its tid to the tgid, and the old
@@ -4000,7 +4000,14 @@ linux_nat_target::thread_name (struct thread_info *thr)
 const char *
 linux_nat_target::pid_to_exec_file (int pid)
 {
-  return linux_proc_pid_to_exec_file (pid);
+  /* If there's no sysroot.  Or the sysroot is just 'target:' and the
+     inferior is in the same mount namespce, then we can consider the
+     filesystem local.  */
+  bool local_fs = (gdb_sysroot.empty ()
+                  || (gdb_sysroot == TARGET_SYSROOT_PREFIX
+                      && linux_ns_same (pid, LINUX_NS_MNT)));
+
+  return linux_proc_pid_to_exec_file (pid, local_fs);
 }
 
 /* Object representing an /proc/PID/mem open file.  We keep one such

diff --git a/gdb/nat/linux-procfs.c b/gdb/nat/linux-procfs.c
index a72df2a09d7b71f37cf1616916d1ff2c555a9c62..d4f9af32bb9e7a378d60fc07cc1fe763ab395e82 100644 (file)
--- a/gdb/nat/linux-procfs.c
+++ b/gdb/nat/linux-procfs.c
@@ -424,7 +424,7 @@ linux_proc_task_list_dir_exists (pid_t pid)
 /* See linux-procfs.h.  */
 
 const char *
-linux_proc_pid_to_exec_file (int pid)
+linux_proc_pid_to_exec_file (int pid, bool is_local_fs)
 {
   static char buf[PATH_MAX];
   char name[PATH_MAX];
@@ -437,9 +437,31 @@ linux_proc_pid_to_exec_file (int pid)
   else
     buf[len] = '\0';
 
-  /* Use /proc/PID/exe if the actual file can't be read, but /proc/PID/exe
-     can be.  */
-  if (access (buf, R_OK) != 0 && access (name, R_OK) == 0)
+  /* If the inferior and GDB can see the same filesystem, and NAME
+     cannot be read, maybe the file has been deleted, then we can
+     potentially use /proc/PID/exe instead.
+
+     GDB always interprets the results of this function within the
+     current sysroot (which is 'target:' by default).  This means
+     that, if we return /proc/PID/exe, GDB will try to find this file
+     within the sysroot.
+
+     This doesn't make sense if GDB is using a sysroot like:
+     '/some/random/directory/', not only is it possible that NAME
+     could be found within the sysroot, it is unlikely that
+     /proc/PID/exe will exist within the sysroot.
+
+     Similarly, if the sysroot is 'target:', but the inferior is
+     running within a separate MNT namespace, then it is more than
+     likely that the inferior will also be running in a separate PID
+     namespace, in this case PID is the pid on the host system,
+     /proc/PID/exe will not be correct within the inferiors MNT/PID
+     namespace.
+
+     So, we can fallback to use /proc/PID/exe only if IS_LOCAL_FS is
+     true, this indicates that GDB and the inferior are using the same
+     MNT namespace, and GDB's sysroot is either empty, or 'target:'.  */
+  if (is_local_fs && access (buf, R_OK) != 0 && access (name, R_OK) == 0)
     strcpy (buf, name);
 
   return buf;

diff --git a/gdb/nat/linux-procfs.h b/gdb/nat/linux-procfs.h
index 0c806221faa149c34419b3ccd1ebbddfb432592c..4b02d31abdfd0da869971ae606c6587c35f97afb 100644 (file)
--- a/gdb/nat/linux-procfs.h
+++ b/gdb/nat/linux-procfs.h
@@ -87,9 +87,19 @@ extern int linux_proc_task_list_dir_exists (pid_t pid);
 
 /* Return the full absolute name of the executable file that was run
    to create the process PID.  The returned value persists until this
-   function is next called.  */
+   function is next called.
 
-extern const char *linux_proc_pid_to_exec_file (int pid);
+   LOCAL_FS should be true if the file returned from the function will
+   be searched for in the same filesystem as GDB itself is running.
+   In practice, this means LOCAL_FS should be true if PID and GDB are
+   running in the same MNT namespace and GDB's sysroot is either the
+   empty string, or is 'target:'.
+
+   When used from gdbserver, where there is no sysroot, the only check
+   that matters is that PID and gdbserver are running in the same MNT
+   namespace.  */
+
+extern const char *linux_proc_pid_to_exec_file (int pid, bool local_fs);
 
 /* Display possible problems on this system.  Display them only once
    per GDB execution.  */

diff --git a/gdb/testsuite/gdb.base/attach-deleted-exec.exp b/gdb/testsuite/gdb.base/attach-deleted-exec.exp
index 82a7bb409fe2b2608c87dd65948cbe375985b93a..45fac0db4731bdcfe510da99ea643e08439714bf 100644 (file)
--- a/gdb/testsuite/gdb.base/attach-deleted-exec.exp
+++ b/gdb/testsuite/gdb.base/attach-deleted-exec.exp
@@ -67,5 +67,49 @@ if { [regexp $re_nfs $filename] } {
     gdb_assert { [string equal $filename /proc/${testpid}/exe] } $test
 }
 
+# Restart GDB.
+clean_restart
+
+# Setup an empty sysroot.  GDB will fail to find the executable within
+# the sysroot.  Additionally, the presence of a sysroot should prevent
+# GDB from trying to load the executable from /proc/PID/exe.
+set sysroot [standard_output_file "sysroot"]
+gdb_test_no_output "set sysroot $sysroot" \
+    "setup sysroot"
+
+# Attach to the inferior.  GDB should complain about failing to find
+# the executable.  It is the name of the executable that GDB doesn't
+# find that we're interesting in here.  For native targets GDB should
+# be looking for BINFILE, not /proc/PID/exe.
+#
+# For extended-remote targets things are unfortunately harder.  Native
+# GDB looks for BINFILE because it understands that GDB will be
+# looking in the sysroot.  But remote GDB doesn't know if GDB is using
+# a sysroot or not.  As such, gdbserver will return /proc/PID/exe if
+# it knows that the file has been deleted locally.  This isn't great
+# if GDB then plans to look in a sysroot, but equally, if the remote
+# file has been deleted, then the name GDB will return, will have had
+# " (deleted" appended, so we're unlikely to get a hit in the sysroot
+# either way.
+if { [target_info gdb_protocol] == "extended-remote" } {
+    set filename_re "/proc/$testpid/exe"
+} else {
+    set filename_re "\[^\r\n\]+/${testfile} \\(deleted\\)"
+}
+
+verbose -log "APB: warning: No executable has been specified, and target executable $filename_re could not be found\\.  Try using the \"file\" command\\."
+
+gdb_test "attach $testpid" \
+    [multi_line \
+        "Attaching to process $decimal" \
+        "warning: No executable has been specified, and target executable $filename_re could not be found\\.  Try using the \"file\" command\\." \
+        ".*"] \
+    "attach to inferior"
+
+# Check GDB hasn't managed to load an executable.
+gdb_test "info inferior" \
+    "\\*\[^)\]+\\)\\s*" \
+    "confirm no executable is loaded."
+
 # Cleanup.
 kill_wait_spawned_process $test_spawn_id

diff --git a/gdbserver/linux-low.cc b/gdbserver/linux-low.cc
index e507558c64e73581cee23ae8c594fd7f4838c220..39642705b0d8bc614e7da84f0d0e1c34cd045b12 100644 (file)
--- a/gdbserver/linux-low.cc
+++ b/gdbserver/linux-low.cc
@@ -751,7 +751,7 @@ linux_process_target::handle_extended_wait (lwp_info **orig_event_lwp,
       /* Set the event status.  */
       event_lwp->waitstatus.set_execd
        (make_unique_xstrdup
-          (linux_proc_pid_to_exec_file (event_thr->id.lwp ())));
+          (pid_to_exec_file (event_thr->id.lwp ())));
 
       /* Mark the exec status as pending.  */
       event_lwp->stopped = 1;
@@ -6033,7 +6033,7 @@ linux_process_target::supports_pid_to_exec_file ()
 const char *
 linux_process_target::pid_to_exec_file (int pid)
 {
-  return linux_proc_pid_to_exec_file (pid);
+  return linux_proc_pid_to_exec_file (pid, linux_ns_same (pid, LINUX_NS_MNT));
 }
 
 bool