[PATCH 0/2][PR < shlibs/29586>] GDB hangs when trying to resolve memory mapped shared libraries of an inferior under /proc/self/fd/*

[PATCH 0/2][PR < shlibs/29586>] GDB hangs when trying to resolve memory mapped shared libraries of an inferior under /proc/self/fd/*

[Asaf Fisher]

Hello!
If any of you will try to dlopen a memory mapped file in the following way:`dlopen("/proc/self/fd/4"....)`
The operation will trigger GDB's hook on dlopen and will try to load the file "/proc/self/fd/4".
Obviously GDB's process has different FD's on `/proc/self`, and will read from an arbitrary opened file.
Most likely it will open a pipe which will cause GDB to hang.
Here is a rust snippet that will hang GDB once being debugged:
```
use std::{os::unix::prelude::{AsFd, AsRawFd}, io::Write};

use memfd;
use dlopen_derive::{self, WrapperApi};
#[macro_use]
use dlopen::wrapper::{Container,WrapperApi};

#[derive(WrapperApi)]
struct Api<'a> {
    example_rust_fun: fn(arg: i32) -> u32,
    example_c_fun: unsafe extern "C" fn(),
    example_reference: &'a mut i32,
}
fn main() {
    let opts = memfd::MemfdOptions::default().allow_sealing(true);
    let mfd = opts.create("hellooo").unwrap();
    let buff = std::fs::read("/usr/lib64/ld-linux-x86-64.so.2").unwrap();
    mfd.as_file().write(buff.as_slice()).unwrap();
    let fd = mfd.as_file().as_fd().as_raw_fd();
    let fm = format!("/proc/self/fd/{}", fd);
    println!("{}", fm);
    let mut cont: Container<Api> =
        unsafe { Container::load(fm) }.expect("Could not open library or load symbols");

}
```

To fix the problem I added a function to `solib-svr4.c` that will resolve `/proc/self/fd/[num]` to `/proc/[inferior_pid]/fd/[num]`.
To test it I added a test that simply checks if the warning printed by GDB when resolving the path is correct.

Asaf Fisher (2):
  Add test to check GDB handles dlopen of /proc/self/fd/[num] correctly
  Make GDB resolve dlopen of memory mapped shared libraries

 gdb/solib-svr4.c                           | 58 ++++++++++++++-
 gdb/testsuite/gdb.base/solib-proc-self.cc  | 72 ++++++++++++++++++
 gdb/testsuite/gdb.base/solib-proc-self.exp | 86 ++++++++++++++++++++++
 3 files changed, 214 insertions(+), 2 deletions(-)
 create mode 100644 gdb/testsuite/gdb.base/solib-proc-self.cc
 create mode 100644 gdb/testsuite/gdb.base/solib-proc-self.exp

-- 
2.38.0

[PATCH 1/2] Add test to check GDB handles dlopen of /proc/self/fd/[num] correctly

[Asaf Fisher]

This test checks that GDB handles correctly paths in the form of
`/proc/self/...` when inferior dlopen them.

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=29586
---
 gdb/testsuite/gdb.base/solib-proc-self.cc  | 72 ++++++++++++++++++
 gdb/testsuite/gdb.base/solib-proc-self.exp | 86 ++++++++++++++++++++++
 2 files changed, 158 insertions(+)
 create mode 100644 gdb/testsuite/gdb.base/solib-proc-self.cc
 create mode 100644 gdb/testsuite/gdb.base/solib-proc-self.exp

diff --git a/gdb/testsuite/gdb.base/solib-proc-self.cc b/gdb/testsuite/gdb.base/solib-proc-self.cc
new file mode 100644
index 00000000000..dc0b446d53c
--- /dev/null
+++ b/gdb/testsuite/gdb.base/solib-proc-self.cc
@@ -0,0 +1,72 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2007-2022 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#include <sys/mman.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <iostream>
+#include <fstream>
+#include <sstream>
+#include <vector>
+#include <unistd.h>
+
+#ifdef __WIN32__
+#include <windows.h>
+#define dlopen(name, mode) LoadLibrary (name)
+#define dlclose(handle) FreeLibrary (handle)
+#define dlerror() "an error occurred"
+#else
+#include <dlfcn.h>
+#endif
+
+int main()
+{
+  void *handle;
+  /* Read the so's content to a buffer */
+  std::ifstream read_so_file = std::ifstream(SHLIB_NAME);
+  read_so_file.seekg(0, std::ios::end);
+  std::streamsize size = read_so_file.tellg();
+  read_so_file.seekg(0, std::ios::beg);
+  std::vector<char> buffer(size);
+  if (!read_so_file.read(buffer.data(), size))
+  {
+    fprintf (stderr, "Failed to load solib\n");
+    exit(1);
+  }
+
+  int mem_fd = memfd_create("test", 0);
+
+  /* Write the so's data to the memory mapped file. */
+  write(mem_fd, buffer.data(), buffer.size());
+
+  /* Generate the /proc/self/fd/[num] path */
+  std::string prof_self_fd_path; /* break-here */
+  std::stringstream prof_self_fd_path_stream = std::stringstream(prof_self_fd_path);
+  prof_self_fd_path_stream << "/proc/self/fd/" << mem_fd;
+
+  /* Call dlopen on it */
+  handle = dlopen (prof_self_fd_path_stream.str().c_str(), RTLD_LAZY);
+  if (!handle)
+  {
+      fprintf (stderr, "%s\n", dlerror ());
+      exit (1);
+  }
+  /* YAY it worked */
+  dlclose (handle);
+
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.base/solib-proc-self.exp b/gdb/testsuite/gdb.base/solib-proc-self.exp
new file mode 100644
index 00000000000..b59ba357492
--- /dev/null
+++ b/gdb/testsuite/gdb.base/solib-proc-self.exp
@@ -0,0 +1,86 @@
+# Copyright 2007-2022 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+# Test connecting and disconnecting at shared library events.
+
+if {[skip_shlib_tests]} {
+    untested "could not run to main"
+    return 0
+}
+
+standard_testfile .cc
+
+# Chose random lib
+set libfile so-disc-shr
+set libsrc "${srcdir}/${subdir}/${libfile}.c"
+set libname "${libfile}.so"
+set libobj [standard_output_file ${libname}]
+
+# Compile the shared lib
+if { [gdb_compile_shlib $libsrc $libobj {debug}] != ""} {
+    return -1
+}
+
+# Compile test
+if [ prepare_for_testing "failed to prepare" $testfile $srcfile "list shlib_load debug c++ additional_flags=-DSHLIB_NAME=\"${libobj}\"" ] {
+    return -1
+}
+
+gdb_exit
+gdb_start
+gdb_reinitialize_dir $srcdir/$subdir
+gdb_load ${binfile}
+gdb_load_shlib $libobj
+
+if ![runto_main] then {
+    return 0
+}
+
+# Get inferior's PID for later
+set inferior_pid -1
+gdb_test_multiple "info inferior 1" "get inferior pid" {
+    -re "process (\[0-9\]*).*$gdb_prompt $" {
+	set inferior_pid $expect_out(1,string)
+	pass $gdb_test_name
+    }
+}
+
+# Turn on the solib-events so we can see that gdb resolves everything correctly
+gdb_test_no_output "set stop-on-solib-events 1"
+
+# I use this breakpoint to get the memory mapped fd.
+gdb_breakpoint [gdb_get_line_number "break-here"]
+gdb_continue_to_breakpoint "break-here" ".* break-here .*"
+
+set msg "Getting MEMFD"
+set memfd ""
+gdb_test_multiple "p mem_fd" $msg {
+    -re "\\\$$decimal = (\[^\r\n\]*)\r\n$gdb_prompt $" {
+	set memfd $expect_out(1,string)
+	pass $msg
+    }
+}
+
+gdb_test "continue" "Stopped due to shared library event.*" "continue to load"
+
+# Check if inferior resolved the /proc/self/fd/[num] to /proc/[pid]/fd/[num]
+set msg "Inferior's /proc/self resolving $inferior_pid $memfd"
+set inferior_proc_self_path ""
+gdb_test_multiple "continue" $msg {
+    -re "Attempting to replace `self` with inferior's PID. -> (\/proc\/$inferior_pid\/fd\/$memfd\[^\r\n\]*)\r\n.*$gdb_prompt $" {
+	set inferior_proc_self_path $expect_out(1,string)
+	pass $msg
+    }
+}
-- 
2.38.0

[PATCH 2/2] Make GDB resolve dlopen of memory mapped shared libraries

[Asaf Fisher]

Introduced `check_proc_self_file` that checks if a path used by
inferior in dlopen is in the form of `/proc/self/...` and if so resolves
it to `/proc/[pid]/...`

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=29586
---
 gdb/solib-svr4.c | 58 ++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 56 insertions(+), 2 deletions(-)

diff --git a/gdb/solib-svr4.c b/gdb/solib-svr4.c
index 7e83819a03d..231a4fb40e5 100644
--- a/gdb/solib-svr4.c
+++ b/gdb/solib-svr4.c
@@ -34,6 +34,7 @@
 #include "regcache.h"
 #include "gdbthread.h"
 #include "observable.h"
+#include "gdbsupport/pathstuff.h"
 
 #include "solist.h"
 #include "solib.h"
@@ -46,6 +47,9 @@
 #include "gdb_bfd.h"
 #include "probe.h"
 
+#define SLASH_SELF "/self"
+#define PROC_SELF  "/proc" SLASH_SELF
+
 static struct link_map_offsets *svr4_fetch_link_map_offsets (void);
 static int svr4_have_link_map_offsets (void);
 static void svr4_relocate_main_executable (void);
@@ -1187,6 +1191,54 @@ svr4_default_sos (svr4_info *info)
   return newobj;
 }
 
+/* Check and fix a cenerio where the so path that we extract has a path to
+  /proc/self e.g. /proc/self/fd/[fd_num] If inferior dlopen a path that has
+  /proc/self, GDB must not open it directly becuase the files in /proc/self are
+  unique for each process. Instead we resolve /proc/self to
+  /proc/[inferior_pid]. This change will give GDB the correct path */
+
+static size_t check_proc_self_file(char *so_name, char *normalized_so_name,
+                                   size_t out_normalized_so_name_len) {
+  /* We dont want a path with /../ yak. */
+  gdb::unique_xmalloc_ptr<char> normalized_path_obj = gdb_realpath(so_name);
+  gdb::string_view normalized_path = gdb::string_view(
+      normalized_path_obj.get(),
+      std::min(strlen(normalized_path_obj.get()), out_normalized_so_name_len));
+
+  /* Is the path really a /proc/self? */
+  if (0 != normalized_path.rfind(PROC_SELF, 0)) return 0;
+
+  /* Lets get the part of the path after /proc/self e.g. /proc/self/fd -> /fd */
+  size_t slash_self_index = normalized_path.rfind(SLASH_SELF);
+  if (std::string::npos == slash_self_index) return 0;
+  size_t after_self_index = slash_self_index + strlen(SLASH_SELF);
+  gdb::string_view after_self_path = normalized_path.substr(after_self_index);
+
+  /* Get inferior path */
+  int inferior_pid = inferior_ptid.pid();
+  std::string inferior_procfs_path = string_printf("/proc/%d", inferior_pid);
+
+  /* Check if there's enoght space in the out buffer for the normalized path. */
+  size_t normalized_so_name_length =
+      inferior_procfs_path.length() + after_self_path.length();
+  if (out_normalized_so_name_len < normalized_so_name_length) return 0;
+
+  /* Build the full path */
+  inferior_procfs_path.append(std::string(after_self_path));
+
+  warning(_("Detected loaded library (%s) from /proc/self.\nAttempting to "
+            "replace `self` with inferior's PID. -> %s"),
+          normalized_path.begin(), inferior_procfs_path.c_str());
+
+  auto out_length =
+      std::min(inferior_procfs_path.length(), out_normalized_so_name_len);
+
+  /* Copy the new path to the out buffer */
+  strncpy(normalized_so_name, inferior_procfs_path.c_str(), out_length);
+
+  return out_length;
+}
+
 /* Read the whole inferior libraries chain starting at address LM.
    Expect the first entry in the chain's previous entry to be PREV_LM.
    Add the entries to the tail referenced by LINK_PTR_PTR.  Ignore the
@@ -1246,8 +1298,10 @@ svr4_read_so_list (svr4_info *info, CORE_ADDR lm, CORE_ADDR prev_lm,
 	    warning (_("Can't read pathname for load map."));
 	  continue;
 	}
-
-      strncpy (newobj->so_name, buffer.get (), SO_NAME_MAX_PATH_SIZE - 1);
+      /* Check if path is in /proc/self */
+      if (0 == check_proc_self_file(buffer.get(), newobj->so_name,
+                              SO_NAME_MAX_PATH_SIZE - 1))
+        strncpy(newobj->so_name, buffer.get(), SO_NAME_MAX_PATH_SIZE - 1);
       newobj->so_name[SO_NAME_MAX_PATH_SIZE - 1] = '\0';
       strcpy (newobj->so_original_name, newobj->so_name);
 
-- 
2.38.0

Re: [PATCH 2/2] Make GDB resolve dlopen of memory mapped shared libraries

[Alexandra Petlanova Hajkova]

On Mon, Oct 17, 2022 at 9:39 PM Asaf Fisher via Gdb-patches <
gdb-patches@sourceware.org> wrote:

> Introduced `check_proc_self_file` that checks if a path used by
> inferior in dlopen is in the form of `/proc/self/...` and if so resolves
> it to `/proc/[pid]/...`
>
> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=29586
> ---
>  gdb/solib-svr4.c | 58 ++++++++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 56 insertions(+), 2 deletions(-)
>
> diff --git a/gdb/solib-svr4.c b/gdb/solib-svr4.c
> index 7e83819a03d..231a4fb40e5 100644
> --- a/gdb/solib-svr4.c
> +++ b/gdb/solib-svr4.c
> @@ -34,6 +34,7 @@
>  #include "regcache.h"
>  #include "gdbthread.h"
>  #include "observable.h"
> +#include "gdbsupport/pathstuff.h"
>
>  #include "solist.h"
>  #include "solib.h"
> @@ -46,6 +47,9 @@
>  #include "gdb_bfd.h"
>  #include "probe.h"
>
> Looks correct to me. I can confirm gdb.base/solib-proc-self.exp test added
> by [PATCH 1/2]

fails and hangs without this fix and passes after applying it. This patch
needs rebase.

Re: [PATCH 2/2] Make GDB resolve dlopen of memory mapped shared libraries

[Alexandra Petlanova Hajkova]

[-- Attachment #1: Type: text/plain, Size: 1036 bytes --]

On Mon, Oct 17, 2022 at 9:39 PM Asaf Fisher via Gdb-patches <
gdb-patches@sourceware.org> wrote:

> Introduced `check_proc_self_file` that checks if a path used by
> inferior in dlopen is in the form of `/proc/self/...` and if so resolves
> it to `/proc/[pid]/...`
>
> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=29586
> ---
>  gdb/solib-svr4.c | 58 ++++++++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 56 insertions(+), 2 deletions(-)
>
> diff --git a/gdb/solib-svr4.c b/gdb/solib-svr4.c
> index 7e83819a03d..231a4fb40e5 100644
> --- a/gdb/solib-svr4.c
> +++ b/gdb/solib-svr4.c
> @@ -34,6 +34,7 @@
>  #include "regcache.h"
>  #include "gdbthread.h"
>  #include "observable.h"
> +#include "gdbsupport/pathstuff.h"
>
>  #include "solist.h"
>  #include "solib.h"
> @@ -46,6 +47,9 @@
>  #include "gdb_bfd.h"
>  #include "probe.h"
>
> Looks correct to me. I can confirm gdb.base/solib-proc-self.exp test added
> by [PATCH 1/2]

fails and hangs without this fix and passes after applying it. This patch
needs rebase.