2022-02-17 15:51:39

by Christian Göttsche

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

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 <[email protected]>
---
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


2022-05-02 23:17:41

by Christian Göttsche

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

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 <[email protected]>
---
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

2022-05-02 23:20:59

by Christian Göttsche

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

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 <[email protected]>
---
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

2022-05-02 23:28:34

by Christian Göttsche

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

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 <[email protected]>
---
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

2022-05-03 00:12:36

by Christian Göttsche

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

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 <[email protected]>

---
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

2022-05-03 01:09:46

by Christian Göttsche

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

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 <[email protected]>
---
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

2022-05-03 01:16:31

by Christian Göttsche

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

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 <[email protected]>
---
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

2022-05-03 01:21:10

by Christian Göttsche

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

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 <[email protected]>
---
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

2022-05-09 11:40:45

by Jiri Slaby

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

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 <[email protected]>
> ---

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

For the above:
Reviewed-by: Jiri Slaby <[email protected]>


> 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

2022-05-09 11:41:09

by Hans Verkuil

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



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 <[email protected]>
> ---
> 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 <[email protected]>

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;
>

2022-05-09 17:25:04

by Serge E. Hallyn

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

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 <[email protected]>
>
> ---
> 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

2022-06-15 15:45:13

by Christian Göttsche

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

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 <[email protected]>
---
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

2022-06-15 15:50:01

by Christian Göttsche

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

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 <[email protected]>
---
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

2022-06-15 15:52:25

by Christian Göttsche

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

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 <[email protected]>
---
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

2022-06-28 13:07:46

by Christian Brauner

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

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 <[email protected]>
> ---

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
>

2022-06-28 14:32:28

by Christian Göttsche

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

On Tue, 28 Jun 2022 at 14:57, Christian Brauner <[email protected]> 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 <[email protected]>
> > ---
>
> 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/[email protected]/

> > 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
> >