[1/3] lsm,io_uring: add LSM hooks for the new uring_cmd file op

Commit Message

Paul Moore Aug. 22, 2022, 9:21 p.m. UTC

From: Luis Chamberlain <[e-mail]>

io-uring cmd support was added through ee692a21e9bf ("fs,io_uring:
add infrastructure for uring-cmd"), this extended 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.

This was discussed long ago without no clear pointer for something
conclusive, so this enables LSMs to at least reject this new file
operation.

[0] https://lkml.kernel.org/r/[e-mail]

Fixes: ee692a21e9bf ("fs,io_uring: add infrastructure for uring-cmd")
Signed-off-by: Luis Chamberlain <[e-mail]>
Acked-by: Jens Axboe <[e-mail]>
Signed-off-by: Paul Moore <[e-mail]>
---
 include/linux/lsm_hook_defs.h |    1 +
 include/linux/lsm_hooks.h     |    3 +++
 include/linux/security.h      |    5 +++++
 io_uring/uring_cmd.c          |    5 +++++
 security/security.c           |    4 ++++
 5 files changed, 18 insertions(+)

Comments

Greg Kroah-Hartman Aug. 23, 2022, 6:53 a.m. UTC | #1

On Mon, Aug 22, 2022 at 05:21:07PM -0400, Paul Moore wrote:
> From: Luis Chamberlain <[e-mail]>
> 
> io-uring cmd support was added through ee692a21e9bf ("fs,io_uring:
> add infrastructure for uring-cmd"), this extended 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.
> 
> This was discussed long ago without no clear pointer for something
> conclusive, so this enables LSMs to at least reject this new file
> operation.
> 
> [0] https://lkml.kernel.org/r/[e-mail]
> 
> Fixes: ee692a21e9bf ("fs,io_uring: add infrastructure for uring-cmd")

You are not "fixing" anything, you are adding new functionality.
Careful with using "Fixes:" for something like this, you will trigger
the bug-detection scripts and have to fend off stable bot emails for a
long time for stuff that should not be backported to stable trees.

thanks,

greg k-h

Paul Moore Aug. 23, 2022, 4:48 p.m. UTC | #2

On Tue, Aug 23, 2022 at 2:53 AM Greg Kroah-Hartman
<[e-mail]> wrote:
> On Mon, Aug 22, 2022 at 05:21:07PM -0400, Paul Moore wrote:
> > From: Luis Chamberlain <[e-mail]>
> >
> > io-uring cmd support was added through ee692a21e9bf ("fs,io_uring:
> > add infrastructure for uring-cmd"), this extended 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.
> >
> > This was discussed long ago without no clear pointer for something
> > conclusive, so this enables LSMs to at least reject this new file
> > operation.
> >
> > [0] https://lkml.kernel.org/r/[e-mail]
> >
> > Fixes: ee692a21e9bf ("fs,io_uring: add infrastructure for uring-cmd")
>
> You are not "fixing" anything, you are adding new functionality.
> Careful with using "Fixes:" for something like this, you will trigger
> the bug-detection scripts and have to fend off stable bot emails for a
> long time for stuff that should not be backported to stable trees.

This patch, as well as the SELinux and (soon to come) Smack hook
implementations, fix a LSM access control regression that occured when
the IORING_OP_URING_CMD functionality was merged in v5.19.  You may
disagree about this being a regression Greg, but there are at least
three people with their name on this patch that believe it is
important: Luis (patch author), Jens (io_uring maintainer), and myself
(LSM, SELinux maintainer).

--
paul-moore.com

Greg Kroah-Hartman Aug. 24, 2022, 6:12 a.m. UTC | #3

On Tue, Aug 23, 2022 at 12:48:30PM -0400, Paul Moore wrote:
> On Tue, Aug 23, 2022 at 2:53 AM Greg Kroah-Hartman
> <[e-mail]> wrote:
> > On Mon, Aug 22, 2022 at 05:21:07PM -0400, Paul Moore wrote:
> > > From: Luis Chamberlain <[e-mail]>
> > >
> > > io-uring cmd support was added through ee692a21e9bf ("fs,io_uring:
> > > add infrastructure for uring-cmd"), this extended 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.
> > >
> > > This was discussed long ago without no clear pointer for something
> > > conclusive, so this enables LSMs to at least reject this new file
> > > operation.
> > >
> > > [0] https://lkml.kernel.org/r/[e-mail]
> > >
> > > Fixes: ee692a21e9bf ("fs,io_uring: add infrastructure for uring-cmd")
> >
> > You are not "fixing" anything, you are adding new functionality.
> > Careful with using "Fixes:" for something like this, you will trigger
> > the bug-detection scripts and have to fend off stable bot emails for a
> > long time for stuff that should not be backported to stable trees.
> 
> This patch, as well as the SELinux and (soon to come) Smack hook
> implementations, fix a LSM access control regression that occured when
> the IORING_OP_URING_CMD functionality was merged in v5.19.  You may
> disagree about this being a regression Greg, but there are at least
> three people with their name on this patch that believe it is
> important: Luis (patch author), Jens (io_uring maintainer), and myself
> (LSM, SELinux maintainer).

Ok, I'll let it be, but note that "Fixes:" tags do not mean that a patch
will ever get backported to a stable tree, so I guess we don't have to
worry about it :)

thanks,

greg k-h

Paul Moore Aug. 24, 2022, 2 p.m. UTC | #4

On Wed, Aug 24, 2022 at 2:12 AM Greg Kroah-Hartman
<[e-mail]> wrote:
> On Tue, Aug 23, 2022 at 12:48:30PM -0400, Paul Moore wrote:
> > On Tue, Aug 23, 2022 at 2:53 AM Greg Kroah-Hartman
> > <[e-mail]> wrote:
> > > On Mon, Aug 22, 2022 at 05:21:07PM -0400, Paul Moore wrote:
> > > > From: Luis Chamberlain <[e-mail]>
> > > >
> > > > io-uring cmd support was added through ee692a21e9bf ("fs,io_uring:
> > > > add infrastructure for uring-cmd"), this extended 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.
> > > >
> > > > This was discussed long ago without no clear pointer for something
> > > > conclusive, so this enables LSMs to at least reject this new file
> > > > operation.
> > > >
> > > > [0] https://lkml.kernel.org/r/[e-mail]
> > > >
> > > > Fixes: ee692a21e9bf ("fs,io_uring: add infrastructure for uring-cmd")
> > >
> > > You are not "fixing" anything, you are adding new functionality.
> > > Careful with using "Fixes:" for something like this, you will trigger
> > > the bug-detection scripts and have to fend off stable bot emails for a
> > > long time for stuff that should not be backported to stable trees.
> >
> > This patch, as well as the SELinux and (soon to come) Smack hook
> > implementations, fix a LSM access control regression that occured when
> > the IORING_OP_URING_CMD functionality was merged in v5.19.  You may
> > disagree about this being a regression Greg, but there are at least
> > three people with their name on this patch that believe it is
> > important: Luis (patch author), Jens (io_uring maintainer), and myself
> > (LSM, SELinux maintainer).
>
> Ok, I'll let it be, but note that "Fixes:" tags do not mean that a patch
> will ever get backported to a stable tree, so I guess we don't have to
> worry about it :)

Ha!  Now that's the *proper* LSM dismissing GregKH comment this thread
was missing :)

Patch

diff --git a/include/linux/lsm_hook_defs.h b/include/linux/lsm_hook_defs.h
index 806448173033..60fff133c0b1 100644
--- a/include/linux/lsm_hook_defs.h
+++ b/include/linux/lsm_hook_defs.h
@@ -407,4 +407,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_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 84a0d7e02176..3aa6030302f5 100644
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h
@@ -1582,6 +1582,9 @@ 
  *      Check whether the current task is allowed to spawn a io_uring polling
  *      thread (IORING_SETUP_SQPOLL).
  *
+ * @uring_cmd:
+ *      Check whether the file_operations uring_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 1bc362cb413f..7bd0c490703d 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -2060,6 +2060,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_cmd(struct io_uring_cmd *ioucmd);
 #else
 static inline int security_uring_override_creds(const struct cred *new)
 {
@@ -2069,6 +2070,10 @@  static inline int security_uring_sqpoll(void)
 {
 	return 0;
 }
+static inline int security_uring_cmd(struct io_uring_cmd *ioucmd)
+{
+	return 0;
+}
 #endif /* CONFIG_SECURITY */
 #endif /* CONFIG_IO_URING */
 
diff --git a/io_uring/uring_cmd.c b/io_uring/uring_cmd.c
index 8e0cc2d9205e..0f7ad956ddcb 100644
--- a/io_uring/uring_cmd.c
+++ b/io_uring/uring_cmd.c
@@ -3,6 +3,7 @@ 
 #include <linux/errno.h>
 #include <linux/file.h>
 #include <linux/io_uring.h>
+#include <linux/security.h>
 
 #include <uapi/linux/io_uring.h>
 
@@ -88,6 +89,10 @@  int io_uring_cmd(struct io_kiocb *req, unsigned int issue_flags)
 	if (!req->file->f_op->uring_cmd)
 		return -EOPNOTSUPP;
 
+	ret = security_uring_cmd(ioucmd);
+	if (ret)
+		return ret;
+
 	if (ctx->flags & IORING_SETUP_SQE128)
 		issue_flags |= IO_URING_F_SQE128;
 	if (ctx->flags & IORING_SETUP_CQE32)
diff --git a/security/security.c b/security/security.c
index 14d30fec8a00..4b95de24bc8d 100644
--- a/security/security.c
+++ b/security/security.c
@@ -2660,4 +2660,8 @@  int security_uring_sqpoll(void)
 {
 	return call_int_hook(uring_sqpoll, 0);
 }
+int security_uring_cmd(struct io_uring_cmd *ioucmd)
+{
+	return call_int_hook(uring_cmd, 0, ioucmd);
+}
 #endif /* CONFIG_IO_URING */