Re: [ linus-git ] prctl(PR_SET_KEEPCAPS, ...) is broken for some configs, e.g. CONFIG_SECURITY_SELINUX

[Dmitry Adamushko <dmitry.adamushko@gmail.com>]

The following move-it-back-to-generic-place patch fixes the problem.


---
From: Dmitry Adamushko <dmitry.adamushko@gmail.com>
Subject: fix prctl()'s handling of PR_{SET,GET}_KEEPCAPS
    
with the commit 3898b1b4ebff8dcfbcf1807e0661585e06c9a91c

prctl(PR_SET_KEEPCAPS, {1 | 0}, 0, 0, 0);

always returns -EINVAL for the following configs:

1) CONFIG_SECURITY but without any of CONFIG_SECURITY_* modules;

2) CONFIG_SECURITY + CONFIG_SECURITY_SELINUX + CONFIG_SECURITY_SELINUX_DISABLE

both fall back to 'dummy' implementation.

3) CONFIG_SECURITY + CONFIG_SECURITY_SELINUX

for this config it will work when there is a secondary security module.

Here is what happens:

Processing of PR_SET_KEEPCAPS (and a couple of other options) has been
moved from kernel/sys.c::sys_prctl() to security/commoncap.c::cap_task_prctl().

For the aforementioned configs cap_task_prctl() is not called
(moreover, security/commoncap.c is not compiled).

SELinux's implementation of .task_prctl callback resorts to
secondary_ops->task_prctl() which is dummy_task_prctl() (in the
absence of CONFIG_SECURITY_CAPABILITIES (or any other) as a secondary
module).

So the relevant code should be either moved back to sys_prctl() or
placed in some generic function (not in security/commoncap.c) which is
accessible for all configs.

Move it back to sys_prctl().
    
Signed-off-by: Dmitry Adamushko <dmitry.adamushko@gmail.com>

----

diff --git a/kernel/sys.c b/kernel/sys.c
index 14e9728..5b8e583 100644
--- a/kernel/sys.c
+++ b/kernel/sys.c
@@ -24,6 +24,7 @@
 #include <linux/times.h>
 #include <linux/posix-timers.h>
 #include <linux/security.h>
+#include <linux/securebits.h>
 #include <linux/dcookies.h>
 #include <linux/suspend.h>
 #include <linux/tty.h>
@@ -1658,6 +1659,21 @@ asmlinkage long sys_prctl(int option, unsigned long arg2, unsigned long arg3,
 		return error;
 
 	switch (option) {
+		case PR_GET_KEEPCAPS:
+			if (issecure(SECURE_KEEP_CAPS))
+				error = 1;
+			break;
+		case PR_SET_KEEPCAPS:
+			if (arg2 > 1) /* Note, we rely on arg2 being unsigned here */
+				error = -EINVAL;
+			else if (issecure(SECURE_KEEP_CAPS_LOCKED))
+				error = -EPERM;
+			else if (arg2)
+				current->securebits |= issecure_mask(SECURE_KEEP_CAPS);
+			else
+				current->securebits &=
+					~issecure_mask(SECURE_KEEP_CAPS);
+			break;
 		case PR_SET_PDEATHSIG:
 			if (!valid_signal(arg2)) {
 				error = -EINVAL;
@@ -1744,6 +1760,12 @@ asmlinkage long sys_prctl(int option, unsigned long arg2, unsigned long arg3,
 		case PR_SET_TSC:
 			error = SET_TSC_CTL(arg2);
 			break;
+		case PR_CAPBSET_READ:
+			if (!cap_valid(arg2))
+				error = -EINVAL;
+			else
+				error = !!cap_raised(current->cap_bset, arg2);
+			break;
 		default:
 			error = -EINVAL;
 			break;
diff --git a/security/commoncap.c b/security/commoncap.c
index 5edabc7..76f3a76 100644
--- a/security/commoncap.c
+++ b/security/commoncap.c
@@ -576,12 +576,6 @@ int cap_task_prctl(int option, unsigned long arg2, unsigned long arg3,
 	long error = 0;
 
 	switch (option) {
-	case PR_CAPBSET_READ:
-		if (!cap_valid(arg2))
-			error = -EINVAL;
-		else
-			error = !!cap_raised(current->cap_bset, arg2);
-		break;
 #ifdef CONFIG_SECURITY_FILE_CAPABILITIES
 	case PR_CAPBSET_DROP:
 		error = cap_prctl_drop(arg2);
@@ -631,22 +625,6 @@ int cap_task_prctl(int option, unsigned long arg2, unsigned long arg3,
 
 #endif /* def CONFIG_SECURITY_FILE_CAPABILITIES */
 
-	case PR_GET_KEEPCAPS:
-		if (issecure(SECURE_KEEP_CAPS))
-			error = 1;
-		break;
-	case PR_SET_KEEPCAPS:
-		if (arg2 > 1) /* Note, we rely on arg2 being unsigned here */
-			error = -EINVAL;
-		else if (issecure(SECURE_KEEP_CAPS_LOCKED))
-			error = -EPERM;
-		else if (arg2)
-			current->securebits |= issecure_mask(SECURE_KEEP_CAPS);
-		else
-			current->securebits &=
-				~issecure_mask(SECURE_KEEP_CAPS);
-		break;
-
 	default:
 		/* No functionality available - continue with default */
 		return 0;

---