File systemtap-CVE-2010-0411.diff of Package systemtap

Overview Repositories Revisions Requests Users Attributes Meta

File systemtap-CVE-2010-0411.diff of Package systemtap

Subject: VUL-1: systemtap: DoS issue in __get_argv() function
References: bnc#577382
Signed-Off-By: Tony Jones <tonyj@suse.de>

Comprises the following commits.

commit 561079c8601d7ded6fe958b4cec3d0f7aec1ee63
Author: Frank Ch. Eigler <fche@elastic.org>
Date:   Thu Oct 8 09:57:43 2009 -0400

PR10702: preprocessor conditional for kernel CONFIG_foo

* session.h (kernel_config[]): New session field.
    * main.cxx (parse_kernel_config): Populate it.
    * parse.cxx (eval_comparison): Use it.
    * testsuite/buildok/utrace.stp, testsuite/parseok/kconfig.stp: New tests.
    * NEWS, stap.1.in, doc/langref.tex: Mention it.

commit a2d399c87a642190f08ede63dc6fc434a5a8363a
Author: Josh Stone <jistone@redhat.com>
Date:   Thu Feb 4 17:47:31 2010 -0800

PR11234: Rewrite __get_argv without embedded-C

We now implement __get_argv's string building in pure stap script.
    Also, every argument is now quoted, which is different than before, but
    it's much more robust about handling special characters.

--- a/doc/langref.tex
+++ b/doc/langref.tex
@@ -1577,6 +1577,17 @@ probe kernel.function (
 %( arch == "ia64" %?
     probe syscall.vliw = kernel.function("vliw_widget") {}
 %)
+
+\end{verbatim}
+\end{vindent}
+
+The following code adapts to the presence of a kernel CONFIG option.
+
+\begin{vindent}
+\begin{verbatim}
+%( CONFIG_UTRACE == "y" %?
+    probe process.syscall {}
+%)
 \end{verbatim}
 \end{vindent}
 
--- a/main.cxx
+++ b/main.cxx
@@ -334,8 +334,8 @@ setup_signals (sighandler_t handler)
   sigaction (SIGTERM, &sa, NULL);
 }
 
-void
-setup_kernel_release (systemtap_session &s, const char* kstr) {
+void setup_kernel_release (systemtap_session &s, const char* kstr)
+{
     if (kstr[0] == '/') // fully specified path
       {
         s.kernel_build_tree = kstr;
@@ -362,6 +362,28 @@ setup_kernel_release (systemtap_session
         s.kernel_build_tree = "/lib/modules/" + s.kernel_release + "/build";
       }
 }
+
+void parse_kernel_config (systemtap_session &s)
+{
+  // PR10702: pull config options
+  string kernel_config_file = s.kernel_build_tree + "/.config";
+  ifstream kcf (kernel_config_file.c_str());
+  string line;
+  while (getline (kcf, line))
+    {
+      if (line.substr(0, 7) != "CONFIG_") continue;
+      size_t off = line.find('=');
+      if (off == string::npos) continue;
+      string key = line.substr(0, off);
+      string value = line.substr(off+1, string::npos);
+      s.kernel_config[key] = value;
+    }
+  if (s.verbose > 2)
+    clog << "Parsed kernel \"" << kernel_config_file << "\", number of tuples: " << s.kernel_config.size() << endl;
+
+  kcf.close();
+}
+
 int
 main (int argc, char * const argv [])
 {
@@ -935,6 +957,9 @@ main (int argc, char * const argv [])
       clog << "Created temporary directory \"" << s.tmpdir << "\"" << endl;
   }
 
+  // Now that no further changes to s.kernel_build_tree can occur, let's use it.
+  parse_kernel_config (s);
+
   // Create the name of the C source file within the temporary
   // directory.
   s.translated_source = string(s.tmpdir) + "/" + s.module_name + ".c";
@@ -1018,7 +1043,7 @@ main (int argc, char * const argv [])
           // GLOB_NOMATCH is acceptable
 
           if (s.verbose>1 && globbuf.gl_pathc > 0)
-            clog << "Searched '" << dir << "', "
+            clog << "Searched \"" << dir << "\", "
                  << "found " << globbuf.gl_pathc << endl;
 
           for (unsigned j=0; j<globbuf.gl_pathc; j++)
--- a/parse.cxx
+++ b/parse.cxx
@@ -183,12 +183,14 @@ bool eval_comparison (const OPERAND& lhs
 // The basic form is %( CONDITION %? THEN-TOKENS %: ELSE-TOKENS %)
 // where CONDITION is: kernel_v[r] COMPARISON-OP "version-string"
 //                 or: arch COMPARISON-OP "arch-string"
+//                 or: CONFIG_foo COMPARISON-OP "config-string"
 //                 or: "string1" COMPARISON-OP "string2"
 //                 or: number1 COMPARISON-OP number2
 // The %: ELSE-TOKENS part is optional.
 //
 // e.g. %( kernel_v > "2.5" %? "foo" %: "baz" %)
 // e.g. %( arch != "i?86" %? "foo" %: "baz" %)
+// e.g. %( CONFIG_foo %? "foo" %: "baz" %)
 //
 // Up to an entire %( ... %) expression is processed by a single call
 // to this function.  Tokens included by any nested conditions are
@@ -270,6 +272,23 @@ bool eval_pp_conditional (systemtap_sess
 
       return result;
     }
+  else if (l->type == tok_identifier && l->content.substr(0,7) == "CONFIG_" && r->type == tok_string)
+    {
+      string lhs = s.kernel_config[l->content]; // may be empty
+      string rhs = r->content;
+
+      int nomatch = fnmatch (lhs.c_str(), rhs.c_str(), FNM_NOESCAPE); // still spooky
+
+      bool result;
+      if (op->type == tok_operator && op->content == "==")
+        result = !nomatch;
+      else if (op->type == tok_operator && op->content == "!=")
+        result = nomatch;
+      else
+        throw parse_error ("expected '==' or '!='", op);
+
+      return result;
+    }
   else if (l->type == tok_string && r->type == tok_string)
     {
       string lhs = l->content;
@@ -291,10 +310,8 @@ bool eval_pp_conditional (systemtap_sess
 	    && op->type == tok_operator)
     throw parse_error ("expected number literal as right value", r);
 
-  // XXX: support other forms?  "CONFIG_SMP" ?
-
   else
-    throw parse_error ("expected 'arch' or 'kernel_v' or 'kernel_vr'\n"
+    throw parse_error ("expected 'arch' or 'kernel_v' or 'kernel_vr' or 'CONFIG_...'\n"
 		       "             or comparison between strings or integers", l);
 }
 
--- a/session.h
+++ b/session.h
@@ -87,6 +87,7 @@ struct systemtap_session
   std::string kernel_release;
   std::string kernel_base_release;
   std::string kernel_build_tree;
+  std::map<std::string,std::string> kernel_config;
   std::string architecture;
   std::string runtime_path;
   std::string data_path;
--- a/stap.1.in
+++ b/stap.1.in
@@ -340,12 +340,21 @@ by
 If, on the other hand, the first part is the identifier
 .BR arch
 to refer to the processor architecture (as named by the kernel
-build system ARCH/SUBARCH), then the second part then the second
+build system ARCH/SUBARCH), then the second
 part is one of the two string comparison operators
 .BR == " or " != ,
 and the third part is a string literal for matching it.  This
 comparison is a wildcard (mis)match.
 .PP
+Similarly, if the first part is an identifier like
+.BR CONFIG_something
+to refer to a kernel configuration option, then the second part is
+.BR == " or " != ,
+and the third part is a string literal for matching the value
+(commonly "y" or "m").  Nonexistent or unset kernel configuration
+options are represented by the empty string.  This comparison is also
+a wildcard (mis)match.
+.PP
 Otherwise, the CONDITION is expected to be a comparison between two string
 literals or two numeric literals.  In this case, the arguments are the only
 variables usable.
--- a/tapset/aux_syscalls.stp
+++ b/tapset/aux_syscalls.stp
@@ -399,124 +399,53 @@ function __sem_flags:string(semflg:long)
 
 
 /* This function copies an argv from userspace. */
-function __get_argv:string(a:long, first:long)
-%{ /* pure */
-  	char __user *__user *argv = (char __user *__user *)(long)THIS->a;
-	char __user *vstr;
-	int space, rc, len = MAXSTRINGLEN;
-	char *str = THIS->__retvalue;
-	char buf[80];
-	char *ptr = buf;
-
-	
-	if (THIS->first && argv)
-		argv++;
-
-	while (argv != NULL) {
-		if (__stp_get_user (vstr, argv))
-      			break;
-
-		if (vstr == NULL)
-			break;
-
-		rc = _stp_strncpy_from_user(buf, vstr, 79);
-		if (rc <= 0)
-			break;
-
-		/* check for whitespace in string */
-		buf[rc] = 0;
-		ptr = buf;
-		space = 0;
-		while (*ptr && rc--) {
-			if (isspace(*ptr++)) {
-				space = 1;
-				break;
-			}
-		}
-
-		if (len != MAXSTRINGLEN && len) {
-			*str++=' ';
-			len--;
-		}
-
-		if (space && len) {
-			*str++='\"';
-			len--;
-		}
-	
-		rc = strlcpy (str, buf, len); 
-		str += rc;
-		len -= rc;
-
-		if (space && len) {
-			*str++='\"';
-			len--;
-		}
-
-		argv++;
+function __get_argv:string(argv:long, first:long)
+{
+%( CONFIG_64BIT == "y" %?
+	if (first && argv)
+		argv += 8
+	while (argv) {
+		vstr = user_long(argv)
+		if (!vstr)
+			break
+		if (len)
+			str .= " "
+		str .= user_string_quoted(vstr)
+
+		newlen = strlen(str)
+		if (newlen == len)
+			break
+		len = newlen
+		argv += 8
 	}
-	*str = 0;
-%}
-/* This function copies an argv from userspace. */
-function __get_compat_argv:string(a:long, first:long)
-%{ /* pure */
-#ifdef CONFIG_COMPAT
-  	compat_uptr_t __user *__user *argv = (compat_uptr_t __user *__user *)(long)THIS->a;
-	compat_uptr_t __user *vstr;
-	int space, rc, len = MAXSTRINGLEN;
-	char *str = THIS->__retvalue;
-	char buf[80];
-	char *ptr = buf;
-
-	if (THIS->first && argv)
-		argv++;
-
-	while (argv != NULL) {
-		if (__stp_get_user (vstr, argv))
-      			break;
-
-		if (vstr == NULL)
-			break;
-
-		rc = _stp_strncpy_from_user(buf, (char *)vstr, 79);
-		if (rc <= 0)
-			break;
-
-		/* check for whitespace in string */
-		buf[rc] = 0;
-		ptr = buf;
-		space = 0;
-		while (*ptr && rc--) {
-			if (isspace(*ptr++)) {
-				space = 1;
-				break;
-			}
-		}
-
-		if (len != MAXSTRINGLEN && len) {
-			*str++=' ';
-			len--;
-		}
-
-		if (space && len) {
-			*str++='\"';
-			len--;
-		}
-	
-		rc = strlcpy (str, buf, len); 
-		str += rc;
-		len -= rc;
-
-		if (space && len) {
-			*str++='\"';
-			len--;
-		}
 
-		argv++;
+	return str
+%:
+	return __get_compat_argv(argv, first)
+%)
+}
+/* This function copies an argv from userspace. */
+function __get_compat_argv:string(argv:long, first:long)
+{
+	if (first && argv)
+		argv += 4
+	while (argv) {
+		vstr = user_int(argv) & 0xffffffff
+		if (!vstr)
+			break
+		if (len)
+			str .= " "
+		str .= user_string_quoted(vstr)
+
+		newlen = strlen(str)
+		if (newlen == len)
+			break
+		len = newlen
+		argv += 4
 	}
-	*str = 0;
-#endif
-%}
+
+	return str
+}
 
 /*
  * Return the  symbolic string  representation
--- /dev/null
+++ b/testsuite/buildok/utrace.stp
@@ -0,0 +1,3 @@
+#! stap -p4
+
+probe   %( CONFIG_UTRACE == "y" %? process.begin %: never %)    { }
--- /dev/null
+++ b/testsuite/parseok/kconfig.stp
@@ -0,0 +1,3 @@
+#! stap -p2
+
+%( CONFIG_NO_SUCH_CONFIG == "" %? probe never {} %: %)

Places

File systemtap-CVE-2010-0411.diff of Package systemtap

Places