Re: compute_creds fixup in -mm

[Chris Wright <chrisw@osdl.org>]

* Stephen Smalley (sds@epoch.ncsc.mil) wrote:
> On Wed, 2004-04-21 at 13:27, Andy Lutomirski wrote:
> > This doesn't fix selinux, though -- its apply_creds hook just blindly calls
> > commoncap's.  In fact, this breaks all attempts to get nested capability modules
> > right.  The problem is that, AFAICS, not only does ptrace_detach not take
> > task_lock, _exit() doesn't either.  So you get an equivalent race for the shared
> > state check.  I see two ways to fix that:
> 
> I didn't see Chris' patch.  I assume that the worst case is unexpected
> program failure due to lack of capability, right?  The SELinux security

The opposite.  You'd get a program with non-root euid, but full
capability set, and AT_SECURE set false.  My patch is below.

--- a/security/commoncap.c~compute_creds-lock	2004-04-21 00:54:34.000000000 -0700
+++ b/security/commoncap.c	2004-04-21 01:01:00.000000000 -0700
@@ -135,28 +135,26 @@
 
 	task_lock(current);
 
-	if (bprm->e_uid != current->uid || bprm->e_gid != current->gid) {
+	if (bprm->e_uid != current->uid || bprm->e_gid != current->gid ||
+	    !cap_issubset (new_permitted, current->cap_permitted)) {
 		current->mm->dumpable = 0;
 
-		if (must_not_trace_exec(current) && !capable(CAP_SETUID)) {
-			bprm->e_uid = current->uid;
-			bprm->e_gid = current->gid;
+		if (must_not_trace_exec(current)) {
+			if (!capable(CAP_SETUID)) {
+				bprm->e_uid = current->uid;
+				bprm->e_gid = current->gid;
+			}
+			if (!capable (CAP_SETPCAP)) {
+				new_permitted = cap_intersect (new_permitted,
+							current->cap_permitted);
+			}
+
 		}
 	}
 
 	current->suid = current->euid = current->fsuid = bprm->e_uid;
 	current->sgid = current->egid = current->fsgid = bprm->e_gid;
 
-	if (!cap_issubset (new_permitted, current->cap_permitted)) {
-		current->mm->dumpable = 0;
-
-		if (must_not_trace_exec (current) && !capable (CAP_SETPCAP)) {
-			new_permitted = cap_intersect (new_permitted,
-						       current->
-						       cap_permitted);
-		}
-	}
-
 	/* For init, we want to retain the capabilities set
 	 * in the init_task struct. Thus we skip the usual
 	 * capability rules */