From: Quentin Monnet <quentin.monnet@netronome.com>
Date: Sat, 14 Apr 2018 22:23:11 +0000 (+0100)
Subject: Move pre_check for ASCII string out of PRE(sys_prctl)
X-Git-Tag: VALGRIND_3_14_0~58
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=c9d555dafad9215f8e9db941af012296e35106df;p=thirdparty%2Fvalgrind.git

Move pre_check for ASCII string out of PRE(sys_prctl)

The sys_prctl wrapper with PR_SET_NAME option reads an ASCII string passed
as its second argument. This string is supposed to be shorter than a given
limit. As the actual length of the string is unknown, the PRE() wrapper
performs a number of checks on it, including, in worst case, trying to
dereference it byte by byte.

To avoid re-implementing all this logic for other wrappers that could
need it, get the string processing out of the wrapper and move it to a
static function. Note that passing tid as an argument to the function is
required for macros PRE_MEM_RASCIIZ and PRE_MEM_READ to work properly.
---

diff --git a/coregrind/m_syswrap/syswrap-linux.c b/coregrind/m_syswrap/syswrap-linux.c
index bd7d447762..b0d65414a2 100644
--- a/coregrind/m_syswrap/syswrap-linux.c
+++ b/coregrind/m_syswrap/syswrap-linux.c
@@ -1389,6 +1389,36 @@ POST(sys_sysctl)
    }
 }
 
+static void pre_asciiz_str(ThreadId tid, Addr str, SizeT maxlen,
+                           const char *attr_name)
+{
+   const HChar *step_str = (const HChar *)str;
+   SizeT len;
+   UInt i;
+
+   /*
+    * The name can be up to maxlen bytes long, including the terminating null
+    * byte. So do not check more than maxlen bytes.
+    */
+   if (ML_(safe_to_deref)((const HChar *)str, maxlen)) {
+      len = VG_(strnlen)((const HChar *)str, maxlen);
+      if (len < maxlen)
+         PRE_MEM_RASCIIZ(attr_name, str);
+      else
+         PRE_MEM_READ(attr_name, str, maxlen);
+   } else {
+      /*
+       * Do it the slow way, one byte at a time, while checking for terminating
+       * '\0'.
+       */
+      for (i = 0; i < maxlen; i++) {
+         PRE_MEM_READ(attr_name, (Addr)&step_str[i], 1);
+         if (!ML_(safe_to_deref)(&step_str[i], 1) || step_str[i] == '\0')
+            break;
+      }
+   }
+}
+
 PRE(sys_prctl)
 {
    *flags |= SfMayBlock;
@@ -1442,27 +1472,7 @@ PRE(sys_prctl)
       break;
    case VKI_PR_SET_NAME:
       PRE_REG_READ2(int, "prctl", int, option, char *, name);
-      /* The name can be up to TASK_COMM_LEN(16) bytes long, including
-         the terminating null byte. So do not check more than 16 bytes. */
-      if (ML_(safe_to_deref)((const HChar *) (Addr)ARG2, VKI_TASK_COMM_LEN)) {
-         SizeT len = VG_(strnlen)((const HChar *) (Addr)ARG2,
-                                  VKI_TASK_COMM_LEN);
-         if (len < VKI_TASK_COMM_LEN) {
-            PRE_MEM_RASCIIZ("prctl(set-name)", ARG2);
-         } else {
-            PRE_MEM_READ("prctl(set-name)", ARG2, VKI_TASK_COMM_LEN);
-         }
-      } else {
-         /* Do it the slow way, one byte at a time, while checking for
-            terminating '\0'. */
-         const HChar *name = (const HChar *) (Addr)ARG2;
-         for (UInt i = 0; i < VKI_TASK_COMM_LEN; i++) {
-            PRE_MEM_READ("prctl(set-name)", (Addr) &name[i], 1);
-            if (!ML_(safe_to_deref)(&name[i], 1) || name[i] == '\0') {
-               break;
-            }
-         }
-      }
+      pre_asciiz_str(tid, ARG2, VKI_TASK_COMM_LEN, "prctl(set-name)");
       break;
    case VKI_PR_GET_NAME:
       PRE_REG_READ2(int, "prctl", int, option, char *, name);