[PATCH] properly split capset_check+capset_set

[Serge E. Hallyn]

The attached patch removes checks from kernel/capability.c which are
redundant with cap_capset_check() code, and moves the capset_check()
calls to immediately before the capset_set() calls.  This allows
capset_check() to accurately check the setter's permission to set caps
on the target.  Please apply.

thanks,
-serge

Signed-off-by: Serge Hallyn <serue@xxxxxxxxxx>

Index: linux-2.6.10-mm1/kernel/capability.c
===================================================================
--- linux-2.6.10-mm1.orig/kernel/capability.c	2005-01-04 10:23:54.000000000 -0600
+++ linux-2.6.10-mm1/kernel/capability.c	2005-01-04 10:32:48.000000000 -0600
@@ -85,34 +85,60 @@ out:
  * cap_set_pg - set capabilities for all processes in a given process
  * group.  We call this holding task_capability_lock and tasklist_lock.
  */
-static inline void cap_set_pg(int pgrp, kernel_cap_t *effective,
+static inline int cap_set_pg(int pgrp, kernel_cap_t *effective,
 			      kernel_cap_t *inheritable,
 			      kernel_cap_t *permitted)
 {
 	task_t *g, *target;
+	int ret = -EPERM;
+	int found = 0;
 
 	do_each_task_pid(pgrp, PIDTYPE_PGID, g) {
 		target = g;
-		while_each_thread(g, target)
-			security_capset_set(target, effective, inheritable, permitted);
+		while_each_thread(g, target) {
+			if (!security_capset_check(target, effective,
+							inheritable,
+							permitted)) {
+				security_capset_set(target, effective,
+							inheritable,
+							permitted);
+				ret = 0;
+			}
+			found = 1;
+		}
 	} while_each_task_pid(pgrp, PIDTYPE_PGID, g);
+
+	if (!found)
+	     ret = 0;
+	return ret;
 }
 
 /*
  * cap_set_all - set capabilities for all processes other than init
  * and self.  We call this holding task_capability_lock and tasklist_lock.
  */
-static inline void cap_set_all(kernel_cap_t *effective,
+static inline int cap_set_all(kernel_cap_t *effective,
 			       kernel_cap_t *inheritable,
 			       kernel_cap_t *permitted)
 {
      task_t *g, *target;
+     int ret = -EPERM;
+     int found = 0;
 
      do_each_thread(g, target) {
              if (target == current || target->pid == 1)
                      continue;
+             found = 1;
+	     if (security_capset_check(target, effective, inheritable,
+						permitted))
+		     continue;
+	     ret = 0;
 	     security_capset_set(target, effective, inheritable, permitted);
      } while_each_thread(g, target);
+
+     if (!found)
+	     ret = 0;
+     return ret;
 }
 
 /*
@@ -167,36 +193,23 @@ asmlinkage long sys_capset(cap_user_head
      } else
                target = current;
 
-     ret = -EPERM;
-
-     if (security_capset_check(target, &effective, &inheritable, &permitted))
-	     goto out;
-
-     if (!cap_issubset(inheritable, cap_combine(target->cap_inheritable,
-                       current->cap_permitted)))
-             goto out;
-
-     /* verify restrictions on target's new Permitted set */
-     if (!cap_issubset(permitted, cap_combine(target->cap_permitted,
-                       current->cap_permitted)))
-             goto out;
-
-     /* verify the _new_Effective_ is a subset of the _new_Permitted_ */
-     if (!cap_issubset(effective, permitted))
-             goto out;
-
      ret = 0;
 
      /* having verified that the proposed changes are legal,
            we now put them into effect. */
      if (pid < 0) {
              if (pid == -1)  /* all procs other than current and init */
-                     cap_set_all(&effective, &inheritable, &permitted);
+                     ret = cap_set_all(&effective, &inheritable, &permitted);
 
              else            /* all procs in process group */
-                     cap_set_pg(-pid, &effective, &inheritable, &permitted);
+                     ret = cap_set_pg(-pid, &effective, &inheritable,
+		     					&permitted);
      } else {
-	     security_capset_set(target, &effective, &inheritable, &permitted);
+	     ret = security_capset_check(target, &effective, &inheritable,
+	     						&permitted);
+	     if (!ret)
+		     security_capset_set(target, &effective, &inheritable,
+		     					&permitted);
      }
 
 out:
Index: linux-2.6.10-mm1/security/selinux/hooks.c
===================================================================
--- linux-2.6.10-mm1.orig/security/selinux/hooks.c	2005-01-04 10:23:54.000000000 -0600
+++ linux-2.6.10-mm1/security/selinux/hooks.c	2005-01-04 10:24:19.000000000 -0600
@@ -1390,12 +1390,6 @@ static int selinux_capset_check(struct t
 static void selinux_capset_set(struct task_struct *target, kernel_cap_t *effective,
                                kernel_cap_t *inheritable, kernel_cap_t *permitted)
 {
-	int error;
-
-	error = task_has_perm(current, target, PROCESS__SETCAP);
-	if (error)
-		return;
-
 	secondary_ops->capset_set(target, effective, inheritable, permitted);
 }
 
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/