[systemtap/f14] CVE-2011-2502, CVE-2011-2503

fche fche at fedoraproject.org
Mon Jul 25 16:13:39 UTC 2011


commit cd023a1a5891e016ab9aad13f90d6a980ec1b699
Author: Frank Ch. Eigler <fche at redhat.com>
Date:   Mon Jul 25 12:08:32 2011 -0400

    CVE-2011-2502, CVE-2011-2503

 cve-2011-2502.patch |   60 ++++++++++++++++++++++++++++++++++
 cve-2011-2503.patch |   88 +++++++++++++++++++++++++++++++++++++++++++++++++++
 systemtap.spec      |   11 ++++++-
 3 files changed, 158 insertions(+), 1 deletions(-)
---
diff --git a/cve-2011-2502.patch b/cve-2011-2502.patch
new file mode 100644
index 0000000..a681b62
--- /dev/null
+++ b/cve-2011-2502.patch
@@ -0,0 +1,60 @@
+commit e75e70e736ea53078eaa9fd36a5f7186e3e2235c
+Author: Josh Stone <jistone at redhat.com>
+Date:   Fri Jun 24 14:21:26 2011 -0700
+
+    rhbz716476: Don't allow path-based auth for uprobes
+    
+    For users that are only members of stapusr, and not stapdev, we only
+    allow loading modules that are either signed with a trusted certificate
+    or located in controlled paths.  For the script itself, that path is
+    /lib/modules/.../systemtap/, and for uprobes it is the runtime.  When
+    this policy was first written, uprobes only ever came from the runtime
+    path, so the path check just returned 1 always.
+    
+    Later, commit 474d17ad added an optional argument to staprun -u, to
+    allow the user to specify their own signed copy of uprobes to load.
+    Unfortunately, if presented with an unsigned module, that would still
+    fall back to the path check, which blissfully approved it anyway.
+    
+    Our policy is now that stapusr can only load a signed uprobes.ko, so the
+    path check for uprobes now unconditionally returns 0.
+
+diff --git a/runtime/staprun/staprun_funcs.c b/runtime/staprun/staprun_funcs.c
+index 74eef9c..82754d4 100644
+--- a/runtime/staprun/staprun_funcs.c
++++ b/runtime/staprun/staprun_funcs.c
+@@ -387,8 +387,10 @@ check_stap_module_path(const char *module_path, int module_fd)
+ }
+ 
+ /*
+- * Members of the 'stapusr' group can load the uprobes module freely,
+- * since it is loaded from a fixed path in the installed runtime.
++ * Don't allow path-based authorization for the uprobes module at all.
++ * Members of the 'stapusr' group can load a signed uprobes module, but
++ * nothing else.  Later we could consider allowing specific paths, like
++ * the installed runtime or /lib/modules/...
+  *
+  * Returns: -1 on errors, 0 on failure, 1 on success.
+  */
+@@ -398,7 +400,7 @@ check_uprobes_module_path (
+   int module_fd __attribute__ ((unused))
+ )
+ {
+-  return 1;
++  return 0;
+ }
+ 
+ /*
+@@ -596,10 +598,8 @@ void assert_uprobes_module_permissions(
+ 	if (check_signature_rc == MODULE_ALTERED)
+ 		exit(-1);
+ #else
+-	/* If we don't have NSS, then the uprobes module is considered trusted.
+-	   Otherwise a member of the group 'stapusr' will not be able to load it.
+-	 */
+-	check_signature_rc = MODULE_OK;
++	/* If we don't have NSS, the uprobes module is considered untrusted. */
++	check_signature_rc = MODULE_UNTRUSTED;
+ #endif
+ 
+ 	/* root can still load this module.  */
diff --git a/cve-2011-2503.patch b/cve-2011-2503.patch
new file mode 100644
index 0000000..652b61f
--- /dev/null
+++ b/cve-2011-2503.patch
@@ -0,0 +1,88 @@
+commit fa6b56faaa56c98203dcc3fbdda5eab3d91ec62d
+Author: Josh Stone <jistone at redhat.com>
+Date:   Fri Jun 24 15:00:41 2011 -0700
+
+    rhbz716489: read instead of mmap to load modules
+    
+    As staprun is preparing to load a kernel module, we first mmap the whole
+    module as MAP_PRIVATE. Then we proceed with our security checks,
+    including a trusted-signature validation on the mapped region, and if
+    all checks out, we'll call init_module() with that same mapped region.
+    
+    However, MMAP(2) says of MAP_PRIVATE, "It is unspecified whether changes
+    made to the file after the mmap() call are visible in the mapped
+    region."  From my testing, it appears that file changes do indeed show
+    up in our mapped memory.  This means we have a TOCTOU race between
+    verifying the signature of that memory and then calling init_module().
+    
+    By using read() instead of mmap(), we ensure that we have a fully
+    private copy of the module to verify and load, without fear of change.
+
+diff --git a/runtime/staprun/staprun_funcs.c b/runtime/staprun/staprun_funcs.c
+index 74eef9c..e0a5a46 100644
+--- a/runtime/staprun/staprun_funcs.c
++++ b/runtime/staprun/staprun_funcs.c
+@@ -49,7 +49,7 @@ int insert_module(
+   assert_permissions_func assert_permissions
+ ) {
+ 	int i;
+-	long ret;
++	long ret, module_read;
+ 	void *module_file;
+ 	char *opts;
+ 	int saved_errno;
+@@ -109,17 +109,39 @@ int insert_module(
+ 		return -1;
+ 	}
+ 
+-	/* mmap in the entire module. Work with the memory mapped data from this
+-	   point on to avoid a TOCTOU race between path and signature checking
+-	   below and module loading.  */
+-	module_file = mmap(NULL, sbuf.st_size, PROT_READ, MAP_PRIVATE, module_fd, 0);
+-	if (module_file == MAP_FAILED) {
+-		_perr("Error mapping '%s'", module_realpath);
++	/* Allocate memory for the entire module. */
++	module_file = calloc(1, sbuf.st_size);
++	if (module_file == NULL) {
++		_perr("Error allocating memory to read '%s'", module_realpath);
+ 		close(module_fd);
+ 		free(opts);
+ 		return -1;
+ 	}
+ 
++	/* read in the entire module.  Work with this copy of the data from this
++	   point on to avoid a TOCTOU race between path and signature checking
++	   below and module loading.  */
++	module_read = 0;
++	while (module_read < sbuf.st_size) {
++		ret = read(module_fd, module_file + module_read,
++				sbuf.st_size - module_read);
++		if (ret > 0)
++			module_read += ret;
++		else if (ret == 0) {
++			_err("Unexpected EOF reading '%s'", module_realpath);
++			free(module_file);
++			close(module_fd);
++			free(opts);
++			return -1;
++		} else if (errno != EINTR) {
++			_perr("Error reading '%s'", module_realpath);
++			free(module_file);
++			close(module_fd);
++			free(opts);
++			return -1;
++		}
++	}
++
+ 	/* Check whether this module can be loaded by the current user.
+ 	 * check_permissions will exit(-1) if permissions are insufficient*/
+ 	assert_permissions (module_realpath, module_fd, module_file, sbuf.st_size);
+@@ -131,7 +153,7 @@ int insert_module(
+ 
+ 	/* Cleanup. */
+ 	free(opts);
+-	munmap(module_file, sbuf.st_size);
++	free(module_file);
+ 	close(module_fd);
+ 
+ 	if (ret != 0) {
diff --git a/systemtap.spec b/systemtap.spec
index 9b38da8..0018150 100644
--- a/systemtap.spec
+++ b/systemtap.spec
@@ -16,7 +16,7 @@
 
 Name: systemtap
 Version: 1.5
-Release: 7%{?dist}
+Release: 8%{?dist}
 # for version, see also configure.ac
 Summary: Instrumentation System
 Group: Development/System
@@ -66,6 +66,9 @@ BuildRequires: m4
 BuildRequires: elfutils-devel >= %{elfutils_version}
 %endif
 
+Patch4: cve-2011-2502.patch
+Patch5: cve-2011-2503.patch
+
 %if %{with_docs}
 BuildRequires: /usr/bin/latex /usr/bin/dvips /usr/bin/ps2pdf latex2html
 # On F10, xmlto's pdf support was broken off into a sub-package,
@@ -185,6 +188,9 @@ find . \( -name configure -o -name config.h.in \) -print | xargs touch
 cd ..
 %endif
 
+%patch4 -p1
+%patch5 -p1
+
 %patch25 -p1
 
 %build
@@ -502,6 +508,9 @@ exit 0
 
 
 %changelog
+* Mon Jul 25 2011 Frank Ch. Eigler <fche at redhat.com> - 1.5-8
+- CVE-2011-2502, CVE-2011-2503
+
 * Fri Jul 15 2011 William Cohen <wcohen at redhat.com> - 1.5-7
 - Fix sdt.h to avoid warning on arm arches.
 


More information about the scm-commits mailing list