[PATCH] LSM: Reorder security_capset to do access checks properly

[Casey Schaufler <casey@schaufler-ca.com>]

Subject: [PATCH] LSM: Reorder security_capset to do access checks properly

The security module hooks that check whether a process should
be able to set a new capset are currently called after the new
values are set in cap_capset(). This change reverses the order.
The capability module no longer adds cap_capset to the module list.
Instead, it is invoked directly by the LSM infrastructure.
This isn't an approach that generalizes well.

Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
---
 security/commoncap.c |  2 +-
 security/security.c  | 24 ++++++++++++++++++++++--
 2 files changed, 23 insertions(+), 3 deletions(-)

diff --git a/security/commoncap.c b/security/commoncap.c
index 48071ed..f5bce18 100644
--- a/security/commoncap.c
+++ b/security/commoncap.c
@@ -1073,7 +1073,7 @@ struct security_hook_list capability_hooks[] = {
 	LSM_HOOK_INIT(ptrace_access_check, cap_ptrace_access_check),
 	LSM_HOOK_INIT(ptrace_traceme, cap_ptrace_traceme),
 	LSM_HOOK_INIT(capget, cap_capget),
-	LSM_HOOK_INIT(capset, cap_capset),
+	/* Carefull! Do not include cap_capset! */
 	LSM_HOOK_INIT(bprm_set_creds, cap_bprm_set_creds),
 	LSM_HOOK_INIT(bprm_secureexec, cap_bprm_secureexec),
 	LSM_HOOK_INIT(inode_need_killpriv, cap_inode_need_killpriv),
diff --git a/security/security.c b/security/security.c
index 92cd1d8..1610be8 100644
--- a/security/security.c
+++ b/security/security.c
@@ -177,8 +177,28 @@ int security_capset(struct cred *new, const struct cred *old,
 		    const kernel_cap_t *inheritable,
 		    const kernel_cap_t *permitted)
 {
-	return call_int_hook(capset, 0, new, old,
-				effective, inheritable, permitted);
+	struct security_hook_list *hp;
+	int rc;
+
+	/*
+	 * Special case handling because the "new" capabilities
+	 * should not be set until it has been determined that
+	 * all modules approve of the change. Passing NULL pointers
+	 * to all modules except the capabilty module as it is
+	 * expected that only the capability modules needs the
+	 * result pointers.
+	 *
+	 * cap_capset() must not be in the capability module hook list!
+	 */
+	list_for_each_entry(hp, &security_hook_heads.capset, list) {
+		rc = hp->hook.capset(new, old, NULL, NULL, NULL);
+		if (rc != 0)
+			return rc;
+	}
+	/*
+	 * Call cap_capset now to update the new capset.
+	 */
+	return cap_capset(new, old, effective, inheritable, permitted);
 }
 
 int security_capable(const struct cred *cred, struct user_namespace *ns,