[PATCH] Bug fix for security_ops->capable calls

sdsat_private

I've been working on moving more of the capabilities code into
the capabilities module, and I noticed some bugs in the current 
LSM code.  I've attached a minimal patch that simply fixes these bugs
without introducing any of my other changes.  Specifically, the
bugs occur when the kernel directly calls security_ops->capable 
(as in fs/exec.c:must_not_trace_exec to replace the cap_raised
check on the parent process) and when the capability plug directly
calls its cap_capable function.  

Our security_ops->capable() hook differs in two ways from the capable()
function.  First, it takes an explicit task_struct parameter, allowing
us to use it both for capable() calls and for various cap_raised() calls
in the kernel.  Second, it follows the same return code convention as the
other LSM hooks (and most Linux kernel functions) by returning 0 on
success and a negative error value on failure, whereas capable() and
cap_raised() are boolean predicates.  Our replacement for the capable()
function correctly hides these differences, so existing calls to capable()
are fine.  However, to replace the cap_raised() check in
must_not_trace_exec, we directly invoke security_ops->capable()
so that we can pass the parent task.  But our usage is opposite of
what it should be, since security_ops->capable() has the opposite
semantics of cap_raised().  This is the most serious error, since
it affects every module, including the dummy one.

Likewise, in the capability plug itself, we have a similar
must_not_trace_exec test for the capability bits which has
the same error.  We also have a couple of cases where we
directly call cap_capable() rather than capable() to avoid
the extra function call overhead.  However, I've reverted these
to use capable() to avoid confusion.  So, at present, we are only
directly invoking security_ops->capable/cap_capable to replace
cap_raised calls.

--
Stephen D. Smalley, NAI Labs
ssmalleyat_private





Index: lsm/fs/exec.c
diff -u lsm/fs/exec.c:1.11 lsm/fs/exec.c:1.12

--- lsm/fs/exec.c:1.11	Tue Aug  7 09:15:56 2001
+++ lsm/fs/exec.c	Tue Aug 14 08:17:00 2001
@@ -593,7 +593,7 @@
  */
 static inline int must_not_trace_exec(struct task_struct * p)
 {
-	return (p->ptrace & PT_PTRACED) && !security_ops->capable(p->p_pptr, CAP_SYS_PTRACE);
+	return (p->ptrace & PT_PTRACED) && security_ops->capable(p->p_pptr, CAP_SYS_PTRACE);
 }
 
 /* 
Index: lsm/security/capability_plug.c
diff -u lsm/security/capability_plug.c:1.21 lsm/security/capability_plug.c:1.22
--- lsm/security/capability_plug.c:1.21	Mon Aug 13 09:54:37 2001
+++ lsm/security/capability_plug.c	Tue Aug 14 08:17:00 2001
@@ -54,7 +54,7 @@
 { 
 	/* Derived from arch/i386/kernel/ptrace.c:sys_ptrace. */
 	if (!cap_issubset(child->cap_permitted, current->cap_permitted) &&
-	    !cap_capable(current, CAP_SYS_PTRACE))
+	    !capable(CAP_SYS_PTRACE))
 		return -EPERM;
 	else
 		return 0;
@@ -99,7 +99,7 @@
 /* Copied from fs/exec.c */
 static inline int must_not_trace_exec(struct task_struct * p)
 {
-	return (p->ptrace & PT_PTRACED) && !cap_capable(p->p_pptr, CAP_SYS_PTRACE);
+	return (p->ptrace & PT_PTRACED) && cap_capable(p->p_pptr, CAP_SYS_PTRACE);
 }
 
 static void cap_binprm_compute_creds(struct linux_binprm *bprm)
@@ -121,7 +121,7 @@
 		    || atomic_read(&current->fs->count) > 1
 		    || atomic_read(&current->files->count) > 1
 		    || atomic_read(&current->sig->count) > 1) {
-			if(!cap_capable(current,CAP_SETPCAP)) {
+			if(!capable(CAP_SETPCAP)) {
 				new_permitted = cap_intersect(new_permitted,
 							current->cap_permitted);
 			}

_______________________________________________
linux-security-module mailing list
linux-security-moduleat_private
http://mail.wirex.com/mailman/listinfo/linux-security-module
