[RFC PATCH 2/2] capability: use new capable_or functionality

[RFC PATCH 2/2] capability: use new capable_or functionality

[Christian Göttsche]

Use the new added capable_or macro in appropriate cases, where a task
is required to have any of two capabilities.

Reorder CAP_SYS_ADMIN last.

TODO: split into subsystem patches.

Fixes: 94c4b4fd25e6 ("block: Check ADMIN before NICE for IOPRIO_CLASS_RT")

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
---
 block/ioprio.c                                   | 9 +--------
 drivers/media/common/saa7146/saa7146_video.c     | 2 +-
 drivers/media/pci/bt8xx/bttv-driver.c            | 3 +--
 drivers/media/pci/saa7134/saa7134-video.c        | 3 +--
 drivers/media/platform/fsl-viu.c                 | 2 +-
 drivers/media/test-drivers/vivid/vivid-vid-cap.c | 2 +-
 drivers/net/caif/caif_serial.c                   | 2 +-
 drivers/s390/block/dasd_eckd.c                   | 2 +-
 fs/pipe.c                                        | 2 +-
 include/linux/capability.h                       | 4 ++--
 kernel/bpf/syscall.c                             | 2 +-
 kernel/fork.c                                    | 2 +-
 kernel/sys.c                                     | 2 +-
 net/caif/caif_socket.c                           | 2 +-
 net/unix/scm.c                                   | 2 +-
 15 files changed, 16 insertions(+), 25 deletions(-)

diff --git a/block/ioprio.c b/block/ioprio.c
index 2fe068fcaad5..52d5da286323 100644
--- a/block/ioprio.c
+++ b/block/ioprio.c
@@ -37,14 +37,7 @@ int ioprio_check_cap(int ioprio)
 
 	switch (class) {
 		case IOPRIO_CLASS_RT:
-			/*
-			 * Originally this only checked for CAP_SYS_ADMIN,
-			 * which was implicitly allowed for pid 0 by security
-			 * modules such as SELinux. Make sure we check
-			 * CAP_SYS_ADMIN first to avoid a denial/avc for
-			 * possibly missing CAP_SYS_NICE permission.
-			 */
-			if (!capable(CAP_SYS_ADMIN) && !capable(CAP_SYS_NICE))
+			if (!capable_or(CAP_SYS_NICE, CAP_SYS_ADMIN))
 				return -EPERM;
 			fallthrough;
 			/* rt has prio field too */
diff --git a/drivers/media/common/saa7146/saa7146_video.c b/drivers/media/common/saa7146/saa7146_video.c
index 66215d9106a4..5eabc2e77cc2 100644
--- a/drivers/media/common/saa7146/saa7146_video.c
+++ b/drivers/media/common/saa7146/saa7146_video.c
@@ -470,7 +470,7 @@ static int vidioc_s_fbuf(struct file *file, void *fh, const struct v4l2_framebuf
 
 	DEB_EE("VIDIOC_S_FBUF\n");
 
-	if (!capable(CAP_SYS_ADMIN) && !capable(CAP_SYS_RAWIO))
+	if (!capable_or(CAP_SYS_RAWIO, CAP_SYS_ADMIN))
 		return -EPERM;
 
 	/* check args */
diff --git a/drivers/media/pci/bt8xx/bttv-driver.c b/drivers/media/pci/bt8xx/bttv-driver.c
index 8cc9bec43688..c2437ff07246 100644
--- a/drivers/media/pci/bt8xx/bttv-driver.c
+++ b/drivers/media/pci/bt8xx/bttv-driver.c
@@ -2569,8 +2569,7 @@ static int bttv_s_fbuf(struct file *file, void *f,
 	const struct bttv_format *fmt;
 	int retval;
 
-	if (!capable(CAP_SYS_ADMIN) &&
-		!capable(CAP_SYS_RAWIO))
+	if (!capable_or(CAP_SYS_RAWIO, CAP_SYS_ADMIN))
 		return -EPERM;
 
 	/* check args */
diff --git a/drivers/media/pci/saa7134/saa7134-video.c b/drivers/media/pci/saa7134/saa7134-video.c
index 374c8e1087de..356b77c16f87 100644
--- a/drivers/media/pci/saa7134/saa7134-video.c
+++ b/drivers/media/pci/saa7134/saa7134-video.c
@@ -1803,8 +1803,7 @@ static int saa7134_s_fbuf(struct file *file, void *f,
 	struct saa7134_dev *dev = video_drvdata(file);
 	struct saa7134_format *fmt;
 
-	if (!capable(CAP_SYS_ADMIN) &&
-	   !capable(CAP_SYS_RAWIO))
+	if (!capable_or(CAP_SYS_RAWIO, CAP_SYS_ADMIN))
 		return -EPERM;
 
 	/* check args */
diff --git a/drivers/media/platform/fsl-viu.c b/drivers/media/platform/fsl-viu.c
index a4bfa70b49b2..925c34c2b1b3 100644
--- a/drivers/media/platform/fsl-viu.c
+++ b/drivers/media/platform/fsl-viu.c
@@ -803,7 +803,7 @@ static int vidioc_s_fbuf(struct file *file, void *priv, const struct v4l2_frameb
 	const struct v4l2_framebuffer *fb = arg;
 	struct viu_fmt *fmt;
 
-	if (!capable(CAP_SYS_ADMIN) && !capable(CAP_SYS_RAWIO))
+	if (!capable_or(CAP_SYS_RAWIO, CAP_SYS_ADMIN))
 		return -EPERM;
 
 	/* check args */
diff --git a/drivers/media/test-drivers/vivid/vivid-vid-cap.c b/drivers/media/test-drivers/vivid/vivid-vid-cap.c
index b9caa4b26209..a0cfcf6c22c4 100644
--- a/drivers/media/test-drivers/vivid/vivid-vid-cap.c
+++ b/drivers/media/test-drivers/vivid/vivid-vid-cap.c
@@ -1253,7 +1253,7 @@ int vivid_vid_cap_s_fbuf(struct file *file, void *fh,
 	if (dev->multiplanar)
 		return -ENOTTY;
 
-	if (!capable(CAP_SYS_ADMIN) && !capable(CAP_SYS_RAWIO))
+	if (!capable_or(CAP_SYS_RAWIO, CAP_SYS_ADMIN))
 		return -EPERM;
 
 	if (dev->overlay_cap_owner)
diff --git a/drivers/net/caif/caif_serial.c b/drivers/net/caif/caif_serial.c
index 2a7af611d43a..245c30c469c2 100644
--- a/drivers/net/caif/caif_serial.c
+++ b/drivers/net/caif/caif_serial.c
@@ -326,7 +326,7 @@ static int ldisc_open(struct tty_struct *tty)
 	/* No write no play */
 	if (tty->ops->write == NULL)
 		return -EOPNOTSUPP;
-	if (!capable(CAP_SYS_ADMIN) && !capable(CAP_SYS_TTY_CONFIG))
+	if (!capable_or(CAP_SYS_TTY_CONFIG, CAP_SYS_ADMIN))
 		return -EPERM;
 
 	/* release devices to avoid name collision */
diff --git a/drivers/s390/block/dasd_eckd.c b/drivers/s390/block/dasd_eckd.c
index 8410a25a65c1..9b5d22dd3e7b 100644
--- a/drivers/s390/block/dasd_eckd.c
+++ b/drivers/s390/block/dasd_eckd.c
@@ -5319,7 +5319,7 @@ static int dasd_symm_io(struct dasd_device *device, void __user *argp)
 	char psf0, psf1;
 	int rc;
 
-	if (!capable(CAP_SYS_ADMIN) && !capable(CAP_SYS_RAWIO))
+	if (!capable_or(CAP_SYS_RAWIO, CAP_SYS_ADMIN))
 		return -EACCES;
 	psf0 = psf1 = 0;
 
diff --git a/fs/pipe.c b/fs/pipe.c
index cc28623a67b6..47dc9b59b7a5 100644
--- a/fs/pipe.c
+++ b/fs/pipe.c
@@ -775,7 +775,7 @@ bool too_many_pipe_buffers_hard(unsigned long user_bufs)
 
 bool pipe_is_unprivileged_user(void)
 {
-	return !capable(CAP_SYS_RESOURCE) && !capable(CAP_SYS_ADMIN);
+	return !capable_or(CAP_SYS_RESOURCE, CAP_SYS_ADMIN);
 }
 
 struct pipe_inode_info *alloc_pipe_info(void)
diff --git a/include/linux/capability.h b/include/linux/capability.h
index 5c55687a9a05..5ed55b73cb62 100644
--- a/include/linux/capability.h
+++ b/include/linux/capability.h
@@ -262,12 +262,12 @@ extern bool file_ns_capable(const struct file *file, struct user_namespace *ns,
 extern bool ptracer_capable(struct task_struct *tsk, struct user_namespace *ns);
 static inline bool perfmon_capable(void)
 {
-	return capable(CAP_PERFMON) || capable(CAP_SYS_ADMIN);
+	return capable_or(CAP_PERFMON, CAP_SYS_ADMIN);
 }
 
 static inline bool bpf_capable(void)
 {
-	return capable(CAP_BPF) || capable(CAP_SYS_ADMIN);
+	return capable_or(CAP_BPF, CAP_SYS_ADMIN);
 }
 
 static inline bool checkpoint_restore_ns_capable(struct user_namespace *ns)
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index fa4505f9b611..108dd09f978a 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -2243,7 +2243,7 @@ static int bpf_prog_load(union bpf_attr *attr, bpfptr_t uattr)
 	    !bpf_capable())
 		return -EPERM;
 
-	if (is_net_admin_prog_type(type) && !capable(CAP_NET_ADMIN) && !capable(CAP_SYS_ADMIN))
+	if (is_net_admin_prog_type(type) && !capable_or(CAP_NET_ADMIN, CAP_SYS_ADMIN))
 		return -EPERM;
 	if (is_perfmon_prog_type(type) && !perfmon_capable())
 		return -EPERM;
diff --git a/kernel/fork.c b/kernel/fork.c
index d75a528f7b21..067702f2eb15 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -2024,7 +2024,7 @@ static __latent_entropy struct task_struct *copy_process(
 	retval = -EAGAIN;
 	if (is_ucounts_overlimit(task_ucounts(p), UCOUNT_RLIMIT_NPROC, rlimit(RLIMIT_NPROC))) {
 		if (p->real_cred->user != INIT_USER &&
-		    !capable(CAP_SYS_RESOURCE) && !capable(CAP_SYS_ADMIN))
+		    !capable_or(CAP_SYS_RESOURCE, CAP_SYS_ADMIN))
 			goto bad_fork_free;
 	}
 	current->flags &= ~PF_NPROC_EXCEEDED;
diff --git a/kernel/sys.c b/kernel/sys.c
index ecc4cf019242..9df6c5e77620 100644
--- a/kernel/sys.c
+++ b/kernel/sys.c
@@ -481,7 +481,7 @@ static int set_user(struct cred *new)
 	 */
 	if (is_ucounts_overlimit(new->ucounts, UCOUNT_RLIMIT_NPROC, rlimit(RLIMIT_NPROC)) &&
 			new_user != INIT_USER &&
-			!capable(CAP_SYS_RESOURCE) && !capable(CAP_SYS_ADMIN))
+			!capable_or(CAP_SYS_RESOURCE, CAP_SYS_ADMIN))
 		current->flags |= PF_NPROC_EXCEEDED;
 	else
 		current->flags &= ~PF_NPROC_EXCEEDED;
diff --git a/net/caif/caif_socket.c b/net/caif/caif_socket.c
index 2b8892d502f7..60498148126c 100644
--- a/net/caif/caif_socket.c
+++ b/net/caif/caif_socket.c
@@ -1036,7 +1036,7 @@ static int caif_create(struct net *net, struct socket *sock, int protocol,
 		.usersize = sizeof_field(struct caifsock, conn_req.param)
 	};
 
-	if (!capable(CAP_SYS_ADMIN) && !capable(CAP_NET_ADMIN))
+	if (!capable_or(CAP_NET_ADMIN, CAP_SYS_ADMIN))
 		return -EPERM;
 	/*
 	 * The sock->type specifies the socket type to use.
diff --git a/net/unix/scm.c b/net/unix/scm.c
index aa27a02478dc..821be80e6c85 100644
--- a/net/unix/scm.c
+++ b/net/unix/scm.c
@@ -99,7 +99,7 @@ static inline bool too_many_unix_fds(struct task_struct *p)
 	struct user_struct *user = current_user();
 
 	if (unlikely(user->unix_inflight > task_rlimit(p, RLIMIT_NOFILE)))
-		return !capable(CAP_SYS_RESOURCE) && !capable(CAP_SYS_ADMIN);
+		return !capable_or(CAP_SYS_RESOURCE, CAP_SYS_ADMIN);
 	return false;
 }
 
-- 
2.35.1

[RFC PATCH 1/2] capability: add capable_or to test for multiple caps with exactly one audit message

[Christian Göttsche]

Add the interface `capable_or()` as an alternative to or multiple
`capable()` calls, like `capable_or(CAP_SYS_NICE, CAP_SYS_ADMIN)`
instead of `capable(CAP_SYS_NICE) || capable(CAP_SYS_ADMIN)`.
`capable_or()` will in particular generate exactly one audit message,
either for the left most capability in effect or, if the task has none,
the first one.
This is especially helpful with regard to SELinux, where each audit
message about a not allowed capability will create an avc denial.
Using this function with the least invasive capability as left most
argument (e.g. CAP_SYS_NICE before CAP_SYS_ADMIN) enables policy writers
to only allow the least invasive one and SELinux domains pass this check
with only capability:sys_nice or capability:sys_admin allowed without
any avc denial message.

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
---
 include/linux/capability.h |  6 +++++
 kernel/capability.c        | 48 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 54 insertions(+)

diff --git a/include/linux/capability.h b/include/linux/capability.h
index 65efb74c3585..5c55687a9a05 100644
--- a/include/linux/capability.h
+++ b/include/linux/capability.h
@@ -207,6 +207,8 @@ extern bool has_ns_capability(struct task_struct *t,
 extern bool has_capability_noaudit(struct task_struct *t, int cap);
 extern bool has_ns_capability_noaudit(struct task_struct *t,
 				      struct user_namespace *ns, int cap);
+#define capable_or(...) _capable_or_impl((sizeof((int[]){__VA_ARGS__}) / sizeof(int)), __VA_ARGS__)
+extern bool _capable_or_impl(int count, ...);
 extern bool capable(int cap);
 extern bool ns_capable(struct user_namespace *ns, int cap);
 extern bool ns_capable_noaudit(struct user_namespace *ns, int cap);
@@ -230,6 +232,10 @@ static inline bool has_ns_capability_noaudit(struct task_struct *t,
 {
 	return true;
 }
+static inline bool capable_or(int first_cap, ...)
+{
+	return true;
+}
 static inline bool capable(int cap)
 {
 	return true;
diff --git a/kernel/capability.c b/kernel/capability.c
index 46a361dde042..5b73a58b914e 100644
--- a/kernel/capability.c
+++ b/kernel/capability.c
@@ -434,6 +434,54 @@ bool ns_capable_setid(struct user_namespace *ns, int cap)
 }
 EXPORT_SYMBOL(ns_capable_setid);
 
+/**
+ * _capable_or - Determine if the current task has one of multiple superior capabilities in effect
+ * @cap: The capabilities to be tested for
+ *
+ * Return true if the current task has at least one of the given superior capabilities currently
+ * available for use, false if not.
+ *
+ * In contrast to or'ing capable() this call will create exactly one audit message, either for the
+ * left most capability in effect or (if the task has none of the tested capabilities) the first
+ * capabilit in the test list.
+ *
+ * The capabilities should be ordered from least to most invasive, i.e. CAP_SYS_ADMIN last.
+ *
+ * This sets PF_SUPERPRIV on the task if the capability is available on the
+ * assumption that it's about to be used.
+ */
+bool _capable_or_impl(int count, ...)
+{
+	va_list args;
+	const struct cred *cred = current_cred();
+	int cap, first_cap, i;
+
+	BUG_ON(count < 1);
+	BUG_ON(count > CAP_LAST_CAP);
+
+	va_start(args, count);
+
+	for (i = 0; i < count; i++) {
+		int ret;
+
+		cap = va_arg(args, int);
+		if (i == 0)
+			first_cap = cap;
+
+		ret = security_capable(cred, &init_user_ns, cap, CAP_OPT_NOAUDIT);
+		if (ret == 0)
+			goto out;
+	}
+
+	/* if none of the capabilities was allowed, create an audit event for the first one */
+	cap = first_cap;
+
+out:
+	va_end(args);
+	return ns_capable(&init_user_ns, cap);
+}
+EXPORT_SYMBOL(_capable_or_impl);
+
 /**
  * capable - Determine if the current task has a superior capability in effect
  * @cap: The capability to be tested for
-- 
2.35.1

Re: [RFC PATCH 2/2] capability: use new capable_or functionality

[Alexei Starovoitov]

On Thu, Feb 17, 2022 at 6:50 AM Christian Göttsche
<cgzones@googlemail.com> wrote:
>
> Use the new added capable_or macro in appropriate cases, where a task
> is required to have any of two capabilities.
>
> Reorder CAP_SYS_ADMIN last.
>
> TODO: split into subsystem patches.

Yes. Please.

The bpf side picked the existing order because we were aware
of that selinux issue.
Looks like there is no good order that works for all.
So the new helper makes a lot of sense.

> Fixes: 94c4b4fd25e6 ("block: Check ADMIN before NICE for IOPRIO_CLASS_RT")

[PATCH v2 2/8] capability: use new capable_or functionality

[Christian Göttsche]

Use the new added capable_or function in appropriate cases, where a task
is required to have any of two capabilities.

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
---
 include/linux/capability.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/linux/capability.h b/include/linux/capability.h
index a16d1edea9b3..1f26d6bae4f3 100644
--- a/include/linux/capability.h
+++ b/include/linux/capability.h
@@ -261,12 +261,12 @@ extern bool file_ns_capable(const struct file *file, struct user_namespace *ns,
 extern bool ptracer_capable(struct task_struct *tsk, struct user_namespace *ns);
 static inline bool perfmon_capable(void)
 {
-	return capable(CAP_PERFMON) || capable(CAP_SYS_ADMIN);
+	return capable_or(CAP_PERFMON, CAP_SYS_ADMIN);
 }
 
 static inline bool bpf_capable(void)
 {
-	return capable(CAP_BPF) || capable(CAP_SYS_ADMIN);
+	return capable_or(CAP_BPF, CAP_SYS_ADMIN);
 }
 
 static inline bool checkpoint_restore_ns_capable(struct user_namespace *ns)
-- 
2.36.0

[PATCH v2 3/8] block: use new capable_or functionality

[Christian Göttsche]

Use the new added capable_or function in appropriate cases, where a task
is required to have any of two capabilities.

Reorder CAP_SYS_ADMIN last.

Fixes: 94c4b4fd25e6 ("block: Check ADMIN before NICE for IOPRIO_CLASS_RT")

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
---
 block/ioprio.c | 9 +--------
 1 file changed, 1 insertion(+), 8 deletions(-)

diff --git a/block/ioprio.c b/block/ioprio.c
index 2fe068fcaad5..52d5da286323 100644
--- a/block/ioprio.c
+++ b/block/ioprio.c
@@ -37,14 +37,7 @@ int ioprio_check_cap(int ioprio)
 
 	switch (class) {
 		case IOPRIO_CLASS_RT:
-			/*
-			 * Originally this only checked for CAP_SYS_ADMIN,
-			 * which was implicitly allowed for pid 0 by security
-			 * modules such as SELinux. Make sure we check
-			 * CAP_SYS_ADMIN first to avoid a denial/avc for
-			 * possibly missing CAP_SYS_NICE permission.
-			 */
-			if (!capable(CAP_SYS_ADMIN) && !capable(CAP_SYS_NICE))
+			if (!capable_or(CAP_SYS_NICE, CAP_SYS_ADMIN))
 				return -EPERM;
 			fallthrough;
 			/* rt has prio field too */
-- 
2.36.0

[PATCH v2 4/8] drivers: use new capable_or functionality

[Christian Göttsche]

Use the new added capable_or function in appropriate cases, where a task
is required to have any of two capabilities.

Reorder CAP_SYS_ADMIN last.

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
---
 drivers/media/common/saa7146/saa7146_video.c     | 2 +-
 drivers/media/pci/bt8xx/bttv-driver.c            | 3 +--
 drivers/media/pci/saa7134/saa7134-video.c        | 3 +--
 drivers/media/platform/nxp/fsl-viu.c             | 2 +-
 drivers/media/test-drivers/vivid/vivid-vid-cap.c | 2 +-
 drivers/net/caif/caif_serial.c                   | 2 +-
 drivers/s390/block/dasd_eckd.c                   | 2 +-
 7 files changed, 7 insertions(+), 9 deletions(-)

diff --git a/drivers/media/common/saa7146/saa7146_video.c b/drivers/media/common/saa7146/saa7146_video.c
index 66215d9106a4..5eabc2e77cc2 100644
--- a/drivers/media/common/saa7146/saa7146_video.c
+++ b/drivers/media/common/saa7146/saa7146_video.c
@@ -470,7 +470,7 @@ static int vidioc_s_fbuf(struct file *file, void *fh, const struct v4l2_framebuf
 
 	DEB_EE("VIDIOC_S_FBUF\n");
 
-	if (!capable(CAP_SYS_ADMIN) && !capable(CAP_SYS_RAWIO))
+	if (!capable_or(CAP_SYS_RAWIO, CAP_SYS_ADMIN))
 		return -EPERM;
 
 	/* check args */
diff --git a/drivers/media/pci/bt8xx/bttv-driver.c b/drivers/media/pci/bt8xx/bttv-driver.c
index 5ca3d0cc653a..4143f380d44d 100644
--- a/drivers/media/pci/bt8xx/bttv-driver.c
+++ b/drivers/media/pci/bt8xx/bttv-driver.c
@@ -2569,8 +2569,7 @@ static int bttv_s_fbuf(struct file *file, void *f,
 	const struct bttv_format *fmt;
 	int retval;
 
-	if (!capable(CAP_SYS_ADMIN) &&
-		!capable(CAP_SYS_RAWIO))
+	if (!capable_or(CAP_SYS_RAWIO, CAP_SYS_ADMIN))
 		return -EPERM;
 
 	/* check args */
diff --git a/drivers/media/pci/saa7134/saa7134-video.c b/drivers/media/pci/saa7134/saa7134-video.c
index 48543ad3d595..684208ebfdbd 100644
--- a/drivers/media/pci/saa7134/saa7134-video.c
+++ b/drivers/media/pci/saa7134/saa7134-video.c
@@ -1798,8 +1798,7 @@ static int saa7134_s_fbuf(struct file *file, void *f,
 	struct saa7134_dev *dev = video_drvdata(file);
 	struct saa7134_format *fmt;
 
-	if (!capable(CAP_SYS_ADMIN) &&
-	   !capable(CAP_SYS_RAWIO))
+	if (!capable_or(CAP_SYS_RAWIO, CAP_SYS_ADMIN))
 		return -EPERM;
 
 	/* check args */
diff --git a/drivers/media/platform/nxp/fsl-viu.c b/drivers/media/platform/nxp/fsl-viu.c
index afc96f6db2a1..c5ed4c4a1587 100644
--- a/drivers/media/platform/nxp/fsl-viu.c
+++ b/drivers/media/platform/nxp/fsl-viu.c
@@ -803,7 +803,7 @@ static int vidioc_s_fbuf(struct file *file, void *priv, const struct v4l2_frameb
 	const struct v4l2_framebuffer *fb = arg;
 	struct viu_fmt *fmt;
 
-	if (!capable(CAP_SYS_ADMIN) && !capable(CAP_SYS_RAWIO))
+	if (!capable_or(CAP_SYS_RAWIO, CAP_SYS_ADMIN))
 		return -EPERM;
 
 	/* check args */
diff --git a/drivers/media/test-drivers/vivid/vivid-vid-cap.c b/drivers/media/test-drivers/vivid/vivid-vid-cap.c
index b9caa4b26209..a0cfcf6c22c4 100644
--- a/drivers/media/test-drivers/vivid/vivid-vid-cap.c
+++ b/drivers/media/test-drivers/vivid/vivid-vid-cap.c
@@ -1253,7 +1253,7 @@ int vivid_vid_cap_s_fbuf(struct file *file, void *fh,
 	if (dev->multiplanar)
 		return -ENOTTY;
 
-	if (!capable(CAP_SYS_ADMIN) && !capable(CAP_SYS_RAWIO))
+	if (!capable_or(CAP_SYS_RAWIO, CAP_SYS_ADMIN))
 		return -EPERM;
 
 	if (dev->overlay_cap_owner)
diff --git a/drivers/net/caif/caif_serial.c b/drivers/net/caif/caif_serial.c
index 688075859ae4..f17b618d8858 100644
--- a/drivers/net/caif/caif_serial.c
+++ b/drivers/net/caif/caif_serial.c
@@ -326,7 +326,7 @@ static int ldisc_open(struct tty_struct *tty)
 	/* No write no play */
 	if (tty->ops->write == NULL)
 		return -EOPNOTSUPP;
-	if (!capable(CAP_SYS_ADMIN) && !capable(CAP_SYS_TTY_CONFIG))
+	if (!capable_or(CAP_SYS_TTY_CONFIG, CAP_SYS_ADMIN))
 		return -EPERM;
 
 	/* release devices to avoid name collision */
diff --git a/drivers/s390/block/dasd_eckd.c b/drivers/s390/block/dasd_eckd.c
index 8410a25a65c1..9b5d22dd3e7b 100644
--- a/drivers/s390/block/dasd_eckd.c
+++ b/drivers/s390/block/dasd_eckd.c
@@ -5319,7 +5319,7 @@ static int dasd_symm_io(struct dasd_device *device, void __user *argp)
 	char psf0, psf1;
 	int rc;
 
-	if (!capable(CAP_SYS_ADMIN) && !capable(CAP_SYS_RAWIO))
+	if (!capable_or(CAP_SYS_RAWIO, CAP_SYS_ADMIN))
 		return -EACCES;
 	psf0 = psf1 = 0;
 
-- 
2.36.0

[PATCH v2 5/8] fs: use new capable_or functionality

[Christian Göttsche]

Use the new added capable_or function in appropriate cases, where a task
is required to have any of two capabilities.

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
---
 fs/pipe.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/pipe.c b/fs/pipe.c
index 9648ac15164a..d91a2bdc837d 100644
--- a/fs/pipe.c
+++ b/fs/pipe.c
@@ -776,7 +776,7 @@ bool too_many_pipe_buffers_hard(unsigned long user_bufs)
 
 bool pipe_is_unprivileged_user(void)
 {
-	return !capable(CAP_SYS_RESOURCE) && !capable(CAP_SYS_ADMIN);
+	return !capable_or(CAP_SYS_RESOURCE, CAP_SYS_ADMIN);
 }
 
 struct pipe_inode_info *alloc_pipe_info(void)
-- 
2.36.0

[PATCH v2 6/8] kernel: use new capable_or functionality

[Christian Göttsche]

Use the new added capable_or function in appropriate cases, where a task
is required to have any of two capabilities.

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
---
 kernel/fork.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/fork.c b/kernel/fork.c
index 9796897560ab..3ae87b864380 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -2098,7 +2098,7 @@ static __latent_entropy struct task_struct *copy_process(
 	retval = -EAGAIN;
 	if (is_ucounts_overlimit(task_ucounts(p), UCOUNT_RLIMIT_NPROC, rlimit(RLIMIT_NPROC))) {
 		if (p->real_cred->user != INIT_USER &&
-		    !capable(CAP_SYS_RESOURCE) && !capable(CAP_SYS_ADMIN))
+		    !capable_or(CAP_SYS_RESOURCE, CAP_SYS_ADMIN))
 			goto bad_fork_cleanup_count;
 	}
 	current->flags &= ~PF_NPROC_EXCEEDED;
-- 
2.36.0

[PATCH v2 7/8] kernel/bpf: use new capable_or functionality

[Christian Göttsche]

Use the new added capable_or function in appropriate cases, where a task
is required to have any of two capabilities.

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
---
 kernel/bpf/syscall.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index cdaa1152436a..95a2cf3e78c9 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -2249,7 +2249,7 @@ static int bpf_prog_load(union bpf_attr *attr, bpfptr_t uattr)
 	    !bpf_capable())
 		return -EPERM;
 
-	if (is_net_admin_prog_type(type) && !capable(CAP_NET_ADMIN) && !capable(CAP_SYS_ADMIN))
+	if (is_net_admin_prog_type(type) && !capable_or(CAP_NET_ADMIN, CAP_SYS_ADMIN))
 		return -EPERM;
 	if (is_perfmon_prog_type(type) && !perfmon_capable())
 		return -EPERM;
-- 
2.36.0

[PATCH v2 8/8] net: use new capable_or functionality

[Christian Göttsche]

Use the new added capable_or function in appropriate cases, where a task
is required to have any of two capabilities.

Reorder CAP_SYS_ADMIN last.

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
---
 net/caif/caif_socket.c | 2 +-
 net/unix/scm.c         | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/caif/caif_socket.c b/net/caif/caif_socket.c
index 2b8892d502f7..60498148126c 100644
--- a/net/caif/caif_socket.c
+++ b/net/caif/caif_socket.c
@@ -1036,7 +1036,7 @@ static int caif_create(struct net *net, struct socket *sock, int protocol,
 		.usersize = sizeof_field(struct caifsock, conn_req.param)
 	};
 
-	if (!capable(CAP_SYS_ADMIN) && !capable(CAP_NET_ADMIN))
+	if (!capable_or(CAP_NET_ADMIN, CAP_SYS_ADMIN))
 		return -EPERM;
 	/*
 	 * The sock->type specifies the socket type to use.
diff --git a/net/unix/scm.c b/net/unix/scm.c
index aa27a02478dc..821be80e6c85 100644
--- a/net/unix/scm.c
+++ b/net/unix/scm.c
@@ -99,7 +99,7 @@ static inline bool too_many_unix_fds(struct task_struct *p)
 	struct user_struct *user = current_user();
 
 	if (unlikely(user->unix_inflight > task_rlimit(p, RLIMIT_NOFILE)))
-		return !capable(CAP_SYS_RESOURCE) && !capable(CAP_SYS_ADMIN);
+		return !capable_or(CAP_SYS_RESOURCE, CAP_SYS_ADMIN);
 	return false;
 }
 
-- 
2.36.0

[PATCH v2 1/8] capability: add capable_or to test for multiple caps with exactly one audit message

[Christian Göttsche]

Add the interface `capable_or()` as an alternative to or multiple
`capable()` calls, like `capable_or(CAP_SYS_NICE, CAP_SYS_ADMIN)`
instead of `capable(CAP_SYS_NICE) || capable(CAP_SYS_ADMIN)`.
`capable_or()` will in particular generate exactly one audit message,
either for the left most capability in effect or, if the task has none,
the first one.
This is especially helpful with regard to SELinux, where each audit
message about a not allowed capability will create an avc denial.
Using this function with the least invasive capability as left most
argument (e.g. CAP_SYS_NICE before CAP_SYS_ADMIN) enables policy writers
to only allow the least invasive one and SELinux domains pass this check
with only capability:sys_nice or capability:sys_admin allowed without
any avc denial message.

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>

---
v2:
   avoid varargs and fix to two capabilities; capable_or3() can be added
   later if needed
---
 include/linux/capability.h |  5 +++++
 kernel/capability.c        | 29 +++++++++++++++++++++++++++++
 2 files changed, 34 insertions(+)

diff --git a/include/linux/capability.h b/include/linux/capability.h
index 65efb74c3585..a16d1edea9b3 100644
--- a/include/linux/capability.h
+++ b/include/linux/capability.h
@@ -207,6 +207,7 @@ extern bool has_ns_capability(struct task_struct *t,
 extern bool has_capability_noaudit(struct task_struct *t, int cap);
 extern bool has_ns_capability_noaudit(struct task_struct *t,
 				      struct user_namespace *ns, int cap);
+extern bool capable_or(int cap1, int cap2);
 extern bool capable(int cap);
 extern bool ns_capable(struct user_namespace *ns, int cap);
 extern bool ns_capable_noaudit(struct user_namespace *ns, int cap);
@@ -230,6 +231,10 @@ static inline bool has_ns_capability_noaudit(struct task_struct *t,
 {
 	return true;
 }
+static inline bool capable_or(int cap1, int cap2)
+{
+	return true;
+}
 static inline bool capable(int cap)
 {
 	return true;
diff --git a/kernel/capability.c b/kernel/capability.c
index 765194f5d678..cd8f3efe6d08 100644
--- a/kernel/capability.c
+++ b/kernel/capability.c
@@ -435,6 +435,35 @@ bool ns_capable_setid(struct user_namespace *ns, int cap)
 }
 EXPORT_SYMBOL(ns_capable_setid);
 
+/**
+ * capable_or - Determine if the current task has one of two superior capabilities in effect
+ * @cap1: The capabilities to be tested for first
+ * @cap2: The capabilities to be tested for secondly
+ *
+ * Return true if the current task has at one of the two given superior
+ * capabilities currently available for use, false if not.
+ *
+ * In contrast to or'ing capable() this call will create exactly one audit
+ * message, either for @cap1, if it is granted or both are not permitted,
+ * or @cap2, if it is granted while the other one is not.
+ *
+ * The capabilities should be ordered from least to most invasive, i.e. CAP_SYS_ADMIN last.
+ *
+ * This sets PF_SUPERPRIV on the task if the capability is available on the
+ * assumption that it's about to be used.
+ */
+bool capable_or(int cap1, int cap2)
+{
+	if (ns_capable_noaudit(&init_user_ns, cap1))
+		return ns_capable(&init_user_ns, cap1);
+
+	if (ns_capable_noaudit(&init_user_ns, cap2))
+		return ns_capable(&init_user_ns, cap2);
+
+	return ns_capable(&init_user_ns, cap1);
+}
+EXPORT_SYMBOL(capable_or);
+
 /**
  * capable - Determine if the current task has a superior capability in effect
  * @cap: The capability to be tested for
-- 
2.36.0

Re: [PATCH v2 4/8] drivers: use new capable_or functionality

[Jiri Slaby]

On 02. 05. 22, 18:00, Christian Göttsche wrote:
> Use the new added capable_or function in appropriate cases, where a task
> is required to have any of two capabilities.
> 
> Reorder CAP_SYS_ADMIN last.
> 
> Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
> ---

>   drivers/net/caif/caif_serial.c                   | 2 +-

For the above:
Reviewed-by: Jiri Slaby <jirislaby@kernel.org>


>   7 files changed, 7 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/media/common/saa7146/saa7146_video.c b/drivers/media/common/saa7146/saa7146_video.c
> index 66215d9106a4..5eabc2e77cc2 100644
> --- a/drivers/media/common/saa7146/saa7146_video.c
> +++ b/drivers/media/common/saa7146/saa7146_video.c
> @@ -470,7 +470,7 @@ static int vidioc_s_fbuf(struct file *file, void *fh, const struct v4l2_framebuf
>   
>   	DEB_EE("VIDIOC_S_FBUF\n");
>   
> -	if (!capable(CAP_SYS_ADMIN) && !capable(CAP_SYS_RAWIO))
> +	if (!capable_or(CAP_SYS_RAWIO, CAP_SYS_ADMIN))
>   		return -EPERM;
>   
>   	/* check args */
> diff --git a/drivers/media/pci/bt8xx/bttv-driver.c b/drivers/media/pci/bt8xx/bttv-driver.c
> index 5ca3d0cc653a..4143f380d44d 100644
> --- a/drivers/media/pci/bt8xx/bttv-driver.c
> +++ b/drivers/media/pci/bt8xx/bttv-driver.c
> @@ -2569,8 +2569,7 @@ static int bttv_s_fbuf(struct file *file, void *f,
>   	const struct bttv_format *fmt;
>   	int retval;
>   
> -	if (!capable(CAP_SYS_ADMIN) &&
> -		!capable(CAP_SYS_RAWIO))
> +	if (!capable_or(CAP_SYS_RAWIO, CAP_SYS_ADMIN))
>   		return -EPERM;
>   
>   	/* check args */
> diff --git a/drivers/media/pci/saa7134/saa7134-video.c b/drivers/media/pci/saa7134/saa7134-video.c
> index 48543ad3d595..684208ebfdbd 100644
> --- a/drivers/media/pci/saa7134/saa7134-video.c
> +++ b/drivers/media/pci/saa7134/saa7134-video.c
> @@ -1798,8 +1798,7 @@ static int saa7134_s_fbuf(struct file *file, void *f,
>   	struct saa7134_dev *dev = video_drvdata(file);
>   	struct saa7134_format *fmt;
>   
> -	if (!capable(CAP_SYS_ADMIN) &&
> -	   !capable(CAP_SYS_RAWIO))
> +	if (!capable_or(CAP_SYS_RAWIO, CAP_SYS_ADMIN))
>   		return -EPERM;
>   
>   	/* check args */
> diff --git a/drivers/media/platform/nxp/fsl-viu.c b/drivers/media/platform/nxp/fsl-viu.c
> index afc96f6db2a1..c5ed4c4a1587 100644
> --- a/drivers/media/platform/nxp/fsl-viu.c
> +++ b/drivers/media/platform/nxp/fsl-viu.c
> @@ -803,7 +803,7 @@ static int vidioc_s_fbuf(struct file *file, void *priv, const struct v4l2_frameb
>   	const struct v4l2_framebuffer *fb = arg;
>   	struct viu_fmt *fmt;
>   
> -	if (!capable(CAP_SYS_ADMIN) && !capable(CAP_SYS_RAWIO))
> +	if (!capable_or(CAP_SYS_RAWIO, CAP_SYS_ADMIN))
>   		return -EPERM;
>   
>   	/* check args */
> diff --git a/drivers/media/test-drivers/vivid/vivid-vid-cap.c b/drivers/media/test-drivers/vivid/vivid-vid-cap.c
> index b9caa4b26209..a0cfcf6c22c4 100644
> --- a/drivers/media/test-drivers/vivid/vivid-vid-cap.c
> +++ b/drivers/media/test-drivers/vivid/vivid-vid-cap.c
> @@ -1253,7 +1253,7 @@ int vivid_vid_cap_s_fbuf(struct file *file, void *fh,
>   	if (dev->multiplanar)
>   		return -ENOTTY;
>   
> -	if (!capable(CAP_SYS_ADMIN) && !capable(CAP_SYS_RAWIO))
> +	if (!capable_or(CAP_SYS_RAWIO, CAP_SYS_ADMIN))
>   		return -EPERM;
>   
>   	if (dev->overlay_cap_owner)
> diff --git a/drivers/net/caif/caif_serial.c b/drivers/net/caif/caif_serial.c
> index 688075859ae4..f17b618d8858 100644
> --- a/drivers/net/caif/caif_serial.c
> +++ b/drivers/net/caif/caif_serial.c
> @@ -326,7 +326,7 @@ static int ldisc_open(struct tty_struct *tty)
>   	/* No write no play */
>   	if (tty->ops->write == NULL)
>   		return -EOPNOTSUPP;
> -	if (!capable(CAP_SYS_ADMIN) && !capable(CAP_SYS_TTY_CONFIG))
> +	if (!capable_or(CAP_SYS_TTY_CONFIG, CAP_SYS_ADMIN))
>   		return -EPERM;
>   
>   	/* release devices to avoid name collision */
> diff --git a/drivers/s390/block/dasd_eckd.c b/drivers/s390/block/dasd_eckd.c
> index 8410a25a65c1..9b5d22dd3e7b 100644
> --- a/drivers/s390/block/dasd_eckd.c
> +++ b/drivers/s390/block/dasd_eckd.c
> @@ -5319,7 +5319,7 @@ static int dasd_symm_io(struct dasd_device *device, void __user *argp)
>   	char psf0, psf1;
>   	int rc;
>   
> -	if (!capable(CAP_SYS_ADMIN) && !capable(CAP_SYS_RAWIO))
> +	if (!capable_or(CAP_SYS_RAWIO, CAP_SYS_ADMIN))
>   		return -EACCES;
>   	psf0 = psf1 = 0;
>   


-- 
js
suse labs

Re: [PATCH v2 4/8] drivers: use new capable_or functionality

[Hans Verkuil]

On 5/2/22 18:00, Christian Göttsche wrote:
> Use the new added capable_or function in appropriate cases, where a task
> is required to have any of two capabilities.
> 
> Reorder CAP_SYS_ADMIN last.
> 
> Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
> ---
>  drivers/media/common/saa7146/saa7146_video.c     | 2 +-
>  drivers/media/pci/bt8xx/bttv-driver.c            | 3 +--
>  drivers/media/pci/saa7134/saa7134-video.c        | 3 +--
>  drivers/media/platform/nxp/fsl-viu.c             | 2 +-
>  drivers/media/test-drivers/vivid/vivid-vid-cap.c | 2 +-

For the media drivers:

Acked-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>

Regards,

	Hans

>  drivers/net/caif/caif_serial.c                   | 2 +-
>  drivers/s390/block/dasd_eckd.c                   | 2 +-
>  7 files changed, 7 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/media/common/saa7146/saa7146_video.c b/drivers/media/common/saa7146/saa7146_video.c
> index 66215d9106a4..5eabc2e77cc2 100644
> --- a/drivers/media/common/saa7146/saa7146_video.c
> +++ b/drivers/media/common/saa7146/saa7146_video.c
> @@ -470,7 +470,7 @@ static int vidioc_s_fbuf(struct file *file, void *fh, const struct v4l2_framebuf
>  
>  	DEB_EE("VIDIOC_S_FBUF\n");
>  
> -	if (!capable(CAP_SYS_ADMIN) && !capable(CAP_SYS_RAWIO))
> +	if (!capable_or(CAP_SYS_RAWIO, CAP_SYS_ADMIN))
>  		return -EPERM;
>  
>  	/* check args */
> diff --git a/drivers/media/pci/bt8xx/bttv-driver.c b/drivers/media/pci/bt8xx/bttv-driver.c
> index 5ca3d0cc653a..4143f380d44d 100644
> --- a/drivers/media/pci/bt8xx/bttv-driver.c
> +++ b/drivers/media/pci/bt8xx/bttv-driver.c
> @@ -2569,8 +2569,7 @@ static int bttv_s_fbuf(struct file *file, void *f,
>  	const struct bttv_format *fmt;
>  	int retval;
>  
> -	if (!capable(CAP_SYS_ADMIN) &&
> -		!capable(CAP_SYS_RAWIO))
> +	if (!capable_or(CAP_SYS_RAWIO, CAP_SYS_ADMIN))
>  		return -EPERM;
>  
>  	/* check args */
> diff --git a/drivers/media/pci/saa7134/saa7134-video.c b/drivers/media/pci/saa7134/saa7134-video.c
> index 48543ad3d595..684208ebfdbd 100644
> --- a/drivers/media/pci/saa7134/saa7134-video.c
> +++ b/drivers/media/pci/saa7134/saa7134-video.c
> @@ -1798,8 +1798,7 @@ static int saa7134_s_fbuf(struct file *file, void *f,
>  	struct saa7134_dev *dev = video_drvdata(file);
>  	struct saa7134_format *fmt;
>  
> -	if (!capable(CAP_SYS_ADMIN) &&
> -	   !capable(CAP_SYS_RAWIO))
> +	if (!capable_or(CAP_SYS_RAWIO, CAP_SYS_ADMIN))
>  		return -EPERM;
>  
>  	/* check args */
> diff --git a/drivers/media/platform/nxp/fsl-viu.c b/drivers/media/platform/nxp/fsl-viu.c
> index afc96f6db2a1..c5ed4c4a1587 100644
> --- a/drivers/media/platform/nxp/fsl-viu.c
> +++ b/drivers/media/platform/nxp/fsl-viu.c
> @@ -803,7 +803,7 @@ static int vidioc_s_fbuf(struct file *file, void *priv, const struct v4l2_frameb
>  	const struct v4l2_framebuffer *fb = arg;
>  	struct viu_fmt *fmt;
>  
> -	if (!capable(CAP_SYS_ADMIN) && !capable(CAP_SYS_RAWIO))
> +	if (!capable_or(CAP_SYS_RAWIO, CAP_SYS_ADMIN))
>  		return -EPERM;
>  
>  	/* check args */
> diff --git a/drivers/media/test-drivers/vivid/vivid-vid-cap.c b/drivers/media/test-drivers/vivid/vivid-vid-cap.c
> index b9caa4b26209..a0cfcf6c22c4 100644
> --- a/drivers/media/test-drivers/vivid/vivid-vid-cap.c
> +++ b/drivers/media/test-drivers/vivid/vivid-vid-cap.c
> @@ -1253,7 +1253,7 @@ int vivid_vid_cap_s_fbuf(struct file *file, void *fh,
>  	if (dev->multiplanar)
>  		return -ENOTTY;
>  
> -	if (!capable(CAP_SYS_ADMIN) && !capable(CAP_SYS_RAWIO))
> +	if (!capable_or(CAP_SYS_RAWIO, CAP_SYS_ADMIN))
>  		return -EPERM;
>  
>  	if (dev->overlay_cap_owner)
> diff --git a/drivers/net/caif/caif_serial.c b/drivers/net/caif/caif_serial.c
> index 688075859ae4..f17b618d8858 100644
> --- a/drivers/net/caif/caif_serial.c
> +++ b/drivers/net/caif/caif_serial.c
> @@ -326,7 +326,7 @@ static int ldisc_open(struct tty_struct *tty)
>  	/* No write no play */
>  	if (tty->ops->write == NULL)
>  		return -EOPNOTSUPP;
> -	if (!capable(CAP_SYS_ADMIN) && !capable(CAP_SYS_TTY_CONFIG))
> +	if (!capable_or(CAP_SYS_TTY_CONFIG, CAP_SYS_ADMIN))
>  		return -EPERM;
>  
>  	/* release devices to avoid name collision */
> diff --git a/drivers/s390/block/dasd_eckd.c b/drivers/s390/block/dasd_eckd.c
> index 8410a25a65c1..9b5d22dd3e7b 100644
> --- a/drivers/s390/block/dasd_eckd.c
> +++ b/drivers/s390/block/dasd_eckd.c
> @@ -5319,7 +5319,7 @@ static int dasd_symm_io(struct dasd_device *device, void __user *argp)
>  	char psf0, psf1;
>  	int rc;
>  
> -	if (!capable(CAP_SYS_ADMIN) && !capable(CAP_SYS_RAWIO))
> +	if (!capable_or(CAP_SYS_RAWIO, CAP_SYS_ADMIN))
>  		return -EACCES;
>  	psf0 = psf1 = 0;
>  

Re: [PATCH v2 1/8] capability: add capable_or to test for multiple caps with exactly one audit message

[Serge E. Hallyn]

On Mon, May 02, 2022 at 06:00:30PM +0200, Christian Göttsche wrote:
> Add the interface `capable_or()` as an alternative to or multiple

How about 'capable_contains_oneof(x, y)' or 'capable_of_one(a, b)'?

> `capable()` calls, like `capable_or(CAP_SYS_NICE, CAP_SYS_ADMIN)`
> instead of `capable(CAP_SYS_NICE) || capable(CAP_SYS_ADMIN)`.
> `capable_or()` will in particular generate exactly one audit message,
> either for the left most capability in effect or, if the task has none,
> the first one.
> This is especially helpful with regard to SELinux, where each audit
> message about a not allowed capability will create an avc denial.
> Using this function with the least invasive capability as left most
> argument (e.g. CAP_SYS_NICE before CAP_SYS_ADMIN) enables policy writers
> to only allow the least invasive one and SELinux domains pass this check
> with only capability:sys_nice or capability:sys_admin allowed without
> any avc denial message.
> 
> Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
> 
> ---
> v2:
>    avoid varargs and fix to two capabilities; capable_or3() can be added
>    later if needed
> ---
>  include/linux/capability.h |  5 +++++
>  kernel/capability.c        | 29 +++++++++++++++++++++++++++++
>  2 files changed, 34 insertions(+)
> 
> diff --git a/include/linux/capability.h b/include/linux/capability.h
> index 65efb74c3585..a16d1edea9b3 100644
> --- a/include/linux/capability.h
> +++ b/include/linux/capability.h
> @@ -207,6 +207,7 @@ extern bool has_ns_capability(struct task_struct *t,
>  extern bool has_capability_noaudit(struct task_struct *t, int cap);
>  extern bool has_ns_capability_noaudit(struct task_struct *t,
>  				      struct user_namespace *ns, int cap);
> +extern bool capable_or(int cap1, int cap2);
>  extern bool capable(int cap);
>  extern bool ns_capable(struct user_namespace *ns, int cap);
>  extern bool ns_capable_noaudit(struct user_namespace *ns, int cap);
> @@ -230,6 +231,10 @@ static inline bool has_ns_capability_noaudit(struct task_struct *t,
>  {
>  	return true;
>  }
> +static inline bool capable_or(int cap1, int cap2)
> +{
> +	return true;
> +}
>  static inline bool capable(int cap)
>  {
>  	return true;
> diff --git a/kernel/capability.c b/kernel/capability.c
> index 765194f5d678..cd8f3efe6d08 100644
> --- a/kernel/capability.c
> +++ b/kernel/capability.c
> @@ -435,6 +435,35 @@ bool ns_capable_setid(struct user_namespace *ns, int cap)
>  }
>  EXPORT_SYMBOL(ns_capable_setid);
>  
> +/**
> + * capable_or - Determine if the current task has one of two superior capabilities in effect
> + * @cap1: The capabilities to be tested for first
> + * @cap2: The capabilities to be tested for secondly
> + *
> + * Return true if the current task has at one of the two given superior

s/has at one/has at least one/ ?

> + * capabilities currently available for use, false if not.
> + *
> + * In contrast to or'ing capable() this call will create exactly one audit
> + * message, either for @cap1, if it is granted or both are not permitted,
> + * or @cap2, if it is granted while the other one is not.
> + *
> + * The capabilities should be ordered from least to most invasive, i.e. CAP_SYS_ADMIN last.
> + *
> + * This sets PF_SUPERPRIV on the task if the capability is available on the
> + * assumption that it's about to be used.
> + */
> +bool capable_or(int cap1, int cap2)
> +{
> +	if (ns_capable_noaudit(&init_user_ns, cap1))
> +		return ns_capable(&init_user_ns, cap1);
> +
> +	if (ns_capable_noaudit(&init_user_ns, cap2))
> +		return ns_capable(&init_user_ns, cap2);
> +
> +	return ns_capable(&init_user_ns, cap1);

Hm, too bad about the repetition of work, but I guess it has to be
this way right now.

> +}
> +EXPORT_SYMBOL(capable_or);
> +
>  /**
>   * capable - Determine if the current task has a superior capability in effect
>   * @cap: The capability to be tested for
> -- 
> 2.36.0

Re: [PATCH v2 8/8] net: use new capable_or functionality

[Serge E. Hallyn]

On Mon, May 02, 2022 at 06:00:29PM +0200, Christian Göttsche wrote:
> Use the new added capable_or function in appropriate cases, where a task
> is required to have any of two capabilities.
> 
> Reorder CAP_SYS_ADMIN last.
> 
> Signed-off-by: Christian Göttsche <cgzones@googlemail.com>

Thanks, for 2-8:

Reviewed-by: Serge Hallyn <serge@hallyn.com>

though I'd still like to talk about the name :)

> ---
>  net/caif/caif_socket.c | 2 +-
>  net/unix/scm.c         | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/net/caif/caif_socket.c b/net/caif/caif_socket.c
> index 2b8892d502f7..60498148126c 100644
> --- a/net/caif/caif_socket.c
> +++ b/net/caif/caif_socket.c
> @@ -1036,7 +1036,7 @@ static int caif_create(struct net *net, struct socket *sock, int protocol,
>  		.usersize = sizeof_field(struct caifsock, conn_req.param)
>  	};
>  
> -	if (!capable(CAP_SYS_ADMIN) && !capable(CAP_NET_ADMIN))
> +	if (!capable_or(CAP_NET_ADMIN, CAP_SYS_ADMIN))
>  		return -EPERM;
>  	/*
>  	 * The sock->type specifies the socket type to use.
> diff --git a/net/unix/scm.c b/net/unix/scm.c
> index aa27a02478dc..821be80e6c85 100644
> --- a/net/unix/scm.c
> +++ b/net/unix/scm.c
> @@ -99,7 +99,7 @@ static inline bool too_many_unix_fds(struct task_struct *p)
>  	struct user_struct *user = current_user();
>  
>  	if (unlikely(user->unix_inflight > task_rlimit(p, RLIMIT_NOFILE)))
> -		return !capable(CAP_SYS_RESOURCE) && !capable(CAP_SYS_ADMIN);
> +		return !capable_or(CAP_SYS_RESOURCE, CAP_SYS_ADMIN);
>  	return false;
>  }
>  
> -- 
> 2.36.0

Re: [PATCH v2 8/8] net: use new capable_or functionality

[Serge E. Hallyn]

On Mon, May 09, 2022 at 12:15:09PM -0500, Serge E. Hallyn wrote:
> On Mon, May 02, 2022 at 06:00:29PM +0200, Christian Göttsche wrote:
> > Use the new added capable_or function in appropriate cases, where a task
> > is required to have any of two capabilities.
> > 
> > Reorder CAP_SYS_ADMIN last.
> > 
> > Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
> 
> Thanks, for 2-8:
> 
> Reviewed-by: Serge Hallyn <serge@hallyn.com>
> 
> though I'd still like to talk about the name :)

Just checking in - is this being discussed elsewhere?

> > ---
> >  net/caif/caif_socket.c | 2 +-
> >  net/unix/scm.c         | 2 +-
> >  2 files changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/net/caif/caif_socket.c b/net/caif/caif_socket.c
> > index 2b8892d502f7..60498148126c 100644
> > --- a/net/caif/caif_socket.c
> > +++ b/net/caif/caif_socket.c
> > @@ -1036,7 +1036,7 @@ static int caif_create(struct net *net, struct socket *sock, int protocol,
> >  		.usersize = sizeof_field(struct caifsock, conn_req.param)
> >  	};
> >  
> > -	if (!capable(CAP_SYS_ADMIN) && !capable(CAP_NET_ADMIN))
> > +	if (!capable_or(CAP_NET_ADMIN, CAP_SYS_ADMIN))
> >  		return -EPERM;
> >  	/*
> >  	 * The sock->type specifies the socket type to use.
> > diff --git a/net/unix/scm.c b/net/unix/scm.c
> > index aa27a02478dc..821be80e6c85 100644
> > --- a/net/unix/scm.c
> > +++ b/net/unix/scm.c
> > @@ -99,7 +99,7 @@ static inline bool too_many_unix_fds(struct task_struct *p)
> >  	struct user_struct *user = current_user();
> >  
> >  	if (unlikely(user->unix_inflight > task_rlimit(p, RLIMIT_NOFILE)))
> > -		return !capable(CAP_SYS_RESOURCE) && !capable(CAP_SYS_ADMIN);
> > +		return !capable_or(CAP_SYS_RESOURCE, CAP_SYS_ADMIN);
> >  	return false;
> >  }
> >  
> > -- 
> > 2.36.0

[PATCH v3 2/8] capability: use new capable_any functionality

[Christian Göttsche]

Use the new added capable_any function in appropriate cases, where a
task is required to have any of two capabilities.

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
---
v3:
  - rename to capable_any()
  - simplify checkpoint_restore_ns_capable()
---
 include/linux/capability.h | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/include/linux/capability.h b/include/linux/capability.h
index 7316d5339a6e..092cb9773079 100644
--- a/include/linux/capability.h
+++ b/include/linux/capability.h
@@ -266,18 +266,17 @@ extern bool file_ns_capable(const struct file *file, struct user_namespace *ns,
 extern bool ptracer_capable(struct task_struct *tsk, struct user_namespace *ns);
 static inline bool perfmon_capable(void)
 {
-	return capable(CAP_PERFMON) || capable(CAP_SYS_ADMIN);
+	return capable_any(CAP_PERFMON, CAP_SYS_ADMIN);
 }
 
 static inline bool bpf_capable(void)
 {
-	return capable(CAP_BPF) || capable(CAP_SYS_ADMIN);
+	return capable_any(CAP_BPF, CAP_SYS_ADMIN);
 }
 
 static inline bool checkpoint_restore_ns_capable(struct user_namespace *ns)
 {
-	return ns_capable(ns, CAP_CHECKPOINT_RESTORE) ||
-		ns_capable(ns, CAP_SYS_ADMIN);
+	return ns_capable_any(ns, CAP_CHECKPOINT_RESTORE, CAP_SYS_ADMIN);
 }
 
 /* audit system wants to get cap info from files as well */
-- 
2.36.1

[PATCH v3 3/8] block: use new capable_any functionality

[Christian Göttsche]

Use the new added capable_any function in appropriate cases, where a
task is required to have any of two capabilities.

Reorder CAP_SYS_ADMIN last.

Fixes: 94c4b4fd25e6 ("block: Check ADMIN before NICE for IOPRIO_CLASS_RT")

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
---
v3:
   rename to capable_any()
---
 block/ioprio.c | 9 +--------
 1 file changed, 1 insertion(+), 8 deletions(-)

diff --git a/block/ioprio.c b/block/ioprio.c
index 2fe068fcaad5..6441c052f837 100644
--- a/block/ioprio.c
+++ b/block/ioprio.c
@@ -37,14 +37,7 @@ int ioprio_check_cap(int ioprio)
 
 	switch (class) {
 		case IOPRIO_CLASS_RT:
-			/*
-			 * Originally this only checked for CAP_SYS_ADMIN,
-			 * which was implicitly allowed for pid 0 by security
-			 * modules such as SELinux. Make sure we check
-			 * CAP_SYS_ADMIN first to avoid a denial/avc for
-			 * possibly missing CAP_SYS_NICE permission.
-			 */
-			if (!capable(CAP_SYS_ADMIN) && !capable(CAP_SYS_NICE))
+			if (!capable_any(CAP_SYS_NICE, CAP_SYS_ADMIN))
 				return -EPERM;
 			fallthrough;
 			/* rt has prio field too */
-- 
2.36.1

[PATCH v3 4/8] drivers: use new capable_any functionality

[Christian Göttsche]

Use the new added capable_any function in appropriate cases, where a
task is required to have any of two capabilities.

Reorder CAP_SYS_ADMIN last.

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
---
v3:
   rename to capable_any()
---
 drivers/media/common/saa7146/saa7146_video.c     | 2 +-
 drivers/media/pci/bt8xx/bttv-driver.c            | 3 +--
 drivers/media/pci/saa7134/saa7134-video.c        | 3 +--
 drivers/media/platform/nxp/fsl-viu.c             | 2 +-
 drivers/media/test-drivers/vivid/vivid-vid-cap.c | 2 +-
 drivers/net/caif/caif_serial.c                   | 2 +-
 drivers/s390/block/dasd_eckd.c                   | 2 +-
 7 files changed, 7 insertions(+), 9 deletions(-)

diff --git a/drivers/media/common/saa7146/saa7146_video.c b/drivers/media/common/saa7146/saa7146_video.c
index 2296765079a4..f0d08935b096 100644
--- a/drivers/media/common/saa7146/saa7146_video.c
+++ b/drivers/media/common/saa7146/saa7146_video.c
@@ -469,7 +469,7 @@ static int vidioc_s_fbuf(struct file *file, void *fh, const struct v4l2_framebuf
 
 	DEB_EE("VIDIOC_S_FBUF\n");
 
-	if (!capable(CAP_SYS_ADMIN) && !capable(CAP_SYS_RAWIO))
+	if (!capable_any(CAP_SYS_RAWIO, CAP_SYS_ADMIN))
 		return -EPERM;
 
 	/* check args */
diff --git a/drivers/media/pci/bt8xx/bttv-driver.c b/drivers/media/pci/bt8xx/bttv-driver.c
index d40b537f4e98..7098cff2ea51 100644
--- a/drivers/media/pci/bt8xx/bttv-driver.c
+++ b/drivers/media/pci/bt8xx/bttv-driver.c
@@ -2567,8 +2567,7 @@ static int bttv_s_fbuf(struct file *file, void *f,
 	const struct bttv_format *fmt;
 	int retval;
 
-	if (!capable(CAP_SYS_ADMIN) &&
-		!capable(CAP_SYS_RAWIO))
+	if (!capable_any(CAP_SYS_RAWIO, CAP_SYS_ADMIN))
 		return -EPERM;
 
 	/* check args */
diff --git a/drivers/media/pci/saa7134/saa7134-video.c b/drivers/media/pci/saa7134/saa7134-video.c
index 4d8974c9fcc9..23104c04a9aa 100644
--- a/drivers/media/pci/saa7134/saa7134-video.c
+++ b/drivers/media/pci/saa7134/saa7134-video.c
@@ -1797,8 +1797,7 @@ static int saa7134_s_fbuf(struct file *file, void *f,
 	struct saa7134_dev *dev = video_drvdata(file);
 	struct saa7134_format *fmt;
 
-	if (!capable(CAP_SYS_ADMIN) &&
-	   !capable(CAP_SYS_RAWIO))
+	if (!capable_any(CAP_SYS_RAWIO, CAP_SYS_ADMIN))
 		return -EPERM;
 
 	/* check args */
diff --git a/drivers/media/platform/nxp/fsl-viu.c b/drivers/media/platform/nxp/fsl-viu.c
index afc96f6db2a1..81a90c113dc6 100644
--- a/drivers/media/platform/nxp/fsl-viu.c
+++ b/drivers/media/platform/nxp/fsl-viu.c
@@ -803,7 +803,7 @@ static int vidioc_s_fbuf(struct file *file, void *priv, const struct v4l2_frameb
 	const struct v4l2_framebuffer *fb = arg;
 	struct viu_fmt *fmt;
 
-	if (!capable(CAP_SYS_ADMIN) && !capable(CAP_SYS_RAWIO))
+	if (!capable_any(CAP_SYS_RAWIO, CAP_SYS_ADMIN))
 		return -EPERM;
 
 	/* check args */
diff --git a/drivers/media/test-drivers/vivid/vivid-vid-cap.c b/drivers/media/test-drivers/vivid/vivid-vid-cap.c
index b9caa4b26209..918913e47069 100644
--- a/drivers/media/test-drivers/vivid/vivid-vid-cap.c
+++ b/drivers/media/test-drivers/vivid/vivid-vid-cap.c
@@ -1253,7 +1253,7 @@ int vivid_vid_cap_s_fbuf(struct file *file, void *fh,
 	if (dev->multiplanar)
 		return -ENOTTY;
 
-	if (!capable(CAP_SYS_ADMIN) && !capable(CAP_SYS_RAWIO))
+	if (!capable_any(CAP_SYS_RAWIO, CAP_SYS_ADMIN))
 		return -EPERM;
 
 	if (dev->overlay_cap_owner)
diff --git a/drivers/net/caif/caif_serial.c b/drivers/net/caif/caif_serial.c
index 688075859ae4..ca3f82a0e3a6 100644
--- a/drivers/net/caif/caif_serial.c
+++ b/drivers/net/caif/caif_serial.c
@@ -326,7 +326,7 @@ static int ldisc_open(struct tty_struct *tty)
 	/* No write no play */
 	if (tty->ops->write == NULL)
 		return -EOPNOTSUPP;
-	if (!capable(CAP_SYS_ADMIN) && !capable(CAP_SYS_TTY_CONFIG))
+	if (!capable_any(CAP_SYS_TTY_CONFIG, CAP_SYS_ADMIN))
 		return -EPERM;
 
 	/* release devices to avoid name collision */
diff --git a/drivers/s390/block/dasd_eckd.c b/drivers/s390/block/dasd_eckd.c
index 836838f7d686..66f6db7a11fc 100644
--- a/drivers/s390/block/dasd_eckd.c
+++ b/drivers/s390/block/dasd_eckd.c
@@ -5330,7 +5330,7 @@ static int dasd_symm_io(struct dasd_device *device, void __user *argp)
 	char psf0, psf1;
 	int rc;
 
-	if (!capable(CAP_SYS_ADMIN) && !capable(CAP_SYS_RAWIO))
+	if (!capable_any(CAP_SYS_RAWIO, CAP_SYS_ADMIN))
 		return -EACCES;
 	psf0 = psf1 = 0;
 
-- 
2.36.1

[PATCH v3 5/8] fs: use new capable_any functionality

[Christian Göttsche]

Use the new added capable_any function in appropriate cases, where a
task is required to have any of two capabilities.

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
---
v3:
   rename to capable_any()
---
 fs/pipe.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/pipe.c b/fs/pipe.c
index 74ae9fafd25a..18ab3baeec44 100644
--- a/fs/pipe.c
+++ b/fs/pipe.c
@@ -776,7 +776,7 @@ bool too_many_pipe_buffers_hard(unsigned long user_bufs)
 
 bool pipe_is_unprivileged_user(void)
 {
-	return !capable(CAP_SYS_RESOURCE) && !capable(CAP_SYS_ADMIN);
+	return !capable_any(CAP_SYS_RESOURCE, CAP_SYS_ADMIN);
 }
 
 struct pipe_inode_info *alloc_pipe_info(void)
-- 
2.36.1

[PATCH v3 6/8] kernel: use new capable_any functionality

[Christian Göttsche]

Use the new added capable_any function in appropriate cases, where a
task is required to have any of two capabilities.

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
---
v3:
   rename to capable_any()
---
 kernel/fork.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/fork.c b/kernel/fork.c
index 9d44f2d46c69..1665fb4591c7 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -2104,7 +2104,7 @@ static __latent_entropy struct task_struct *copy_process(
 	retval = -EAGAIN;
 	if (is_ucounts_overlimit(task_ucounts(p), UCOUNT_RLIMIT_NPROC, rlimit(RLIMIT_NPROC))) {
 		if (p->real_cred->user != INIT_USER &&
-		    !capable(CAP_SYS_RESOURCE) && !capable(CAP_SYS_ADMIN))
+		    !capable_any(CAP_SYS_RESOURCE, CAP_SYS_ADMIN))
 			goto bad_fork_cleanup_count;
 	}
 	current->flags &= ~PF_NPROC_EXCEEDED;
-- 
2.36.1

[PATCH v3 7/8] bpf: use new capable_any functionality

[Christian Göttsche]

Use the new added capable_any function in appropriate cases, where a
task is required to have any of two capabilities.

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
---
v3:
   rename to capable_any()
---
 kernel/bpf/syscall.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index 2b69306d3c6e..92e274c7a5c2 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -2473,7 +2473,7 @@ static int bpf_prog_load(union bpf_attr *attr, bpfptr_t uattr)
 	    !bpf_capable())
 		return -EPERM;
 
-	if (is_net_admin_prog_type(type) && !capable(CAP_NET_ADMIN) && !capable(CAP_SYS_ADMIN))
+	if (is_net_admin_prog_type(type) && !capable_any(CAP_NET_ADMIN, CAP_SYS_ADMIN))
 		return -EPERM;
 	if (is_perfmon_prog_type(type) && !perfmon_capable())
 		return -EPERM;
-- 
2.36.1

[PATCH v3 8/8] net: use new capable_any functionality

[Christian Göttsche]

Use the new added capable_any function in appropriate cases, where a
task is required to have any of two capabilities.

Reorder CAP_SYS_ADMIN last.

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
---
v3:
  - rename to capable_any()
  - make use of ns_capable_any
---
 net/caif/caif_socket.c   |  2 +-
 net/core/sock.c          | 12 ++++--------
 net/ieee802154/socket.c  |  6 ++----
 net/ipv4/ip_sockglue.c   |  3 +--
 net/ipv6/ipv6_sockglue.c |  3 +--
 net/unix/scm.c           |  2 +-
 6 files changed, 10 insertions(+), 18 deletions(-)

diff --git a/net/caif/caif_socket.c b/net/caif/caif_socket.c
index 251e666ba9a2..2d3df7658e04 100644
--- a/net/caif/caif_socket.c
+++ b/net/caif/caif_socket.c
@@ -1036,7 +1036,7 @@ static int caif_create(struct net *net, struct socket *sock, int protocol,
 		.usersize = sizeof_field(struct caifsock, conn_req.param)
 	};
 
-	if (!capable(CAP_SYS_ADMIN) && !capable(CAP_NET_ADMIN))
+	if (!capable_any(CAP_NET_ADMIN, CAP_SYS_ADMIN))
 		return -EPERM;
 	/*
 	 * The sock->type specifies the socket type to use.
diff --git a/net/core/sock.c b/net/core/sock.c
index 2ff40dd0a7a6..6b04301982d8 100644
--- a/net/core/sock.c
+++ b/net/core/sock.c
@@ -1163,8 +1163,7 @@ int sock_setsockopt(struct socket *sock, int level, int optname,
 
 	case SO_PRIORITY:
 		if ((val >= 0 && val <= 6) ||
-		    ns_capable(sock_net(sk)->user_ns, CAP_NET_RAW) ||
-		    ns_capable(sock_net(sk)->user_ns, CAP_NET_ADMIN))
+		    ns_capable_any(sock_net(sk)->user_ns, CAP_NET_RAW, CAP_NET_ADMIN))
 			sk->sk_priority = val;
 		else
 			ret = -EPERM;
@@ -1309,8 +1308,7 @@ int sock_setsockopt(struct socket *sock, int level, int optname,
 			clear_bit(SOCK_PASSSEC, &sock->flags);
 		break;
 	case SO_MARK:
-		if (!ns_capable(sock_net(sk)->user_ns, CAP_NET_RAW) &&
-		    !ns_capable(sock_net(sk)->user_ns, CAP_NET_ADMIN)) {
+		if (!ns_capable_any(sock_net(sk)->user_ns, CAP_NET_RAW, CAP_NET_ADMIN)) {
 			ret = -EPERM;
 			break;
 		}
@@ -1318,8 +1316,7 @@ int sock_setsockopt(struct socket *sock, int level, int optname,
 		__sock_set_mark(sk, val);
 		break;
 	case SO_RCVMARK:
-		if (!ns_capable(sock_net(sk)->user_ns, CAP_NET_RAW) &&
-		    !ns_capable(sock_net(sk)->user_ns, CAP_NET_ADMIN)) {
+		if (!ns_capable_any(sock_net(sk)->user_ns, CAP_NET_RAW, CAP_NET_ADMIN)) {
 			ret = -EPERM;
 			break;
 		}
@@ -2680,8 +2677,7 @@ int __sock_cmsg_send(struct sock *sk, struct msghdr *msg, struct cmsghdr *cmsg,
 
 	switch (cmsg->cmsg_type) {
 	case SO_MARK:
-		if (!ns_capable(sock_net(sk)->user_ns, CAP_NET_RAW) &&
-		    !ns_capable(sock_net(sk)->user_ns, CAP_NET_ADMIN))
+		if (!ns_capable_any(sock_net(sk)->user_ns, CAP_NET_RAW, CAP_NET_ADMIN))
 			return -EPERM;
 		if (cmsg->cmsg_len != CMSG_LEN(sizeof(u32)))
 			return -EINVAL;
diff --git a/net/ieee802154/socket.c b/net/ieee802154/socket.c
index 718fb77bb372..882483602c27 100644
--- a/net/ieee802154/socket.c
+++ b/net/ieee802154/socket.c
@@ -894,8 +894,7 @@ static int dgram_setsockopt(struct sock *sk, int level, int optname,
 		ro->want_lqi = !!val;
 		break;
 	case WPAN_SECURITY:
-		if (!ns_capable(net->user_ns, CAP_NET_ADMIN) &&
-		    !ns_capable(net->user_ns, CAP_NET_RAW)) {
+		if (!ns_capable_any(net->user_ns, CAP_NET_RAW, CAP_NET_ADMIN)) {
 			err = -EPERM;
 			break;
 		}
@@ -918,8 +917,7 @@ static int dgram_setsockopt(struct sock *sk, int level, int optname,
 		}
 		break;
 	case WPAN_SECURITY_LEVEL:
-		if (!ns_capable(net->user_ns, CAP_NET_ADMIN) &&
-		    !ns_capable(net->user_ns, CAP_NET_RAW)) {
+		if (!ns_capable_any(net->user_ns, CAP_NET_RAW, CAP_NET_ADMIN)) {
 			err = -EPERM;
 			break;
 		}
diff --git a/net/ipv4/ip_sockglue.c b/net/ipv4/ip_sockglue.c
index 445a9ecaefa1..2da0a450edf6 100644
--- a/net/ipv4/ip_sockglue.c
+++ b/net/ipv4/ip_sockglue.c
@@ -1339,8 +1339,7 @@ static int do_ip_setsockopt(struct sock *sk, int level, int optname,
 		break;
 
 	case IP_TRANSPARENT:
-		if (!!val && !ns_capable(sock_net(sk)->user_ns, CAP_NET_RAW) &&
-		    !ns_capable(sock_net(sk)->user_ns, CAP_NET_ADMIN)) {
+		if (!!val && !ns_capable_any(sock_net(sk)->user_ns, CAP_NET_RAW, CAP_NET_ADMIN)) {
 			err = -EPERM;
 			break;
 		}
diff --git a/net/ipv6/ipv6_sockglue.c b/net/ipv6/ipv6_sockglue.c
index 222f6bf220ba..25babd7ce844 100644
--- a/net/ipv6/ipv6_sockglue.c
+++ b/net/ipv6/ipv6_sockglue.c
@@ -634,8 +634,7 @@ static int do_ipv6_setsockopt(struct sock *sk, int level, int optname,
 		break;
 
 	case IPV6_TRANSPARENT:
-		if (valbool && !ns_capable(net->user_ns, CAP_NET_RAW) &&
-		    !ns_capable(net->user_ns, CAP_NET_ADMIN)) {
+		if (valbool && !ns_capable_any(net->user_ns, CAP_NET_RAW, CAP_NET_ADMIN)) {
 			retv = -EPERM;
 			break;
 		}
diff --git a/net/unix/scm.c b/net/unix/scm.c
index aa27a02478dc..6c47baf04d7d 100644
--- a/net/unix/scm.c
+++ b/net/unix/scm.c
@@ -99,7 +99,7 @@ static inline bool too_many_unix_fds(struct task_struct *p)
 	struct user_struct *user = current_user();
 
 	if (unlikely(user->unix_inflight > task_rlimit(p, RLIMIT_NOFILE)))
-		return !capable(CAP_SYS_RESOURCE) && !capable(CAP_SYS_ADMIN);
+		return !capable_any(CAP_SYS_RESOURCE, CAP_SYS_ADMIN);
 	return false;
 }
 
-- 
2.36.1

[PATCH v3 1/8] capability: add any wrapper to test for multiple caps with exactly one audit message

[Christian Göttsche]

Add the interfaces `capable_any()` and `ns_capable_any()` as an
alternative to multiple `capable()`/`ns_capable()` calls, like
`capable_any(CAP_SYS_NICE, CAP_SYS_ADMIN)` instead of
`capable(CAP_SYS_NICE) || capable(CAP_SYS_ADMIN)`.

`capable_any()`/`ns_capable_any()` will in particular generate exactly
one audit message, either for the left most capability in effect or, if
the task has none, the first one.

This is especially helpful with regard to SELinux, where each audit
message about a not allowed capability will create an AVC denial.
Using this function with the least invasive capability as left most
argument (e.g. CAP_SYS_NICE before CAP_SYS_ADMIN) enables policy writers
to only allow the least invasive one and SELinux domains pass this check
with only capability:sys_nice or capability:sys_admin allowed without
any AVC denial message.

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>

---
v3:
   - rename to capable_any()
   - fix typo in function documentation
   - add ns_capable_any()
v2:
   avoid varargs and fix to two capabilities; capable_or3() can be added
   later if needed
---
 include/linux/capability.h | 10 +++++++
 kernel/capability.c        | 53 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 63 insertions(+)

diff --git a/include/linux/capability.h b/include/linux/capability.h
index 65efb74c3585..7316d5339a6e 100644
--- a/include/linux/capability.h
+++ b/include/linux/capability.h
@@ -208,7 +208,9 @@ extern bool has_capability_noaudit(struct task_struct *t, int cap);
 extern bool has_ns_capability_noaudit(struct task_struct *t,
 				      struct user_namespace *ns, int cap);
 extern bool capable(int cap);
+extern bool capable_any(int cap1, int cap2);
 extern bool ns_capable(struct user_namespace *ns, int cap);
+extern bool ns_capable_any(struct user_namespace *ns, int cap1, int cap2);
 extern bool ns_capable_noaudit(struct user_namespace *ns, int cap);
 extern bool ns_capable_setid(struct user_namespace *ns, int cap);
 #else
@@ -234,10 +236,18 @@ static inline bool capable(int cap)
 {
 	return true;
 }
+static inline bool capable_any(int cap1, int cap2)
+{
+	return true;
+}
 static inline bool ns_capable(struct user_namespace *ns, int cap)
 {
 	return true;
 }
+static inline bool ns_capable_any(struct user_namespace *ns, int cap1, int cap2)
+{
+	return true;
+}
 static inline bool ns_capable_noaudit(struct user_namespace *ns, int cap)
 {
 	return true;
diff --git a/kernel/capability.c b/kernel/capability.c
index 765194f5d678..ab9b889c3f4d 100644
--- a/kernel/capability.c
+++ b/kernel/capability.c
@@ -435,6 +435,59 @@ bool ns_capable_setid(struct user_namespace *ns, int cap)
 }
 EXPORT_SYMBOL(ns_capable_setid);
 
+/**
+ * ns_capable_any - Determine if the current task has one of two superior capabilities in effect
+ * @ns:  The usernamespace we want the capability in
+ * @cap1: The capabilities to be tested for first
+ * @cap2: The capabilities to be tested for secondly
+ *
+ * Return true if the current task has at least one of the two given superior
+ * capabilities currently available for use, false if not.
+ *
+ * In contrast to or'ing capable() this call will create exactly one audit
+ * message, either for @cap1, if it is granted or both are not permitted,
+ * or @cap2, if it is granted while the other one is not.
+ *
+ * The capabilities should be ordered from least to most invasive, i.e. CAP_SYS_ADMIN last.
+ *
+ * This sets PF_SUPERPRIV on the task if the capability is available on the
+ * assumption that it's about to be used.
+ */
+bool ns_capable_any(struct user_namespace *ns, int cap1, int cap2)
+{
+	if (ns_capable_noaudit(ns, cap1))
+		return ns_capable(ns, cap1);
+
+	if (ns_capable_noaudit(ns, cap2))
+		return ns_capable(ns, cap2);
+
+	return ns_capable(ns, cap1);
+}
+EXPORT_SYMBOL(ns_capable_any);
+
+/**
+ * capable_any - Determine if the current task has one of two superior capabilities in effect
+ * @cap1: The capabilities to be tested for first
+ * @cap2: The capabilities to be tested for secondly
+ *
+ * Return true if the current task has at least one of the two given superior
+ * capabilities currently available for use, false if not.
+ *
+ * In contrast to or'ing capable() this call will create exactly one audit
+ * message, either for @cap1, if it is granted or both are not permitted,
+ * or @cap2, if it is granted while the other one is not.
+ *
+ * The capabilities should be ordered from least to most invasive, i.e. CAP_SYS_ADMIN last.
+ *
+ * This sets PF_SUPERPRIV on the task if the capability is available on the
+ * assumption that it's about to be used.
+ */
+bool capable_any(int cap1, int cap2)
+{
+	return ns_capable_any(&init_user_ns, cap1, cap2);
+}
+EXPORT_SYMBOL(capable_any);
+
 /**
  * capable - Determine if the current task has a superior capability in effect
  * @cap: The capability to be tested for
-- 
2.36.1

Re: [PATCH v3 4/8] drivers: use new capable_any functionality

[Laurent Pinchart]

Hi Christian,

Thank you for the patch.

On Wed, Jun 15, 2022 at 05:26:18PM +0200, Christian Göttsche wrote:
> Use the new added capable_any function in appropriate cases, where a
> task is required to have any of two capabilities.
> 
> Reorder CAP_SYS_ADMIN last.
> 
> Signed-off-by: Christian Göttsche <cgzones@googlemail.com>

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
> v3:
>    rename to capable_any()
> ---
>  drivers/media/common/saa7146/saa7146_video.c     | 2 +-
>  drivers/media/pci/bt8xx/bttv-driver.c            | 3 +--
>  drivers/media/pci/saa7134/saa7134-video.c        | 3 +--
>  drivers/media/platform/nxp/fsl-viu.c             | 2 +-
>  drivers/media/test-drivers/vivid/vivid-vid-cap.c | 2 +-
>  drivers/net/caif/caif_serial.c                   | 2 +-
>  drivers/s390/block/dasd_eckd.c                   | 2 +-
>  7 files changed, 7 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/media/common/saa7146/saa7146_video.c b/drivers/media/common/saa7146/saa7146_video.c
> index 2296765079a4..f0d08935b096 100644
> --- a/drivers/media/common/saa7146/saa7146_video.c
> +++ b/drivers/media/common/saa7146/saa7146_video.c
> @@ -469,7 +469,7 @@ static int vidioc_s_fbuf(struct file *file, void *fh, const struct v4l2_framebuf
>  
>  	DEB_EE("VIDIOC_S_FBUF\n");
>  
> -	if (!capable(CAP_SYS_ADMIN) && !capable(CAP_SYS_RAWIO))
> +	if (!capable_any(CAP_SYS_RAWIO, CAP_SYS_ADMIN))
>  		return -EPERM;
>  
>  	/* check args */
> diff --git a/drivers/media/pci/bt8xx/bttv-driver.c b/drivers/media/pci/bt8xx/bttv-driver.c
> index d40b537f4e98..7098cff2ea51 100644
> --- a/drivers/media/pci/bt8xx/bttv-driver.c
> +++ b/drivers/media/pci/bt8xx/bttv-driver.c
> @@ -2567,8 +2567,7 @@ static int bttv_s_fbuf(struct file *file, void *f,
>  	const struct bttv_format *fmt;
>  	int retval;
>  
> -	if (!capable(CAP_SYS_ADMIN) &&
> -		!capable(CAP_SYS_RAWIO))
> +	if (!capable_any(CAP_SYS_RAWIO, CAP_SYS_ADMIN))
>  		return -EPERM;
>  
>  	/* check args */
> diff --git a/drivers/media/pci/saa7134/saa7134-video.c b/drivers/media/pci/saa7134/saa7134-video.c
> index 4d8974c9fcc9..23104c04a9aa 100644
> --- a/drivers/media/pci/saa7134/saa7134-video.c
> +++ b/drivers/media/pci/saa7134/saa7134-video.c
> @@ -1797,8 +1797,7 @@ static int saa7134_s_fbuf(struct file *file, void *f,
>  	struct saa7134_dev *dev = video_drvdata(file);
>  	struct saa7134_format *fmt;
>  
> -	if (!capable(CAP_SYS_ADMIN) &&
> -	   !capable(CAP_SYS_RAWIO))
> +	if (!capable_any(CAP_SYS_RAWIO, CAP_SYS_ADMIN))
>  		return -EPERM;
>  
>  	/* check args */
> diff --git a/drivers/media/platform/nxp/fsl-viu.c b/drivers/media/platform/nxp/fsl-viu.c
> index afc96f6db2a1..81a90c113dc6 100644
> --- a/drivers/media/platform/nxp/fsl-viu.c
> +++ b/drivers/media/platform/nxp/fsl-viu.c
> @@ -803,7 +803,7 @@ static int vidioc_s_fbuf(struct file *file, void *priv, const struct v4l2_frameb
>  	const struct v4l2_framebuffer *fb = arg;
>  	struct viu_fmt *fmt;
>  
> -	if (!capable(CAP_SYS_ADMIN) && !capable(CAP_SYS_RAWIO))
> +	if (!capable_any(CAP_SYS_RAWIO, CAP_SYS_ADMIN))
>  		return -EPERM;
>  
>  	/* check args */
> diff --git a/drivers/media/test-drivers/vivid/vivid-vid-cap.c b/drivers/media/test-drivers/vivid/vivid-vid-cap.c
> index b9caa4b26209..918913e47069 100644
> --- a/drivers/media/test-drivers/vivid/vivid-vid-cap.c
> +++ b/drivers/media/test-drivers/vivid/vivid-vid-cap.c
> @@ -1253,7 +1253,7 @@ int vivid_vid_cap_s_fbuf(struct file *file, void *fh,
>  	if (dev->multiplanar)
>  		return -ENOTTY;
>  
> -	if (!capable(CAP_SYS_ADMIN) && !capable(CAP_SYS_RAWIO))
> +	if (!capable_any(CAP_SYS_RAWIO, CAP_SYS_ADMIN))
>  		return -EPERM;
>  
>  	if (dev->overlay_cap_owner)
> diff --git a/drivers/net/caif/caif_serial.c b/drivers/net/caif/caif_serial.c
> index 688075859ae4..ca3f82a0e3a6 100644
> --- a/drivers/net/caif/caif_serial.c
> +++ b/drivers/net/caif/caif_serial.c
> @@ -326,7 +326,7 @@ static int ldisc_open(struct tty_struct *tty)
>  	/* No write no play */
>  	if (tty->ops->write == NULL)
>  		return -EOPNOTSUPP;
> -	if (!capable(CAP_SYS_ADMIN) && !capable(CAP_SYS_TTY_CONFIG))
> +	if (!capable_any(CAP_SYS_TTY_CONFIG, CAP_SYS_ADMIN))
>  		return -EPERM;
>  
>  	/* release devices to avoid name collision */
> diff --git a/drivers/s390/block/dasd_eckd.c b/drivers/s390/block/dasd_eckd.c
> index 836838f7d686..66f6db7a11fc 100644
> --- a/drivers/s390/block/dasd_eckd.c
> +++ b/drivers/s390/block/dasd_eckd.c
> @@ -5330,7 +5330,7 @@ static int dasd_symm_io(struct dasd_device *device, void __user *argp)
>  	char psf0, psf1;
>  	int rc;
>  
> -	if (!capable(CAP_SYS_ADMIN) && !capable(CAP_SYS_RAWIO))
> +	if (!capable_any(CAP_SYS_RAWIO, CAP_SYS_ADMIN))
>  		return -EACCES;
>  	psf0 = psf1 = 0;
>  

-- 
Regards,

Laurent Pinchart

Re: [PATCH v3 3/8] block: use new capable_any functionality

[Bart Van Assche]

On 6/15/22 08:26, Christian Göttsche wrote:
> Use the new added capable_any function in appropriate cases, where a
> task is required to have any of two capabilities.
> 
> Reorder CAP_SYS_ADMIN last.
> 
> Fixes: 94c4b4fd25e6 ("block: Check ADMIN before NICE for IOPRIO_CLASS_RT")
> 
> Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
> ---
> v3:
>     rename to capable_any()
> ---
>   block/ioprio.c | 9 +--------
>   1 file changed, 1 insertion(+), 8 deletions(-)
> 
> diff --git a/block/ioprio.c b/block/ioprio.c
> index 2fe068fcaad5..6441c052f837 100644
> --- a/block/ioprio.c
> +++ b/block/ioprio.c
> @@ -37,14 +37,7 @@ int ioprio_check_cap(int ioprio)
>   
>   	switch (class) {
>   		case IOPRIO_CLASS_RT:
> -			/*
> -			 * Originally this only checked for CAP_SYS_ADMIN,
> -			 * which was implicitly allowed for pid 0 by security
> -			 * modules such as SELinux. Make sure we check
> -			 * CAP_SYS_ADMIN first to avoid a denial/avc for
> -			 * possibly missing CAP_SYS_NICE permission.
> -			 */
> -			if (!capable(CAP_SYS_ADMIN) && !capable(CAP_SYS_NICE))
> +			if (!capable_any(CAP_SYS_NICE, CAP_SYS_ADMIN))
>   				return -EPERM;
>   			fallthrough;
>   			/* rt has prio field too */

Reviewed-by: Bart Van Assche <bvanassche@acm.org>

Re: [PATCH v3 1/8] capability: add any wrapper to test for multiple caps with exactly one audit message

[Serge E. Hallyn]

On Wed, Jun 15, 2022 at 05:26:23PM +0200, Christian Göttsche wrote:
> Add the interfaces `capable_any()` and `ns_capable_any()` as an
> alternative to multiple `capable()`/`ns_capable()` calls, like
> `capable_any(CAP_SYS_NICE, CAP_SYS_ADMIN)` instead of
> `capable(CAP_SYS_NICE) || capable(CAP_SYS_ADMIN)`.
> 
> `capable_any()`/`ns_capable_any()` will in particular generate exactly
> one audit message, either for the left most capability in effect or, if
> the task has none, the first one.
> 
> This is especially helpful with regard to SELinux, where each audit
> message about a not allowed capability will create an AVC denial.
> Using this function with the least invasive capability as left most
> argument (e.g. CAP_SYS_NICE before CAP_SYS_ADMIN) enables policy writers
> to only allow the least invasive one and SELinux domains pass this check
> with only capability:sys_nice or capability:sys_admin allowed without
> any AVC denial message.
> 
> Signed-off-by: Christian Göttsche <cgzones@googlemail.com>

Reviewed-by: Serge Hallyn <serge@hallyn.com>

> 
> ---
> v3:
>    - rename to capable_any()
>    - fix typo in function documentation
>    - add ns_capable_any()
> v2:
>    avoid varargs and fix to two capabilities; capable_or3() can be added
>    later if needed
> ---
>  include/linux/capability.h | 10 +++++++
>  kernel/capability.c        | 53 ++++++++++++++++++++++++++++++++++++++
>  2 files changed, 63 insertions(+)
> 
> diff --git a/include/linux/capability.h b/include/linux/capability.h
> index 65efb74c3585..7316d5339a6e 100644
> --- a/include/linux/capability.h
> +++ b/include/linux/capability.h
> @@ -208,7 +208,9 @@ extern bool has_capability_noaudit(struct task_struct *t, int cap);
>  extern bool has_ns_capability_noaudit(struct task_struct *t,
>  				      struct user_namespace *ns, int cap);
>  extern bool capable(int cap);
> +extern bool capable_any(int cap1, int cap2);
>  extern bool ns_capable(struct user_namespace *ns, int cap);
> +extern bool ns_capable_any(struct user_namespace *ns, int cap1, int cap2);
>  extern bool ns_capable_noaudit(struct user_namespace *ns, int cap);
>  extern bool ns_capable_setid(struct user_namespace *ns, int cap);
>  #else
> @@ -234,10 +236,18 @@ static inline bool capable(int cap)
>  {
>  	return true;
>  }
> +static inline bool capable_any(int cap1, int cap2)
> +{
> +	return true;
> +}
>  static inline bool ns_capable(struct user_namespace *ns, int cap)
>  {
>  	return true;
>  }
> +static inline bool ns_capable_any(struct user_namespace *ns, int cap1, int cap2)
> +{
> +	return true;
> +}
>  static inline bool ns_capable_noaudit(struct user_namespace *ns, int cap)
>  {
>  	return true;
> diff --git a/kernel/capability.c b/kernel/capability.c
> index 765194f5d678..ab9b889c3f4d 100644
> --- a/kernel/capability.c
> +++ b/kernel/capability.c
> @@ -435,6 +435,59 @@ bool ns_capable_setid(struct user_namespace *ns, int cap)
>  }
>  EXPORT_SYMBOL(ns_capable_setid);
>  
> +/**
> + * ns_capable_any - Determine if the current task has one of two superior capabilities in effect
> + * @ns:  The usernamespace we want the capability in
> + * @cap1: The capabilities to be tested for first
> + * @cap2: The capabilities to be tested for secondly
> + *
> + * Return true if the current task has at least one of the two given superior
> + * capabilities currently available for use, false if not.
> + *
> + * In contrast to or'ing capable() this call will create exactly one audit
> + * message, either for @cap1, if it is granted or both are not permitted,
> + * or @cap2, if it is granted while the other one is not.
> + *
> + * The capabilities should be ordered from least to most invasive, i.e. CAP_SYS_ADMIN last.
> + *
> + * This sets PF_SUPERPRIV on the task if the capability is available on the
> + * assumption that it's about to be used.
> + */
> +bool ns_capable_any(struct user_namespace *ns, int cap1, int cap2)
> +{
> +	if (ns_capable_noaudit(ns, cap1))
> +		return ns_capable(ns, cap1);
> +
> +	if (ns_capable_noaudit(ns, cap2))
> +		return ns_capable(ns, cap2);
> +
> +	return ns_capable(ns, cap1);
> +}
> +EXPORT_SYMBOL(ns_capable_any);
> +
> +/**
> + * capable_any - Determine if the current task has one of two superior capabilities in effect
> + * @cap1: The capabilities to be tested for first
> + * @cap2: The capabilities to be tested for secondly
> + *
> + * Return true if the current task has at least one of the two given superior
> + * capabilities currently available for use, false if not.
> + *
> + * In contrast to or'ing capable() this call will create exactly one audit
> + * message, either for @cap1, if it is granted or both are not permitted,
> + * or @cap2, if it is granted while the other one is not.
> + *
> + * The capabilities should be ordered from least to most invasive, i.e. CAP_SYS_ADMIN last.
> + *
> + * This sets PF_SUPERPRIV on the task if the capability is available on the
> + * assumption that it's about to be used.
> + */
> +bool capable_any(int cap1, int cap2)
> +{
> +	return ns_capable_any(&init_user_ns, cap1, cap2);
> +}
> +EXPORT_SYMBOL(capable_any);
> +
>  /**
>   * capable - Determine if the current task has a superior capability in effect
>   * @cap: The capability to be tested for
> -- 
> 2.36.1

Re: [PATCH v3 5/8] fs: use new capable_any functionality

[Christian Brauner]

On Wed, Jun 15, 2022 at 05:26:19PM +0200, Christian Göttsche wrote:
> Use the new added capable_any function in appropriate cases, where a
> task is required to have any of two capabilities.
> 
> Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
> ---

Not seeing the whole patch series so it's a bit difficult to judge but
in general we've needed something like this for quite some time.

> v3:
>    rename to capable_any()
> ---
>  fs/pipe.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/pipe.c b/fs/pipe.c
> index 74ae9fafd25a..18ab3baeec44 100644
> --- a/fs/pipe.c
> +++ b/fs/pipe.c
> @@ -776,7 +776,7 @@ bool too_many_pipe_buffers_hard(unsigned long user_bufs)
>  
>  bool pipe_is_unprivileged_user(void)
>  {
> -	return !capable(CAP_SYS_RESOURCE) && !capable(CAP_SYS_ADMIN);
> +	return !capable_any(CAP_SYS_RESOURCE, CAP_SYS_ADMIN);
>  }
>  
>  struct pipe_inode_info *alloc_pipe_info(void)
> -- 
> 2.36.1
> 

Re: [PATCH v3 5/8] fs: use new capable_any functionality

[Christian Göttsche]

On Tue, 28 Jun 2022 at 14:57, Christian Brauner <brauner@kernel.org> wrote:
>
> On Wed, Jun 15, 2022 at 05:26:19PM +0200, Christian Göttsche wrote:
> > Use the new added capable_any function in appropriate cases, where a
> > task is required to have any of two capabilities.
> >
> > Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
> > ---
>
> Not seeing the whole patch series so it's a bit difficult to judge but
> in general we've needed something like this for quite some time.

Full patch series:
https://patchwork.kernel.org/project/selinux/list/?series=650662
or
https://lore.kernel.org/lkml/20220615152623.311223-8-cgzones@googlemail.com/

> > v3:
> >    rename to capable_any()
> > ---
> >  fs/pipe.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/fs/pipe.c b/fs/pipe.c
> > index 74ae9fafd25a..18ab3baeec44 100644
> > --- a/fs/pipe.c
> > +++ b/fs/pipe.c
> > @@ -776,7 +776,7 @@ bool too_many_pipe_buffers_hard(unsigned long user_bufs)
> >
> >  bool pipe_is_unprivileged_user(void)
> >  {
> > -     return !capable(CAP_SYS_RESOURCE) && !capable(CAP_SYS_ADMIN);
> > +     return !capable_any(CAP_SYS_RESOURCE, CAP_SYS_ADMIN);
> >  }
> >
> >  struct pipe_inode_info *alloc_pipe_info(void)
> > --
> > 2.36.1
> >

Re: [PATCH v3 1/8] capability: add any wrapper to test for multiple caps with exactly one audit message

[Christian Göttsche]

On Mon, 27 Jun 2022 at 00:34, Serge E. Hallyn <serge@hallyn.com> wrote:
>
> On Wed, Jun 15, 2022 at 05:26:23PM +0200, Christian Göttsche wrote:
> > Add the interfaces `capable_any()` and `ns_capable_any()` as an
> > alternative to multiple `capable()`/`ns_capable()` calls, like
> > `capable_any(CAP_SYS_NICE, CAP_SYS_ADMIN)` instead of
> > `capable(CAP_SYS_NICE) || capable(CAP_SYS_ADMIN)`.
> >
> > `capable_any()`/`ns_capable_any()` will in particular generate exactly
> > one audit message, either for the left most capability in effect or, if
> > the task has none, the first one.
> >
> > This is especially helpful with regard to SELinux, where each audit
> > message about a not allowed capability will create an AVC denial.
> > Using this function with the least invasive capability as left most
> > argument (e.g. CAP_SYS_NICE before CAP_SYS_ADMIN) enables policy writers
> > to only allow the least invasive one and SELinux domains pass this check
> > with only capability:sys_nice or capability:sys_admin allowed without
> > any AVC denial message.
> >
> > Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
>
> Reviewed-by: Serge Hallyn <serge@hallyn.com>
>

Kindly ping.

So far patch 3 was reviewed [1] and patch 4 was reviewed [2,3] and
partially acked [4].

Currently this series trivially rebases on top of 6.0-rc1.
Should I send a rebased v4 or what is the best way to move forward?

[1]: https://lore.kernel.org/all/7fd6f544-0bd2-62fe-bddd-869364f351e8@acm.org/
[2]: https://lore.kernel.org/all/Yqn+sCXTHeTH5v+R@pendragon.ideasonboard.com/
[3]: https://lore.kernel.org/all/09374557-8c8d-1925-340c-784f29630ec5@kernel.org/
[4]: https://lore.kernel.org/all/73a603a2-5e5e-1b45-8e19-ab0795027336@xs4all.nl/

Re: [PATCH v3 1/8] capability: add any wrapper to test for multiple caps with exactly one audit message

[Paul Moore]

On Tue, Aug 30, 2022 at 11:05 AM Christian Göttsche
<cgzones@googlemail.com> wrote:
>
> On Mon, 27 Jun 2022 at 00:34, Serge E. Hallyn <serge@hallyn.com> wrote:
> >
> > On Wed, Jun 15, 2022 at 05:26:23PM +0200, Christian Göttsche wrote:
> > > Add the interfaces `capable_any()` and `ns_capable_any()` as an
> > > alternative to multiple `capable()`/`ns_capable()` calls, like
> > > `capable_any(CAP_SYS_NICE, CAP_SYS_ADMIN)` instead of
> > > `capable(CAP_SYS_NICE) || capable(CAP_SYS_ADMIN)`.
> > >
> > > `capable_any()`/`ns_capable_any()` will in particular generate exactly
> > > one audit message, either for the left most capability in effect or, if
> > > the task has none, the first one.
> > >
> > > This is especially helpful with regard to SELinux, where each audit
> > > message about a not allowed capability will create an AVC denial.
> > > Using this function with the least invasive capability as left most
> > > argument (e.g. CAP_SYS_NICE before CAP_SYS_ADMIN) enables policy writers
> > > to only allow the least invasive one and SELinux domains pass this check
> > > with only capability:sys_nice or capability:sys_admin allowed without
> > > any AVC denial message.
> > >
> > > Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
> >
> > Reviewed-by: Serge Hallyn <serge@hallyn.com>
>
> Kindly ping.
>
> So far patch 3 was reviewed [1] and patch 4 was reviewed [2,3] and
> partially acked [4].
>
> Currently this series trivially rebases on top of 6.0-rc1.
> Should I send a rebased v4 or what is the best way to move forward?

Hi Christian,

Sorry for the delay, this is one of those things that was stalled a
bit during the maintainer hand-off.  It's on my list of things to look
at, it is just unfortunate that we have had a lot of things going on
at the LSM layer lately; don't respin it just yet, let me take a quick
look first ...

-- 
paul-moore.com

Re: [PATCH v3 1/8] capability: add any wrapper to test for multiple caps with exactly one audit message

[Paul Moore]

On Wed, Jun 15, 2022 at 11:27 AM Christian Göttsche
<cgzones@googlemail.com> wrote:
>
> Add the interfaces `capable_any()` and `ns_capable_any()` as an
> alternative to multiple `capable()`/`ns_capable()` calls, like
> `capable_any(CAP_SYS_NICE, CAP_SYS_ADMIN)` instead of
> `capable(CAP_SYS_NICE) || capable(CAP_SYS_ADMIN)`.
>
> `capable_any()`/`ns_capable_any()` will in particular generate exactly
> one audit message, either for the left most capability in effect or, if
> the task has none, the first one.
>
> This is especially helpful with regard to SELinux, where each audit
> message about a not allowed capability will create an AVC denial.
> Using this function with the least invasive capability as left most
> argument (e.g. CAP_SYS_NICE before CAP_SYS_ADMIN) enables policy writers
> to only allow the least invasive one and SELinux domains pass this check
> with only capability:sys_nice or capability:sys_admin allowed without
> any AVC denial message.
>
> Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
>
> ---
> v3:
>    - rename to capable_any()
>    - fix typo in function documentation
>    - add ns_capable_any()
> v2:
>    avoid varargs and fix to two capabilities; capable_or3() can be added
>    later if needed
> ---
>  include/linux/capability.h | 10 +++++++
>  kernel/capability.c        | 53 ++++++++++++++++++++++++++++++++++++++
>  2 files changed, 63 insertions(+)

...

> diff --git a/kernel/capability.c b/kernel/capability.c
> index 765194f5d678..ab9b889c3f4d 100644
> --- a/kernel/capability.c
> +++ b/kernel/capability.c
> @@ -435,6 +435,59 @@ bool ns_capable_setid(struct user_namespace *ns, int cap)
>  }
>  EXPORT_SYMBOL(ns_capable_setid);
>
> +/**
> + * ns_capable_any - Determine if the current task has one of two superior capabilities in effect
> + * @ns:  The usernamespace we want the capability in
> + * @cap1: The capabilities to be tested for first
> + * @cap2: The capabilities to be tested for secondly
> + *
> + * Return true if the current task has at least one of the two given superior
> + * capabilities currently available for use, false if not.
> + *
> + * In contrast to or'ing capable() this call will create exactly one audit
> + * message, either for @cap1, if it is granted or both are not permitted,
> + * or @cap2, if it is granted while the other one is not.
> + *
> + * The capabilities should be ordered from least to most invasive, i.e. CAP_SYS_ADMIN last.
> + *
> + * This sets PF_SUPERPRIV on the task if the capability is available on the
> + * assumption that it's about to be used.
> + */
> +bool ns_capable_any(struct user_namespace *ns, int cap1, int cap2)
> +{
> +       if (ns_capable_noaudit(ns, cap1))
> +               return ns_capable(ns, cap1);
> +
> +       if (ns_capable_noaudit(ns, cap2))
> +               return ns_capable(ns, cap2);
> +
> +       return ns_capable(ns, cap1);

I'm slightly concerned that some people are going to be upset about
making an additional call into the capabilities code with this
function.  I think we need to be a bit more clever here to take out
some of the extra work.

I wonder if we create a new capability function, call it
ns_capable_audittrue(...) or something like that, that only generates
an audit record if the current task has the requested capability; if
the current task does not have the requested capability no audit
record is generated.  With this new function I think we could rewrite
ns_capable_any(...) like this:

  bool ns_capable_any(ns, cap1, cap2)
  {
    if (ns_capable_audittrue(ns, cap1))
      return true;
    if (ns_capable_audittrue(ns, cap2))
      return true;
    return ns_capable(ns, cap1);
  }

... we would still have an extra capability check in the failure case,
but that's an error case anyway and not likely to draw much concern.

Of course this would require some additional work, meaning a new
CAP_OPT_XXX flag (CAP_OPT_AUDITTRUE?), and updates to the individual
LSMs.  However, the good news here is that it appears only SELinux and
AppArmor would need modification (the others don't care about
capabilities or audit) and in each case the modification to support
the new CAP_OPT_AUDITTRUE flag look pretty simple.

Thoughts?

> +}
> +EXPORT_SYMBOL(ns_capable_any);

-- 
paul-moore.com

Re: [PATCH v3 1/8] capability: add any wrapper to test for multiple caps with exactly one audit message

[Paul Moore]

On Thu, Sep 1, 2022 at 8:56 PM Paul Moore <paul@paul-moore.com> wrote:
>
> On Wed, Jun 15, 2022 at 11:27 AM Christian Göttsche
> <cgzones@googlemail.com> wrote:
> >
> > Add the interfaces `capable_any()` and `ns_capable_any()` as an
> > alternative to multiple `capable()`/`ns_capable()` calls, like
> > `capable_any(CAP_SYS_NICE, CAP_SYS_ADMIN)` instead of
> > `capable(CAP_SYS_NICE) || capable(CAP_SYS_ADMIN)`.
> >
> > `capable_any()`/`ns_capable_any()` will in particular generate exactly
> > one audit message, either for the left most capability in effect or, if
> > the task has none, the first one.
> >
> > This is especially helpful with regard to SELinux, where each audit
> > message about a not allowed capability will create an AVC denial.
> > Using this function with the least invasive capability as left most
> > argument (e.g. CAP_SYS_NICE before CAP_SYS_ADMIN) enables policy writers
> > to only allow the least invasive one and SELinux domains pass this check
> > with only capability:sys_nice or capability:sys_admin allowed without
> > any AVC denial message.
> >
> > Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
> >
> > ---
> > v3:
> >    - rename to capable_any()
> >    - fix typo in function documentation
> >    - add ns_capable_any()
> > v2:
> >    avoid varargs and fix to two capabilities; capable_or3() can be added
> >    later if needed
> > ---
> >  include/linux/capability.h | 10 +++++++
> >  kernel/capability.c        | 53 ++++++++++++++++++++++++++++++++++++++
> >  2 files changed, 63 insertions(+)
>
> ...
>
> > diff --git a/kernel/capability.c b/kernel/capability.c
> > index 765194f5d678..ab9b889c3f4d 100644
> > --- a/kernel/capability.c
> > +++ b/kernel/capability.c
> > @@ -435,6 +435,59 @@ bool ns_capable_setid(struct user_namespace *ns, int cap)
> >  }
> >  EXPORT_SYMBOL(ns_capable_setid);
> >
> > +/**
> > + * ns_capable_any - Determine if the current task has one of two superior capabilities in effect
> > + * @ns:  The usernamespace we want the capability in
> > + * @cap1: The capabilities to be tested for first
> > + * @cap2: The capabilities to be tested for secondly
> > + *
> > + * Return true if the current task has at least one of the two given superior
> > + * capabilities currently available for use, false if not.
> > + *
> > + * In contrast to or'ing capable() this call will create exactly one audit
> > + * message, either for @cap1, if it is granted or both are not permitted,
> > + * or @cap2, if it is granted while the other one is not.
> > + *
> > + * The capabilities should be ordered from least to most invasive, i.e. CAP_SYS_ADMIN last.
> > + *
> > + * This sets PF_SUPERPRIV on the task if the capability is available on the
> > + * assumption that it's about to be used.
> > + */
> > +bool ns_capable_any(struct user_namespace *ns, int cap1, int cap2)
> > +{
> > +       if (ns_capable_noaudit(ns, cap1))
> > +               return ns_capable(ns, cap1);
> > +
> > +       if (ns_capable_noaudit(ns, cap2))
> > +               return ns_capable(ns, cap2);
> > +
> > +       return ns_capable(ns, cap1);
>
> I'm slightly concerned that some people are going to be upset about
> making an additional call into the capabilities code with this
> function.  I think we need to be a bit more clever here to take out
> some of the extra work.
>
> I wonder if we create a new capability function, call it
> ns_capable_audittrue(...) or something like that, that only generates
> an audit record if the current task has the requested capability ...

To be clear, when I mean by generating an audit record when true is
that the LSMs implementing the security_capable() hook would only call
their audit related code when the capability requirement was met, in
many cases that will *not* likely generate an audit record, but you
get the basic idea I hope.  For SELinux this would likely mean
modifying cred_has_capability() something like this ...

diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 03bca97c8b29..c1b7f0582d16 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -1581,6 +1581,7 @@ static int cred_has_capability(const struct cred *cred,
       u16 sclass;
       u32 sid = cred_sid(cred);
       u32 av = CAP_TO_MASK(cap);
+       bool audit;
       int rc;

       ad.type = LSM_AUDIT_DATA_CAP;
@@ -1601,7 +1602,10 @@ static int cred_has_capability(const struct cred *cred,

       rc = avc_has_perm_noaudit(&selinux_state,
                                 sid, sid, sclass, av, 0, &avd);
-       if (!(opts & CAP_OPT_NOAUDIT)) {
+       audit = !(opts & CAP_OPT_NOAUDIT);
+       if (opts & CAP_OPT_AUDITTRUE)
+               audit = !rc;
+       if (audit) {
               int rc2 = avc_audit(&selinux_state,
                                   sid, sid, sclass, av, &avd, rc, &ad);
               if (rc2)

[There is likely a cleaner patch than the above, this was just mean as
a demonstration]

> ... if
> the current task does not have the requested capability no audit
> record is generated.  With this new function I think we could rewrite
> ns_capable_any(...) like this:
>
>   bool ns_capable_any(ns, cap1, cap2)
>   {
>     if (ns_capable_audittrue(ns, cap1))
>       return true;
>     if (ns_capable_audittrue(ns, cap2))
>       return true;
>     return ns_capable(ns, cap1);
>   }
>
> ... we would still have an extra capability check in the failure case,
> but that's an error case anyway and not likely to draw much concern.
>
> Of course this would require some additional work, meaning a new
> CAP_OPT_XXX flag (CAP_OPT_AUDITTRUE?), and updates to the individual
> LSMs.  However, the good news here is that it appears only SELinux and
> AppArmor would need modification (the others don't care about
> capabilities or audit) and in each case the modification to support
> the new CAP_OPT_AUDITTRUE flag look pretty simple.
>
> Thoughts?
>
> > +}
> > +EXPORT_SYMBOL(ns_capable_any);

-- 
paul-moore.com