2022-02-17 16:27:50

by Christian Göttsche

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

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


2022-02-17 20:08:02

by Alexei Starovoitov

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

On Thu, Feb 17, 2022 at 6:50 AM Christian Göttsche
<[email protected]> 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")