Re: [PATCH 03/17] io_uring: add infra and support for IORING_OP_URING_CMD

Luis Chamberlain
2022-03-11T17:12:03Z

On Thu, Mar 10, 2022 at 07:43:04PM -0700, Jens Axboe wrote:
> On 3/10/22 6:51 PM, Luis Chamberlain wrote:
> > On Tue, Mar 08, 2022 at 08:50:51PM +0530, Kanchan Joshi wrote:
> >> From: Jens Axboe <axboe@kernel.dk>
> >>
> >> This is a file private kind of request. io_uring doesn't know what's
> >> in this command type, it's for the file_operations->async_cmd()
> >> handler to deal with.
> >>
> >> Signed-off-by: Jens Axboe <axboe@kernel.dk>
> >> Signed-off-by: Kanchan Joshi <joshi.k@samsung.com>
> >> ---
> >
> > <-- snip -->
> >
> >> +static int io_uring_cmd(struct io_kiocb *req, unsigned int issue_flags)
> >> +{
> >> + struct file *file = req->file;
> >> + int ret;
> >> + struct io_uring_cmd *ioucmd = &req->uring_cmd;
> >> +
> >> + ioucmd->flags |= issue_flags;
> >> + ret = file->f_op->async_cmd(ioucmd);
> >
> > I think we're going to have to add a security_file_async_cmd() check
> > before this call here. Because otherwise we're enabling to, for
> > example, bypass security_file_ioctl() for example using the new
> > iouring-cmd interface.
> >
> > Or is this already thought out with the existing security_uring_*() stuff?
>
> Unless the request sets .audit_skip, it'll be included already in terms
> of logging.

Neat.

> But I'd prefer not to lodge this in with ioctls, unless
> we're going to be doing actual ioctls.

Oh sure, I have been an advocate to ensure folks don't conflate async_cmd
with ioctl. However it *can* enable subsystems to enable ioctl
passthrough, but each of those subsystems need to vet for this on their
own terms. I'd hate to see / hear some LSM surprises later.

> But definitely something to keep in mind and make sure that we're under
> the right umbrella in terms of auditing and security.

Paul, how about something like this for starters (and probably should
be squashed into this series so its not a separate commit) ?

From f3ddbe822374cc1c7002bd795c1ae486d370cbd1 Mon Sep 17 00:00:00 2001
From: Luis Chamberlain <mcgrof@kernel.org>
Date: Fri, 11 Mar 2022 08:55:50 -0800
Subject: [PATCH] lsm,io_uring: add LSM hooks to for the new async_cmd file op

io-uring is extending the struct file_operations to allow a new
command which each subsystem can use to enable command passthrough.
Add an LSM specific for the command passthrough which enables LSMs
to inspect the command details.

Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
---
fs/io_uring.c | 5 +++++
include/linux/lsm_hook_defs.h | 1 +
include/linux/lsm_hooks.h | 3 +++
include/linux/security.h | 5 +++++
security/security.c | 4 ++++
5 files changed, 18 insertions(+)

diff --git a/fs/io_uring.c b/fs/io_uring.c
index 3f6eacc98e31..1c4e6b2cb61a 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -4190,6 +4190,11 @@ static int io_uring_cmd_prep(struct io_kiocb *req,
struct io_ring_ctx *ctx = req->ctx;
struct io_uring_cmd *ioucmd = &req->uring_cmd;
u32 ucmd_flags = READ_ONCE(sqe->uring_cmd_flags);
+ int ret;
+
+ ret = security_uring_async_cmd(ioucmd);
+ if (ret)
+ return ret;

if (!req->file->f_op->async_cmd)
return -EOPNOTSUPP;
diff --git a/include/linux/lsm_hook_defs.h b/include/linux/lsm_hook_defs.h
index 819ec92dc2a8..4a20f8e6b295 100644
--- a/include/linux/lsm_hook_defs.h
+++ b/include/linux/lsm_hook_defs.h
@@ -404,4 +404,5 @@ LSM_HOOK(int, 0, perf_event_write, struct perf_event *event)
#ifdef CONFIG_IO_URING
LSM_HOOK(int, 0, uring_override_creds, const struct cred *new)
LSM_HOOK(int, 0, uring_sqpoll, void)
+LSM_HOOK(int, 0, uring_async_cmd, struct io_uring_cmd *ioucmd)
#endif /* CONFIG_IO_URING */
diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
index 3bf5c658bc44..21b18cf138c2 100644
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h
@@ -1569,6 +1569,9 @@
* Check whether the current task is allowed to spawn a io_uring polling
* thread (IORING_SETUP_SQPOLL).
*
+ * @uring_async_cmd:
+ * Check whether the file_operations async_cmd is allowed to run.
+ *
*/
union security_list_options {
#define LSM_HOOK(RET, DEFAULT, NAME, ...) RET (*NAME)(__VA_ARGS__);
diff --git a/include/linux/security.h b/include/linux/security.h
index 6d72772182c8..4d7f72813d75 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -2041,6 +2041,7 @@ static inline int security_perf_event_write(struct perf_event *event)
#ifdef CONFIG_SECURITY
extern int security_uring_override_creds(const struct cred *new);
extern int security_uring_sqpoll(void);
+extern int security_uring_async_cmd(struct io_uring_cmd *ioucmd);
#else
static inline int security_uring_override_creds(const struct cred *new)
{
@@ -2050,6 +2051,10 @@ static inline int security_uring_sqpoll(void)
{
return 0;
}
+static inline int security_uring_async_cmd(struct io_uring_cmd *ioucmd)
+{
+ return 0;
+}
#endif /* CONFIG_SECURITY */
#endif /* CONFIG_IO_URING */

diff --git a/security/security.c b/security/security.c
index 22261d79f333..ef96be2f953a 100644
--- a/security/security.c
+++ b/security/security.c
@@ -2640,4 +2640,8 @@ int security_uring_sqpoll(void)
{
return call_int_hook(uring_sqpoll, 0);
}
+int security_uring_async_cmd(struct io_uring_cmd *ioucmd)
+{
+ return call_int_hook(uring_async_cmd, 0, ioucmd);
+}
#endif /* CONFIG_IO_URING */
--
2.34.1