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

Message ID 20221017193858.3006-3-asaffisher.dev@gmail.com
State New
Headers
Series GDB hangs when trying to resolve memory mapped shared libraries of an inferior under /proc/self/fd/* |

Commit Message

Asaf Fisher Oct. 17, 2022, 7:38 p.m. UTC
  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(-)
  

Comments

Alexandra Petlanova Hajkova Oct. 21, 2022, 10:50 a.m. UTC | #1
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.
  

Patch

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);