Sanity check VG_(realpath) and VG_(readlink) return values

When VG_(realpath) calls VG_(readlink) it failed to check if
VG_(readlink) succeeds, possibly writing to tmp[-1] (on the stack). It
also didn't check the getcwd syscall succeeded, which would cause the
resolved name to start with undefined bits (from the stack).

VG_(data_size) was using too small a (stack) buffer for the
VG_(realpath) call and didn't check whether the call actually
succeeded.

At startup initimg-{darwin,freebsd,linux,solaris} also didn't check
VG_(realpath) would resolve before setting VG_(resolved_exename) to
possibly random bits on the stack. Fix that by using the (unresolved)
exe_name in those cases.

https://bugs.kde.org/show_bug.cgi?id=515612

diff --git a/NEWS b/NEWS
index bc42d644339655f5f126c73528b4c609fa52f315..93c0a0c7e0808d6900633a0c1e8aff3e9243f6bf 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -80,6 +80,7 @@ are not entered into bugzilla tend to get forgotten about or ignored.
 515183  Error occurred while executing the command
         `valgrind --num-callers=1 ./hello_world`
 515265  Add SSE4.1 BLENDPS and PBLENDW instruction for x86 32 bit
+515612  Sanity check VG_(realpath) and VG_(readlink) return values
 
 
 To see details of a given bug, visit

diff --git a/coregrind/m_debuginfo/debuginfo.c b/coregrind/m_debuginfo/debuginfo.c
index 8c63cfc390b9a2db1e9ab6f02eaca6fed13b119a..6f3c281e1dd4f2af766142fd62258e6685988cac 100644 (file)
--- a/coregrind/m_debuginfo/debuginfo.c
+++ b/coregrind/m_debuginfo/debuginfo.c
@@ -5235,12 +5235,14 @@ void VG_(load_all_debuginfo) (void)
 
 SizeT VG_(data_size)(void)
 {
-   HChar resolved[1000];
-   VG_(realpath)( VG_(args_the_exename), resolved);
-
-   for (DebugInfo* di = debugInfo_list; di; di = di->next) {
-      if (di->data_size  && VG_(strcmp)(di->soname, "NONE") == 0 && VG_(strcmp)(resolved, di->fsm.filename) == 0) {
-         return VG_PGROUNDUP(di->data_size);
+   HChar resolved[VKI_PATH_MAX];
+   if (VG_(realpath)( VG_(args_the_exename), resolved)) {
+      for (DebugInfo* di = debugInfo_list; di; di = di->next) {
+         if (di->data_size
+             && VG_(strcmp)(di->soname, "NONE") == 0
+             && VG_(strcmp)(resolved, di->fsm.filename) == 0) {
+            return VG_PGROUNDUP(di->data_size);
+         }
       }
    }
    return 0U;

diff --git a/coregrind/m_initimg/initimg-darwin.c b/coregrind/m_initimg/initimg-darwin.c
index 869f5ff83c9605c1613e474450573775e73688ff..3a00e7ef06b0223ff47a998104b5313a87754db2 100644 (file)
--- a/coregrind/m_initimg/initimg-darwin.c
+++ b/coregrind/m_initimg/initimg-darwin.c
@@ -504,8 +504,13 @@ Addr setup_client_stack( void*  init_sp,
          exe_name = interp_name;
       }
       HChar resolved_name[VKI_PATH_MAX];
-      VG_(realpath)(exe_name, resolved_name);
-      VG_(resolved_exename) = VG_(strdup)("initimg-darwin.scs.2", resolved_name);
+      if (VG_(realpath)(exe_name, resolved_name)) {
+         VG_(resolved_exename) = VG_(strdup)("initimg-darwin.scs.2", resolved_name);
+      } else {
+         /* This should not really happen. realpath tried and failed.
+            So lets just continue with the exe_name as is. */
+         VG_(resolved_exename) = VG_(strdup)("initimg-darwin.scs.3", exe_name);
+      }
    }
 
    /* client_SP is pointing at client's argc/argv */

diff --git a/coregrind/m_initimg/initimg-freebsd.c b/coregrind/m_initimg/initimg-freebsd.c
index c871f47bb20ed944808f9c8b856e4b4cb49a43f8..9e95bbfa0cc646a2d6d50772fffd597ddc2a486c 100644 (file)
--- a/coregrind/m_initimg/initimg-freebsd.c
+++ b/coregrind/m_initimg/initimg-freebsd.c
@@ -421,7 +421,11 @@ static Addr setup_client_stack(const void*  init_sp,
       exe_name = interp_name;
    }
    HChar resolved_name[VKI_PATH_MAX];
-   VG_(realpath)(exe_name, resolved_name);
+   if (!VG_(realpath)(exe_name, resolved_name)) {
+      /* This should not really happen. realpath tried and failed.
+         So lets just continue with the exe_name as is. */
+      VG_(strcpy)(resolved_name, exe_name);
+   }
 
    /* use our own auxv as a prototype */
    orig_auxv = find_auxv(init_sp);

diff --git a/coregrind/m_initimg/initimg-linux.c b/coregrind/m_initimg/initimg-linux.c
index a33710c39ee304802ef9010e2265f289bbf7b8a9..acda16dda1eeb6d189976419dc99d9fda7685f92 100644 (file)
--- a/coregrind/m_initimg/initimg-linux.c
+++ b/coregrind/m_initimg/initimg-linux.c
@@ -960,8 +960,13 @@ Addr setup_client_stack( void*  init_sp,
          exe_name = interp_name;
       }
       HChar resolved_name[VKI_PATH_MAX];
-      VG_(realpath)(exe_name, resolved_name);
-      VG_(resolved_exename) = VG_(strdup)("initimg-linux.scs.1", resolved_name);
+      if (VG_(realpath)(exe_name, resolved_name)) {
+         VG_(resolved_exename) = VG_(strdup)("initimg-linux.scs.1", resolved_name);
+      } else {
+         /* This should not really happen. realpath tried and failed.
+            So lets just continue with the exe_name as is. */
+         VG_(resolved_exename) = VG_(strdup)("initimg-linux.scs.2", exe_name);
+      }
    }
 
    /* client_SP is pointing at client's argc/argv */

diff --git a/coregrind/m_initimg/initimg-solaris.c b/coregrind/m_initimg/initimg-solaris.c
index bd2d822e5d208f3cf9c21ce3456e46138cb8cb2c..7c9cf0efc6cf081c22622a3ccc4803a00a28165b 100644 (file)
--- a/coregrind/m_initimg/initimg-solaris.c
+++ b/coregrind/m_initimg/initimg-solaris.c
@@ -100,8 +100,13 @@ static void load_client(/*OUT*/ExeInfo *info,
          exe_name = interp_name;
       }
       HChar resolved_name[VKI_PATH_MAX];
-      VG_(realpath)(exe_name, resolved_name);
-      VG_(resolved_exename) = VG_(strdup)("initimg-solaris.lc.1", resolved_name);
+      if (VG_(realpath)(exe_name, resolved_name)) {
+         VG_(resolved_exename) = VG_(strdup)("initimg-solaris.lc.1", resolved_name);
+      } else {
+         /* This should not really happen. realpath tried and failed.
+            So lets just continue with the exe_name as is. */
+         VG_(resolved_exename) = VG_(strdup)("initimg-solaris.lc.2", exe_name);
+      }
    }
 
    /* Set initial brk values. */

diff --git a/coregrind/m_libcfile.c b/coregrind/m_libcfile.c
index 49abe4c172640ff3650d8dad34f70d19915ed62f..ca3ecdc9a471a66191daf1fe51673627fec44873 100644 (file)
--- a/coregrind/m_libcfile.c
+++ b/coregrind/m_libcfile.c
@@ -1867,6 +1867,8 @@ Bool VG_(realpath)(const HChar *path, HChar *resolved)
 
    if (VKI_S_ISLNK(statbuf.mode)) {
       SizeT link_len = VG_(readlink)(path, tmp, VKI_PATH_MAX);
+      if (link_len < 0)
+         return False;
       tmp[link_len] = '\0';
       resolved_name = tmp;
    } else {
@@ -1885,6 +1887,9 @@ Bool VG_(realpath)(const HChar *path, HChar *resolved)
 #elif defined(VGO_freebsd)
       res = VG_(do_syscall2)(__NR___getcwd, (UWord)wd, VKI_PATH_MAX);
 #endif
+      if (sr_isError(res)) {
+         return False;
+      }
       VG_(snprintf)(resolved, VKI_PATH_MAX, "%s/%s", wd, resolved_name);
    } else {
       VG_(snprintf)(resolved, VKI_PATH_MAX, "%s", resolved_name);