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

[Luis Chamberlain <mcgrof@kernel.org>]

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