[PATCH 0/3] LSM hooks for IORING_OP_URING_CMD

[PATCH 0/3] LSM hooks for IORING_OP_URING_CMD

[Paul Moore]

This patchset includes three patches: one to add a new LSM hook for
the IORING_OP_URING_CMD operation, one to add the SELinux
implementation for the new hook, and one to enable
IORING_OP_URING_CMD for /dev/null.  The last patch, the /dev/null
support, is obviously not critical but it makes testing so much
easier and I believe is in keeping with the general motivation behind
/dev/null.

Luis' patch has already been vetted by Jens and the io_uring folks,
so the only new bits are the SELinux implementation and the trivial
/dev/null implementation of IORING_OP_URING_CMD.  Assuming no one
has any objections over the next few days, I'll plan on sending this
up to Linus during the v6.0-rcX cycle.

I believe Casey is also currently working on Smack support for the
IORING_OP_URING_CMD hook, and as soon as he is ready I can add it
to this patchset (or Casey can send it up himself).

-Paul

---

Luis Chamberlain (1):
      lsm,io_uring: add LSM hooks for the new uring_cmd file op

Paul Moore (2):
      selinux: implement the security_uring_cmd() LSM hook
      /dev/null: add IORING_OP_URING_CMD support


 drivers/char/mem.c                  |  6 ++++++
 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 ++++
 security/selinux/hooks.c            | 24 ++++++++++++++++++++++++
 security/selinux/include/classmap.h |  2 +-
 8 files changed, 49 insertions(+), 1 deletion(-)

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

[Paul Moore]

From: Luis Chamberlain <mcgrof@kernel.org>

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/8adf55db-7bab-f59d-d612-ed906b948d19@schaufler-ca.com

Fixes: ee692a21e9bf ("fs,io_uring: add infrastructure for uring-cmd")
Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
Acked-by: Jens Axboe <axboe@kernel.dk>
Signed-off-by: Paul Moore <paul@paul-moore.com>
---
 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(+)

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

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

[Greg Kroah-Hartman]

On Mon, Aug 22, 2022 at 05:21:07PM -0400, Paul Moore wrote:
> From: Luis Chamberlain <mcgrof@kernel.org>
> 
> 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/8adf55db-7bab-f59d-d612-ed906b948d19@schaufler-ca.com
> 
> 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

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

[Paul Moore]

On Tue, Aug 23, 2022 at 2:53 AM Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
> On Mon, Aug 22, 2022 at 05:21:07PM -0400, Paul Moore wrote:
> > From: Luis Chamberlain <mcgrof@kernel.org>
> >
> > 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/8adf55db-7bab-f59d-d612-ed906b948d19@schaufler-ca.com
> >
> > 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

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

[Greg Kroah-Hartman]

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
> <gregkh@linuxfoundation.org> wrote:
> > On Mon, Aug 22, 2022 at 05:21:07PM -0400, Paul Moore wrote:
> > > From: Luis Chamberlain <mcgrof@kernel.org>
> > >
> > > 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/8adf55db-7bab-f59d-d612-ed906b948d19@schaufler-ca.com
> > >
> > > 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

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

[Paul Moore]

On Wed, Aug 24, 2022 at 2:12 AM Greg Kroah-Hartman
<gregkh@linuxfoundation.org> 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
> > <gregkh@linuxfoundation.org> wrote:
> > > On Mon, Aug 22, 2022 at 05:21:07PM -0400, Paul Moore wrote:
> > > > From: Luis Chamberlain <mcgrof@kernel.org>
> > > >
> > > > 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/8adf55db-7bab-f59d-d612-ed906b948d19@schaufler-ca.com
> > > >
> > > > 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 :)

-- 
paul-moore.com

[PATCH 2/3] selinux: implement the security_uring_cmd() LSM hook

[Paul Moore]

Add a SELinux access control for the iouring IORING_OP_URING_CMD
command.  This includes the addition of a new permission in the
existing "io_uring" object class: "cmd".  The subject of the new
permission check is the domain of the process requesting access, the
object is the open file which points to the device/file that is the
target of the IORING_OP_URING_CMD operation.  A sample policy rule
is shown below:

  allow <domain> <file>:io_uring { cmd };

Cc: stable@vger.kernel.org
Fixes: ee692a21e9bf ("fs,io_uring: add infrastructure for uring-cmd")
Signed-off-by: Paul Moore <paul@paul-moore.com>
---
 security/selinux/hooks.c            |   24 ++++++++++++++++++++++++
 security/selinux/include/classmap.h |    2 +-
 2 files changed, 25 insertions(+), 1 deletion(-)

diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 79573504783b..03bca97c8b29 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -91,6 +91,7 @@
 #include <uapi/linux/mount.h>
 #include <linux/fsnotify.h>
 #include <linux/fanotify.h>
+#include <linux/io_uring.h>
 
 #include "avc.h"
 #include "objsec.h"
@@ -6987,6 +6988,28 @@ static int selinux_uring_sqpoll(void)
 	return avc_has_perm(&selinux_state, sid, sid,
 			    SECCLASS_IO_URING, IO_URING__SQPOLL, NULL);
 }
+
+/**
+ * selinux_uring_cmd - check if IORING_OP_URING_CMD is allowed
+ * @ioucmd: the io_uring command structure
+ *
+ * Check to see if the current domain is allowed to execute an
+ * IORING_OP_URING_CMD against the device/file specified in @ioucmd.
+ *
+ */
+static int selinux_uring_cmd(struct io_uring_cmd *ioucmd)
+{
+	struct file *file = ioucmd->file;
+	struct inode *inode = file_inode(file);
+	struct inode_security_struct *isec = selinux_inode(inode);
+	struct common_audit_data ad;
+
+	ad.type = LSM_AUDIT_DATA_FILE;
+	ad.u.file = file;
+
+	return avc_has_perm(&selinux_state, current_sid(), isec->sid,
+			    SECCLASS_IO_URING, IO_URING__CMD, &ad);
+}
 #endif /* CONFIG_IO_URING */
 
 /*
@@ -7231,6 +7254,7 @@ static struct security_hook_list selinux_hooks[] __lsm_ro_after_init = {
 #ifdef CONFIG_IO_URING
 	LSM_HOOK_INIT(uring_override_creds, selinux_uring_override_creds),
 	LSM_HOOK_INIT(uring_sqpoll, selinux_uring_sqpoll),
+	LSM_HOOK_INIT(uring_cmd, selinux_uring_cmd),
 #endif
 
 	/*
diff --git a/security/selinux/include/classmap.h b/security/selinux/include/classmap.h
index ff757ae5f253..1c2f41ff4e55 100644
--- a/security/selinux/include/classmap.h
+++ b/security/selinux/include/classmap.h
@@ -253,7 +253,7 @@ const struct security_class_mapping secclass_map[] = {
 	{ "anon_inode",
 	  { COMMON_FILE_PERMS, NULL } },
 	{ "io_uring",
-	  { "override_creds", "sqpoll", NULL } },
+	  { "override_creds", "sqpoll", "cmd", NULL } },
 	{ NULL }
   };
 

Re: [PATCH 2/3] selinux: implement the security_uring_cmd() LSM hook

[Greg Kroah-Hartman]

On Mon, Aug 22, 2022 at 05:21:13PM -0400, Paul Moore wrote:
> Add a SELinux access control for the iouring IORING_OP_URING_CMD
> command.  This includes the addition of a new permission in the
> existing "io_uring" object class: "cmd".  The subject of the new
> permission check is the domain of the process requesting access, the
> object is the open file which points to the device/file that is the
> target of the IORING_OP_URING_CMD operation.  A sample policy rule
> is shown below:
> 
>   allow <domain> <file>:io_uring { cmd };
> 
> Cc: stable@vger.kernel.org

This is not stable material as you are adding a new feature.  Please
read the stable documentation for what is and is not allowed.

thanks,

greg k-h

Re: [PATCH 2/3] selinux: implement the security_uring_cmd() LSM hook

[Paul Moore]

On Tue, Aug 23, 2022 at 2:52 AM Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
> On Mon, Aug 22, 2022 at 05:21:13PM -0400, Paul Moore wrote:
> > Add a SELinux access control for the iouring IORING_OP_URING_CMD
> > command.  This includes the addition of a new permission in the
> > existing "io_uring" object class: "cmd".  The subject of the new
> > permission check is the domain of the process requesting access, the
> > object is the open file which points to the device/file that is the
> > target of the IORING_OP_URING_CMD operation.  A sample policy rule
> > is shown below:
> >
> >   allow <domain> <file>:io_uring { cmd };
> >
> > Cc: stable@vger.kernel.org
>
> This is not stable material as you are adding a new feature.  Please
> read the stable documentation for what is and is not allowed.

Strongly disagree, see my comments on patch 1/3 in this patchset.

-- 
paul-moore.com

Re: [PATCH 2/3] selinux: implement the security_uring_cmd() LSM hook

[Joel Granados]

[-- Attachment #1: Type: text/plain, Size: 7540 bytes --]

Hey Paul

I realize that you have already sent this upstream but I wanted to share
the Selinux part of the testing that we did to see if there is any
feedback.

With my tests I see that the selinux_uring_cmd hook is run and it
results in a "avc : denied" when I run it with selinux in permissive
mode with an unpriviledged user. I assume that this is the expected
behavior. Here is how I tested

*** With the patch:
* I ran the io_uring_passthrough.c test on a char device with an
  unpriviledged user.
* I took care of changing the permissions of /dev/ng0n1 to 666 prior
  to any testing.
* made sure that Selinux was in permissive mode.
* Made sure to have audit activated by passing "audit=1" to the kernel
* After noticing that some audit messages where getting lost I upped the
  backlog limit to 256
* Prior to executing the test, I also placed a breakpoint inside
  selinux_uring_cmd to make sure that it was executed.
* This is the output of the audit when I executed the test:

  [  136.615924] audit: type=1400 audit(1662043624.701:94): avc:  denied  { create } for  pid=263 comm="io_uring_passth" anonclass=[io_uring] scontext=system_u:system_r:kernel_t tcontext=system_u:object_r:kernel_t tclass=anon_inode permissive=1
  [  136.621036] audit: type=1300 audit(1662043624.701:94): arch=c000003e syscall=425 success=yes exit=3 a0=40 a1=7ffca29835a0 a2=7ffca29835a0 a3=561529be2300 items=0 ppid=252 pid=263 auid=1001 uid=1001 gid=1002 euid=1001 suid=1001 fsuid=1001 egid=1002 sgid=1002 fsgid=1002 tty=pts1 ses=3 comm="io_uring_passth" exe="/mnt/src/liburing/test/io_uring_passthrough.t" subj=system_u:system_r:kernel_t key=(null)
  [  136.624812] audit: type=1327 audit(1662043624.701:94): proctitle=2F6D6E742F7372632F6C69627572696E672F746573742F696F5F7572696E675F706173737468726F7567682E74002F6465762F6E67306E31
  [  136.626074] audit: type=1400 audit(1662043624.702:95): avc:  denied  { map } for  pid=263 comm="io_uring_passth" path="anon_inode:[io_uring]" dev="anon_inodefs" ino=11715 scontext=system_u:system_r:kernel_t tcontext=system_u:object_r:kernel_t tclass=anon_inode permissive=1
  [  136.628012] audit: type=1400 audit(1662043624.702:95): avc:  denied  { read write } for  pid=263 comm="io_uring_passth" path="anon_inode:[io_uring]" dev="anon_inodefs" ino=11715 scontext=system_u:system_r:kernel_t tcontext=system_u:object_r:kernel_t tclass=anon_inode permissive=1
  [  136.629873] audit: type=1300 audit(1662043624.702:95): arch=c000003e syscall=9 success=yes exit=140179765297152 a0=0 a1=1380 a2=3 a3=8001 items=0 ppid=252 pid=263 auid=1001 uid=1001 gid=1002 euid=1001 suid=1001 fsuid=1001 egid=1002 sgid=1002 fsgid=1002 tty=pts1 ses=3 comm="io_uring_passth" exe="/mnt/src/liburing/test/io_uring_passthrough.t" subj=system_u:system_r:kernel_t key=(null)
  [  136.632415] audit: type=1327 audit(1662043624.702:95): proctitle=2F6D6E742F7372632F6C69627572696E672F746573742F696F5F7572696E675F706173737468726F7567682E74002F6465762F6E67306E31
  [  136.633652] audit: type=1400 audit(1662043624.705:96): avc:  denied  { cmd } for  pid=263 comm="io_uring_passth" path="/dev/ng0n1" dev="devtmpfs" ino=120 scontext=system_u:system_r:kernel_t tcontext=system_u:object_r:device_t tclass=io_uring permissive=1
  [  136.635384] audit: type=1336 audit(1662043624.705:96): uring_op=46 items=0 ppid=252 pid=263 uid=1001 gid=1002 euid=1001 suid=1001 fsuid=1001 egid=1002 sgid=1002 fsgid=1002 subj=system_u:system_r:kernel_t key=(null)
  [  136.636863] audit: type=1336 audit(1662043624.705:96): uring_op=46 items=0 ppid=252 pid=263 uid=1001 gid=1002 euid=1001 suid=1001 fsuid=1001 egid=1002 sgid=1002 fsgid=1002 subj=system_u:system_r:kernel_t key=(null)

* From the output on time 136.633652 I see that the access should have
  been denied had selinux been enforcing. 
* I also saw that the breakpoint hit.

*** Without the patch:
* I ran the io_uring_passthrough.c test on a char device with an
  unpriviledged user.
* I took care of changing the permissions of /dev/ng0n1 to 666 prior
  to any testing.
* made sure that Selinux was in permissive mode.
* Made sure to have audit activated by passing "audit=1" to the kernel
* After noticing that some audit messages where getting lost I upped the
  backlog limit to 256
* There were no audit messages when I executed the test.

As with my smack tests I would really appreciate feecback on the
approach I took to testing and it's validity.

Thx in advance

Best


On Mon, Aug 22, 2022 at 05:21:13PM -0400, Paul Moore wrote:
> Add a SELinux access control for the iouring IORING_OP_URING_CMD
> command.  This includes the addition of a new permission in the
> existing "io_uring" object class: "cmd".  The subject of the new
> permission check is the domain of the process requesting access, the
> object is the open file which points to the device/file that is the
> target of the IORING_OP_URING_CMD operation.  A sample policy rule
> is shown below:
> 
>   allow <domain> <file>:io_uring { cmd };
> 
> Cc: stable@vger.kernel.org
> Fixes: ee692a21e9bf ("fs,io_uring: add infrastructure for uring-cmd")
> Signed-off-by: Paul Moore <paul@paul-moore.com>
> ---
>  security/selinux/hooks.c            |   24 ++++++++++++++++++++++++
>  security/selinux/include/classmap.h |    2 +-
>  2 files changed, 25 insertions(+), 1 deletion(-)
> 
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index 79573504783b..03bca97c8b29 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -91,6 +91,7 @@
>  #include <uapi/linux/mount.h>
>  #include <linux/fsnotify.h>
>  #include <linux/fanotify.h>
> +#include <linux/io_uring.h>
>  
>  #include "avc.h"
>  #include "objsec.h"
> @@ -6987,6 +6988,28 @@ static int selinux_uring_sqpoll(void)
>  	return avc_has_perm(&selinux_state, sid, sid,
>  			    SECCLASS_IO_URING, IO_URING__SQPOLL, NULL);
>  }
> +
> +/**
> + * selinux_uring_cmd - check if IORING_OP_URING_CMD is allowed
> + * @ioucmd: the io_uring command structure
> + *
> + * Check to see if the current domain is allowed to execute an
> + * IORING_OP_URING_CMD against the device/file specified in @ioucmd.
> + *
> + */
> +static int selinux_uring_cmd(struct io_uring_cmd *ioucmd)
> +{
> +	struct file *file = ioucmd->file;
> +	struct inode *inode = file_inode(file);
> +	struct inode_security_struct *isec = selinux_inode(inode);
> +	struct common_audit_data ad;
> +
> +	ad.type = LSM_AUDIT_DATA_FILE;
> +	ad.u.file = file;
> +
> +	return avc_has_perm(&selinux_state, current_sid(), isec->sid,
> +			    SECCLASS_IO_URING, IO_URING__CMD, &ad);
> +}
>  #endif /* CONFIG_IO_URING */
>  
>  /*
> @@ -7231,6 +7254,7 @@ static struct security_hook_list selinux_hooks[] __lsm_ro_after_init = {
>  #ifdef CONFIG_IO_URING
>  	LSM_HOOK_INIT(uring_override_creds, selinux_uring_override_creds),
>  	LSM_HOOK_INIT(uring_sqpoll, selinux_uring_sqpoll),
> +	LSM_HOOK_INIT(uring_cmd, selinux_uring_cmd),
>  #endif
>  
>  	/*
> diff --git a/security/selinux/include/classmap.h b/security/selinux/include/classmap.h
> index ff757ae5f253..1c2f41ff4e55 100644
> --- a/security/selinux/include/classmap.h
> +++ b/security/selinux/include/classmap.h
> @@ -253,7 +253,7 @@ const struct security_class_mapping secclass_map[] = {
>  	{ "anon_inode",
>  	  { COMMON_FILE_PERMS, NULL } },
>  	{ "io_uring",
> -	  { "override_creds", "sqpoll", NULL } },
> +	  { "override_creds", "sqpoll", "cmd", NULL } },
>  	{ NULL }
>    };
>  
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH 2/3] selinux: implement the security_uring_cmd() LSM hook

[Paul Moore]

On Thu, Sep 1, 2022 at 4:15 PM Joel Granados <j.granados@samsung.com> wrote:
> Hey Paul
>
> I realize that you have already sent this upstream but I wanted to share
> the Selinux part of the testing that we did to see if there is any
> feedback.
>
> With my tests I see that the selinux_uring_cmd hook is run and it
> results in a "avc : denied" when I run it with selinux in permissive
> mode with an unpriviledged user. I assume that this is the expected
> behavior. Here is how I tested
>
> *** With the patch:
> * I ran the io_uring_passthrough.c test on a char device with an
>   unpriviledged user.
> * I took care of changing the permissions of /dev/ng0n1 to 666 prior
>   to any testing.
> * made sure that Selinux was in permissive mode.
> * Made sure to have audit activated by passing "audit=1" to the kernel
> * After noticing that some audit messages where getting lost I upped the
>   backlog limit to 256
> * Prior to executing the test, I also placed a breakpoint inside
>   selinux_uring_cmd to make sure that it was executed.
> * This is the output of the audit when I executed the test:
>
>   [  136.615924] audit: type=1400 audit(1662043624.701:94): avc:  denied  { create } for  pid=263 comm="io_uring_passth" anonclass=[io_uring] scontext=system_u:system_r:kernel_t tcontext=system_u:object_r:kernel_t tclass=anon_inode permissive=1
>   [  136.621036] audit: type=1300 audit(1662043624.701:94): arch=c000003e syscall=425 success=yes exit=3 a0=40 a1=7ffca29835a0 a2=7ffca29835a0 a3=561529be2300 items=0 ppid=252 pid=263 auid=1001 uid=1001 gid=1002 euid=1001 suid=1001 fsuid=1001 egid=1002 sgid=1002 fsgid=1002 tty=pts1 ses=3 comm="io_uring_passth" exe="/mnt/src/liburing/test/io_uring_passthrough.t" subj=system_u:system_r:kernel_t key=(null)
>   [  136.624812] audit: type=1327 audit(1662043624.701:94): proctitle=2F6D6E742F7372632F6C69627572696E672F746573742F696F5F7572696E675F706173737468726F7567682E74002F6465762F6E67306E31
>   [  136.626074] audit: type=1400 audit(1662043624.702:95): avc:  denied  { map } for  pid=263 comm="io_uring_passth" path="anon_inode:[io_uring]" dev="anon_inodefs" ino=11715 scontext=system_u:system_r:kernel_t tcontext=system_u:object_r:kernel_t tclass=anon_inode permissive=1
>   [  136.628012] audit: type=1400 audit(1662043624.702:95): avc:  denied  { read write } for  pid=263 comm="io_uring_passth" path="anon_inode:[io_uring]" dev="anon_inodefs" ino=11715 scontext=system_u:system_r:kernel_t tcontext=system_u:object_r:kernel_t tclass=anon_inode permissive=1
>   [  136.629873] audit: type=1300 audit(1662043624.702:95): arch=c000003e syscall=9 success=yes exit=140179765297152 a0=0 a1=1380 a2=3 a3=8001 items=0 ppid=252 pid=263 auid=1001 uid=1001 gid=1002 euid=1001 suid=1001 fsuid=1001 egid=1002 sgid=1002 fsgid=1002 tty=pts1 ses=3 comm="io_uring_passth" exe="/mnt/src/liburing/test/io_uring_passthrough.t" subj=system_u:system_r:kernel_t key=(null)
>   [  136.632415] audit: type=1327 audit(1662043624.702:95): proctitle=2F6D6E742F7372632F6C69627572696E672F746573742F696F5F7572696E675F706173737468726F7567682E74002F6465762F6E67306E31
>   [  136.633652] audit: type=1400 audit(1662043624.705:96): avc:  denied  { cmd } for  pid=263 comm="io_uring_passth" path="/dev/ng0n1" dev="devtmpfs" ino=120 scontext=system_u:system_r:kernel_t tcontext=system_u:object_r:device_t tclass=io_uring permissive=1
>   [  136.635384] audit: type=1336 audit(1662043624.705:96): uring_op=46 items=0 ppid=252 pid=263 uid=1001 gid=1002 euid=1001 suid=1001 fsuid=1001 egid=1002 sgid=1002 fsgid=1002 subj=system_u:system_r:kernel_t key=(null)
>   [  136.636863] audit: type=1336 audit(1662043624.705:96): uring_op=46 items=0 ppid=252 pid=263 uid=1001 gid=1002 euid=1001 suid=1001 fsuid=1001 egid=1002 sgid=1002 fsgid=1002 subj=system_u:system_r:kernel_t key=(null)
>
> * From the output on time 136.633652 I see that the access should have
>   been denied had selinux been enforcing.
> * I also saw that the breakpoint hit.
>
> *** Without the patch:
> * I ran the io_uring_passthrough.c test on a char device with an
>   unpriviledged user.
> * I took care of changing the permissions of /dev/ng0n1 to 666 prior
>   to any testing.
> * made sure that Selinux was in permissive mode.
> * Made sure to have audit activated by passing "audit=1" to the kernel
> * After noticing that some audit messages where getting lost I upped the
>   backlog limit to 256
> * There were no audit messages when I executed the test.
>
> As with my smack tests I would really appreciate feecback on the
> approach I took to testing and it's validity.

Hi Joel,

Thanks for the additional testing and verification!  Work like this is
always welcome, regardless if the patch has already been merged
upstream.

As far as you test approach is concerned, I think you are on the right
track, I might suggest resolving the other SELinux/AVC denials you are
seeing with your test application to help reduce the noise in the
logs.  Are you familiar with the selinux-testsuite (link below)?

* https://github.com/SELinuxProject/selinux-testsuite

-- 
paul-moore.com

Re: [PATCH 2/3] selinux: implement the security_uring_cmd() LSM hook

[Joel Granados]

[-- Attachment #1: Type: text/plain, Size: 5778 bytes --]

Hey Paul

On Thu, Sep 01, 2022 at 05:30:38PM -0400, Paul Moore wrote:
> On Thu, Sep 1, 2022 at 4:15 PM Joel Granados <j.granados@samsung.com> wrote:
> > Hey Paul
> >
> > I realize that you have already sent this upstream but I wanted to share
> > the Selinux part of the testing that we did to see if there is any
> > feedback.
> >
> > With my tests I see that the selinux_uring_cmd hook is run and it
> > results in a "avc : denied" when I run it with selinux in permissive
> > mode with an unpriviledged user. I assume that this is the expected
> > behavior. Here is how I tested
> >
> > *** With the patch:
> > * I ran the io_uring_passthrough.c test on a char device with an
> >   unpriviledged user.
> > * I took care of changing the permissions of /dev/ng0n1 to 666 prior
> >   to any testing.
> > * made sure that Selinux was in permissive mode.
> > * Made sure to have audit activated by passing "audit=1" to the kernel
> > * After noticing that some audit messages where getting lost I upped the
> >   backlog limit to 256
> > * Prior to executing the test, I also placed a breakpoint inside
> >   selinux_uring_cmd to make sure that it was executed.
> > * This is the output of the audit when I executed the test:
> >
> >   [  136.615924] audit: type=1400 audit(1662043624.701:94): avc:  denied  { create } for  pid=263 comm="io_uring_passth" anonclass=[io_uring] scontext=system_u:system_r:kernel_t tcontext=system_u:object_r:kernel_t tclass=anon_inode permissive=1
> >   [  136.621036] audit: type=1300 audit(1662043624.701:94): arch=c000003e syscall=425 success=yes exit=3 a0=40 a1=7ffca29835a0 a2=7ffca29835a0 a3=561529be2300 items=0 ppid=252 pid=263 auid=1001 uid=1001 gid=1002 euid=1001 suid=1001 fsuid=1001 egid=1002 sgid=1002 fsgid=1002 tty=pts1 ses=3 comm="io_uring_passth" exe="/mnt/src/liburing/test/io_uring_passthrough.t" subj=system_u:system_r:kernel_t key=(null)
> >   [  136.624812] audit: type=1327 audit(1662043624.701:94): proctitle=2F6D6E742F7372632F6C69627572696E672F746573742F696F5F7572696E675F706173737468726F7567682E74002F6465762F6E67306E31
> >   [  136.626074] audit: type=1400 audit(1662043624.702:95): avc:  denied  { map } for  pid=263 comm="io_uring_passth" path="anon_inode:[io_uring]" dev="anon_inodefs" ino=11715 scontext=system_u:system_r:kernel_t tcontext=system_u:object_r:kernel_t tclass=anon_inode permissive=1
> >   [  136.628012] audit: type=1400 audit(1662043624.702:95): avc:  denied  { read write } for  pid=263 comm="io_uring_passth" path="anon_inode:[io_uring]" dev="anon_inodefs" ino=11715 scontext=system_u:system_r:kernel_t tcontext=system_u:object_r:kernel_t tclass=anon_inode permissive=1
> >   [  136.629873] audit: type=1300 audit(1662043624.702:95): arch=c000003e syscall=9 success=yes exit=140179765297152 a0=0 a1=1380 a2=3 a3=8001 items=0 ppid=252 pid=263 auid=1001 uid=1001 gid=1002 euid=1001 suid=1001 fsuid=1001 egid=1002 sgid=1002 fsgid=1002 tty=pts1 ses=3 comm="io_uring_passth" exe="/mnt/src/liburing/test/io_uring_passthrough.t" subj=system_u:system_r:kernel_t key=(null)
> >   [  136.632415] audit: type=1327 audit(1662043624.702:95): proctitle=2F6D6E742F7372632F6C69627572696E672F746573742F696F5F7572696E675F706173737468726F7567682E74002F6465762F6E67306E31
> >   [  136.633652] audit: type=1400 audit(1662043624.705:96): avc:  denied  { cmd } for  pid=263 comm="io_uring_passth" path="/dev/ng0n1" dev="devtmpfs" ino=120 scontext=system_u:system_r:kernel_t tcontext=system_u:object_r:device_t tclass=io_uring permissive=1
> >   [  136.635384] audit: type=1336 audit(1662043624.705:96): uring_op=46 items=0 ppid=252 pid=263 uid=1001 gid=1002 euid=1001 suid=1001 fsuid=1001 egid=1002 sgid=1002 fsgid=1002 subj=system_u:system_r:kernel_t key=(null)
> >   [  136.636863] audit: type=1336 audit(1662043624.705:96): uring_op=46 items=0 ppid=252 pid=263 uid=1001 gid=1002 euid=1001 suid=1001 fsuid=1001 egid=1002 sgid=1002 fsgid=1002 subj=system_u:system_r:kernel_t key=(null)
> >
> > * From the output on time 136.633652 I see that the access should have
> >   been denied had selinux been enforcing.
> > * I also saw that the breakpoint hit.
> >
> > *** Without the patch:
> > * I ran the io_uring_passthrough.c test on a char device with an
> >   unpriviledged user.
> > * I took care of changing the permissions of /dev/ng0n1 to 666 prior
> >   to any testing.
> > * made sure that Selinux was in permissive mode.
> > * Made sure to have audit activated by passing "audit=1" to the kernel
> > * After noticing that some audit messages where getting lost I upped the
> >   backlog limit to 256
> > * There were no audit messages when I executed the test.
> >
> > As with my smack tests I would really appreciate feecback on the
> > approach I took to testing and it's validity.
> 
> Hi Joel,
> 
> Thanks for the additional testing and verification!  Work like this is
> always welcome, regardless if the patch has already been merged
> upstream.
np

> 
> As far as you test approach is concerned, I think you are on the right
> track, I might suggest resolving the other SELinux/AVC denials you are
> seeing with your test application to help reduce the noise in the
> logs.  Are you familiar with the selinux-testsuite (link below)?
> 
> * https://protect2.fireeye.com/v1/url?k=6f356c96-0ebe79ac-6f34e7d9-74fe4860008a-01002a6e4c92bb3e&q=1&e=46f33488-9311-49fa-9747-da210f2d147d&u=https%3A%2F%2Fgithub.com%2FSELinuxProject%2Fselinux-testsuite
Thx. Could not figure out how to remove the AVC from a quick look at the
page, but I'll probably figures something out :).

ATM, I'm doing a performance test on the io_uring_passtrhough
path to see how much, if any, perf we loose.

Thx again

Best

Joel

> 
> -- 
> paul-moore.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH 2/3] selinux: implement the security_uring_cmd() LSM hook

[Joel Granados]

[-- Attachment #1: Type: text/plain, Size: 7865 bytes --]

Hello.

Took the time to re-run my performance tests on the current LSM patch
for io_uring_cmd. I'll explain what I did and then give the results:

How I ran it:
I took a version of the kernel with the patch a0d2212773d1 and then
compiled two versions: The first was the vanilla kernel and the other
was the same except for the LSM hook called from io_uring_cmd removed.
Same kernel configurations. For my tests I used one of the test files
from FIO called t/io_uring.c which is basically a READ test. I ran my
tests on both an nvme device and the null device (/dev/null). For the
first I did not change io_uring.c and for the second I replaced the
admin calls with dummy data that was not really needed for testing with
/dev/null. These are the arguments I used for the test
"./t/io_uring -b4096 -d128 -c32 -s32 -p0 -F1 -B0 -O0 -P1 -u1 -n1"
Finally, I'm taking the max of several samples.

Results:
+-------------------------------------+------------------------+-----------------------+
|                Name                 | /dev/ng0n1 (BW: MiB/s) | /dev/null (BW: GiB/s) |
+-------------------------------------+------------------------+-----------------------+
| (A) for-next (vanilla)              |                   1341 |                 30.16 |
| (B) for-next (no io_uring_cmd hook) |                   1362 |                 40.61 |
| [1-(A/B)] * 100                     |             1.54185022 |          25.732578183 |
+-------------------------------------+------------------------+-----------------------+

So on a device (dev/ng0n1) there is a 1% performance difference on a
read. Whereas on the null device (dev/null) there is a 25% difference on
a read.

This difference is interesting and expected as there is a lot more stuff
happening when we go through the actual device.

Best

Joel

On Wed, Sep 07, 2022 at 10:17:29AM +0200, Joel Granados wrote:
> Hey Paul
> 
> On Thu, Sep 01, 2022 at 05:30:38PM -0400, Paul Moore wrote:
> > On Thu, Sep 1, 2022 at 4:15 PM Joel Granados <j.granados@samsung.com> wrote:
> > > Hey Paul
> > >
> > > I realize that you have already sent this upstream but I wanted to share
> > > the Selinux part of the testing that we did to see if there is any
> > > feedback.
> > >
> > > With my tests I see that the selinux_uring_cmd hook is run and it
> > > results in a "avc : denied" when I run it with selinux in permissive
> > > mode with an unpriviledged user. I assume that this is the expected
> > > behavior. Here is how I tested
> > >
> > > *** With the patch:
> > > * I ran the io_uring_passthrough.c test on a char device with an
> > >   unpriviledged user.
> > > * I took care of changing the permissions of /dev/ng0n1 to 666 prior
> > >   to any testing.
> > > * made sure that Selinux was in permissive mode.
> > > * Made sure to have audit activated by passing "audit=1" to the kernel
> > > * After noticing that some audit messages where getting lost I upped the
> > >   backlog limit to 256
> > > * Prior to executing the test, I also placed a breakpoint inside
> > >   selinux_uring_cmd to make sure that it was executed.
> > > * This is the output of the audit when I executed the test:
> > >
> > >   [  136.615924] audit: type=1400 audit(1662043624.701:94): avc:  denied  { create } for  pid=263 comm="io_uring_passth" anonclass=[io_uring] scontext=system_u:system_r:kernel_t tcontext=system_u:object_r:kernel_t tclass=anon_inode permissive=1
> > >   [  136.621036] audit: type=1300 audit(1662043624.701:94): arch=c000003e syscall=425 success=yes exit=3 a0=40 a1=7ffca29835a0 a2=7ffca29835a0 a3=561529be2300 items=0 ppid=252 pid=263 auid=1001 uid=1001 gid=1002 euid=1001 suid=1001 fsuid=1001 egid=1002 sgid=1002 fsgid=1002 tty=pts1 ses=3 comm="io_uring_passth" exe="/mnt/src/liburing/test/io_uring_passthrough.t" subj=system_u:system_r:kernel_t key=(null)
> > >   [  136.624812] audit: type=1327 audit(1662043624.701:94): proctitle=2F6D6E742F7372632F6C69627572696E672F746573742F696F5F7572696E675F706173737468726F7567682E74002F6465762F6E67306E31
> > >   [  136.626074] audit: type=1400 audit(1662043624.702:95): avc:  denied  { map } for  pid=263 comm="io_uring_passth" path="anon_inode:[io_uring]" dev="anon_inodefs" ino=11715 scontext=system_u:system_r:kernel_t tcontext=system_u:object_r:kernel_t tclass=anon_inode permissive=1
> > >   [  136.628012] audit: type=1400 audit(1662043624.702:95): avc:  denied  { read write } for  pid=263 comm="io_uring_passth" path="anon_inode:[io_uring]" dev="anon_inodefs" ino=11715 scontext=system_u:system_r:kernel_t tcontext=system_u:object_r:kernel_t tclass=anon_inode permissive=1
> > >   [  136.629873] audit: type=1300 audit(1662043624.702:95): arch=c000003e syscall=9 success=yes exit=140179765297152 a0=0 a1=1380 a2=3 a3=8001 items=0 ppid=252 pid=263 auid=1001 uid=1001 gid=1002 euid=1001 suid=1001 fsuid=1001 egid=1002 sgid=1002 fsgid=1002 tty=pts1 ses=3 comm="io_uring_passth" exe="/mnt/src/liburing/test/io_uring_passthrough.t" subj=system_u:system_r:kernel_t key=(null)
> > >   [  136.632415] audit: type=1327 audit(1662043624.702:95): proctitle=2F6D6E742F7372632F6C69627572696E672F746573742F696F5F7572696E675F706173737468726F7567682E74002F6465762F6E67306E31
> > >   [  136.633652] audit: type=1400 audit(1662043624.705:96): avc:  denied  { cmd } for  pid=263 comm="io_uring_passth" path="/dev/ng0n1" dev="devtmpfs" ino=120 scontext=system_u:system_r:kernel_t tcontext=system_u:object_r:device_t tclass=io_uring permissive=1
> > >   [  136.635384] audit: type=1336 audit(1662043624.705:96): uring_op=46 items=0 ppid=252 pid=263 uid=1001 gid=1002 euid=1001 suid=1001 fsuid=1001 egid=1002 sgid=1002 fsgid=1002 subj=system_u:system_r:kernel_t key=(null)
> > >   [  136.636863] audit: type=1336 audit(1662043624.705:96): uring_op=46 items=0 ppid=252 pid=263 uid=1001 gid=1002 euid=1001 suid=1001 fsuid=1001 egid=1002 sgid=1002 fsgid=1002 subj=system_u:system_r:kernel_t key=(null)
> > >
> > > * From the output on time 136.633652 I see that the access should have
> > >   been denied had selinux been enforcing.
> > > * I also saw that the breakpoint hit.
> > >
> > > *** Without the patch:
> > > * I ran the io_uring_passthrough.c test on a char device with an
> > >   unpriviledged user.
> > > * I took care of changing the permissions of /dev/ng0n1 to 666 prior
> > >   to any testing.
> > > * made sure that Selinux was in permissive mode.
> > > * Made sure to have audit activated by passing "audit=1" to the kernel
> > > * After noticing that some audit messages where getting lost I upped the
> > >   backlog limit to 256
> > > * There were no audit messages when I executed the test.
> > >
> > > As with my smack tests I would really appreciate feecback on the
> > > approach I took to testing and it's validity.
> > 
> > Hi Joel,
> > 
> > Thanks for the additional testing and verification!  Work like this is
> > always welcome, regardless if the patch has already been merged
> > upstream.
> np
> 
> > 
> > As far as you test approach is concerned, I think you are on the right
> > track, I might suggest resolving the other SELinux/AVC denials you are
> > seeing with your test application to help reduce the noise in the
> > logs.  Are you familiar with the selinux-testsuite (link below)?
> > 
> > * https://protect2.fireeye.com/v1/url?k=6f356c96-0ebe79ac-6f34e7d9-74fe4860008a-01002a6e4c92bb3e&q=1&e=46f33488-9311-49fa-9747-da210f2d147d&u=https%3A%2F%2Fgithub.com%2FSELinuxProject%2Fselinux-testsuite
> Thx. Could not figure out how to remove the AVC from a quick look at the
> page, but I'll probably figures something out :).
> 
> ATM, I'm doing a performance test on the io_uring_passtrhough
> path to see how much, if any, perf we loose.
> 
> Thx again
> 
> Best
> 
> Joel
> 
> > 
> > -- 
> > paul-moore.com



[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

[PATCH 3/3] /dev/null: add IORING_OP_URING_CMD support

[Paul Moore]

This patch adds support for the io_uring command pass through, aka
IORING_OP_URING_CMD, to the /dev/null driver.  As with all of the
/dev/null functionality, the implementation is just a simple sink
where commands go to die, but it should be useful for developers who
need a simple IORING_OP_URING_CMD test device that doesn't require
any special hardware.

Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Paul Moore <paul@paul-moore.com>
---
 drivers/char/mem.c |    6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/char/mem.c b/drivers/char/mem.c
index 84ca98ed1dad..32a932a065a6 100644
--- a/drivers/char/mem.c
+++ b/drivers/char/mem.c
@@ -480,6 +480,11 @@ static ssize_t splice_write_null(struct pipe_inode_info *pipe, struct file *out,
 	return splice_from_pipe(pipe, out, ppos, len, flags, pipe_to_null);
 }
 
+static int uring_cmd_null(struct io_uring_cmd *ioucmd, unsigned int issue_flags)
+{
+	return 0;
+}
+
 static ssize_t read_iter_zero(struct kiocb *iocb, struct iov_iter *iter)
 {
 	size_t written = 0;
@@ -663,6 +668,7 @@ static const struct file_operations null_fops = {
 	.read_iter	= read_iter_null,
 	.write_iter	= write_iter_null,
 	.splice_write	= splice_write_null,
+	.uring_cmd	= uring_cmd_null,
 };
 
 static const struct file_operations __maybe_unused port_fops = {

Re: [PATCH 3/3] /dev/null: add IORING_OP_URING_CMD support

[Jens Axboe]

On 8/22/22 3:21 PM, Paul Moore wrote:
> This patch adds support for the io_uring command pass through, aka
> IORING_OP_URING_CMD, to the /dev/null driver.  As with all of the
> /dev/null functionality, the implementation is just a simple sink
> where commands go to die, but it should be useful for developers who
> need a simple IORING_OP_URING_CMD test device that doesn't require
> any special hardware.
> 
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Signed-off-by: Paul Moore <paul@paul-moore.com>
> ---
>  drivers/char/mem.c |    6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/char/mem.c b/drivers/char/mem.c
> index 84ca98ed1dad..32a932a065a6 100644
> --- a/drivers/char/mem.c
> +++ b/drivers/char/mem.c
> @@ -480,6 +480,11 @@ static ssize_t splice_write_null(struct pipe_inode_info *pipe, struct file *out,
>  	return splice_from_pipe(pipe, out, ppos, len, flags, pipe_to_null);
>  }
>  
> +static int uring_cmd_null(struct io_uring_cmd *ioucmd, unsigned int issue_flags)
> +{
> +	return 0;
> +}

This would be better as:

	return IOU_OK;

using the proper return values for the uring_cmd hook. With that:

Acked-by: Jens Axboe <axboe@kernel.dk>

-- 
Jens Axboe

Re: [PATCH 3/3] /dev/null: add IORING_OP_URING_CMD support

[Paul Moore]

On Mon, Aug 22, 2022 at 6:36 PM Jens Axboe <axboe@kernel.dk> wrote:
> On 8/22/22 3:21 PM, Paul Moore wrote:
> > This patch adds support for the io_uring command pass through, aka
> > IORING_OP_URING_CMD, to the /dev/null driver.  As with all of the
> > /dev/null functionality, the implementation is just a simple sink
> > where commands go to die, but it should be useful for developers who
> > need a simple IORING_OP_URING_CMD test device that doesn't require
> > any special hardware.
> >
> > Cc: Arnd Bergmann <arnd@arndb.de>
> > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > Signed-off-by: Paul Moore <paul@paul-moore.com>
> > ---
> >  drivers/char/mem.c |    6 ++++++
> >  1 file changed, 6 insertions(+)
> >
> > diff --git a/drivers/char/mem.c b/drivers/char/mem.c
> > index 84ca98ed1dad..32a932a065a6 100644
> > --- a/drivers/char/mem.c
> > +++ b/drivers/char/mem.c
> > @@ -480,6 +480,11 @@ static ssize_t splice_write_null(struct pipe_inode_info *pipe, struct file *out,
> >       return splice_from_pipe(pipe, out, ppos, len, flags, pipe_to_null);
> >  }
> >
> > +static int uring_cmd_null(struct io_uring_cmd *ioucmd, unsigned int issue_flags)
> > +{
> > +     return 0;
> > +}
>
> This would be better as:
>
>         return IOU_OK;
>
> using the proper return values for the uring_cmd hook.

The only problem I see with that is that IOU_OK is defined under
io_uring/io_uring.h and not include/linux/io_uring.h so the #include
macro is kinda ugly:

  #include "../../io_uring/io_uring.h"

I'm not sure I want to submit that upstream looking like that.  Are
you okay with leaving the return code as 0 for now and changing it at
a later date?  I'm trying to keep this patchset relatively small since
we are in the -rcX stage, but if you're okay with a simple cut-n-paste
of the enum to linux/io_uring.h I can do that.

> With that:
>
> Acked-by: Jens Axboe <axboe@kernel.dk>

-- 
paul-moore.com

Re: [PATCH 3/3] /dev/null: add IORING_OP_URING_CMD support

[Jens Axboe]

On 8/22/22 5:09 PM, Paul Moore wrote:
> On Mon, Aug 22, 2022 at 6:36 PM Jens Axboe <axboe@kernel.dk> wrote:
>> On 8/22/22 3:21 PM, Paul Moore wrote:
>>> This patch adds support for the io_uring command pass through, aka
>>> IORING_OP_URING_CMD, to the /dev/null driver.  As with all of the
>>> /dev/null functionality, the implementation is just a simple sink
>>> where commands go to die, but it should be useful for developers who
>>> need a simple IORING_OP_URING_CMD test device that doesn't require
>>> any special hardware.
>>>
>>> Cc: Arnd Bergmann <arnd@arndb.de>
>>> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>>> Signed-off-by: Paul Moore <paul@paul-moore.com>
>>> ---
>>>  drivers/char/mem.c |    6 ++++++
>>>  1 file changed, 6 insertions(+)
>>>
>>> diff --git a/drivers/char/mem.c b/drivers/char/mem.c
>>> index 84ca98ed1dad..32a932a065a6 100644
>>> --- a/drivers/char/mem.c
>>> +++ b/drivers/char/mem.c
>>> @@ -480,6 +480,11 @@ static ssize_t splice_write_null(struct pipe_inode_info *pipe, struct file *out,
>>>       return splice_from_pipe(pipe, out, ppos, len, flags, pipe_to_null);
>>>  }
>>>
>>> +static int uring_cmd_null(struct io_uring_cmd *ioucmd, unsigned int issue_flags)
>>> +{
>>> +     return 0;
>>> +}
>>
>> This would be better as:
>>
>>         return IOU_OK;
>>
>> using the proper return values for the uring_cmd hook.
> 
> The only problem I see with that is that IOU_OK is defined under
> io_uring/io_uring.h and not include/linux/io_uring.h so the #include
> macro is kinda ugly:
> 
>   #include "../../io_uring/io_uring.h"
> 
> I'm not sure I want to submit that upstream looking like that.  Are
> you okay with leaving the return code as 0 for now and changing it at
> a later date?  I'm trying to keep this patchset relatively small since
> we are in the -rcX stage, but if you're okay with a simple cut-n-paste
> of the enum to linux/io_uring.h I can do that.

Ugh yes, that should move into the general domain. Yeah I'm fine with it
as it is, we can fix that up (and them nvme as well) at a later point.

-- 
Jens Axboe

Re: [PATCH 3/3] /dev/null: add IORING_OP_URING_CMD support

[Paul Moore]

On Mon, Aug 22, 2022 at 7:13 PM Jens Axboe <axboe@kernel.dk> wrote:
> On 8/22/22 5:09 PM, Paul Moore wrote:
> > On Mon, Aug 22, 2022 at 6:36 PM Jens Axboe <axboe@kernel.dk> wrote:
> >> On 8/22/22 3:21 PM, Paul Moore wrote:
> >>> This patch adds support for the io_uring command pass through, aka
> >>> IORING_OP_URING_CMD, to the /dev/null driver.  As with all of the
> >>> /dev/null functionality, the implementation is just a simple sink
> >>> where commands go to die, but it should be useful for developers who
> >>> need a simple IORING_OP_URING_CMD test device that doesn't require
> >>> any special hardware.
> >>>
> >>> Cc: Arnd Bergmann <arnd@arndb.de>
> >>> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> >>> Signed-off-by: Paul Moore <paul@paul-moore.com>
> >>> ---
> >>>  drivers/char/mem.c |    6 ++++++
> >>>  1 file changed, 6 insertions(+)
> >>>
> >>> diff --git a/drivers/char/mem.c b/drivers/char/mem.c
> >>> index 84ca98ed1dad..32a932a065a6 100644
> >>> --- a/drivers/char/mem.c
> >>> +++ b/drivers/char/mem.c
> >>> @@ -480,6 +480,11 @@ static ssize_t splice_write_null(struct pipe_inode_info *pipe, struct file *out,
> >>>       return splice_from_pipe(pipe, out, ppos, len, flags, pipe_to_null);
> >>>  }
> >>>
> >>> +static int uring_cmd_null(struct io_uring_cmd *ioucmd, unsigned int issue_flags)
> >>> +{
> >>> +     return 0;
> >>> +}
> >>
> >> This would be better as:
> >>
> >>         return IOU_OK;
> >>
> >> using the proper return values for the uring_cmd hook.
> >
> > The only problem I see with that is that IOU_OK is defined under
> > io_uring/io_uring.h and not include/linux/io_uring.h so the #include
> > macro is kinda ugly:
> >
> >   #include "../../io_uring/io_uring.h"
> >
> > I'm not sure I want to submit that upstream looking like that.  Are
> > you okay with leaving the return code as 0 for now and changing it at
> > a later date?  I'm trying to keep this patchset relatively small since
> > we are in the -rcX stage, but if you're okay with a simple cut-n-paste
> > of the enum to linux/io_uring.h I can do that.
>
> Ugh yes, that should move into the general domain. Yeah I'm fine with it
> as it is, we can fix that up (and them nvme as well) at a later point.

Okay, sounds good, I'll leave it as-is.  Is it okay to still add your ACK?

-- 
paul-moore.com

Re: [PATCH 3/3] /dev/null: add IORING_OP_URING_CMD support

[Jens Axboe]

On 8/22/22 5:19 PM, Paul Moore wrote:
> On Mon, Aug 22, 2022 at 7:13 PM Jens Axboe <axboe@kernel.dk> wrote:
>> On 8/22/22 5:09 PM, Paul Moore wrote:
>>> On Mon, Aug 22, 2022 at 6:36 PM Jens Axboe <axboe@kernel.dk> wrote:
>>>> On 8/22/22 3:21 PM, Paul Moore wrote:
>>>>> This patch adds support for the io_uring command pass through, aka
>>>>> IORING_OP_URING_CMD, to the /dev/null driver.  As with all of the
>>>>> /dev/null functionality, the implementation is just a simple sink
>>>>> where commands go to die, but it should be useful for developers who
>>>>> need a simple IORING_OP_URING_CMD test device that doesn't require
>>>>> any special hardware.
>>>>>
>>>>> Cc: Arnd Bergmann <arnd@arndb.de>
>>>>> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>>>>> Signed-off-by: Paul Moore <paul@paul-moore.com>
>>>>> ---
>>>>>  drivers/char/mem.c |    6 ++++++
>>>>>  1 file changed, 6 insertions(+)
>>>>>
>>>>> diff --git a/drivers/char/mem.c b/drivers/char/mem.c
>>>>> index 84ca98ed1dad..32a932a065a6 100644
>>>>> --- a/drivers/char/mem.c
>>>>> +++ b/drivers/char/mem.c
>>>>> @@ -480,6 +480,11 @@ static ssize_t splice_write_null(struct pipe_inode_info *pipe, struct file *out,
>>>>>       return splice_from_pipe(pipe, out, ppos, len, flags, pipe_to_null);
>>>>>  }
>>>>>
>>>>> +static int uring_cmd_null(struct io_uring_cmd *ioucmd, unsigned int issue_flags)
>>>>> +{
>>>>> +     return 0;
>>>>> +}
>>>>
>>>> This would be better as:
>>>>
>>>>         return IOU_OK;
>>>>
>>>> using the proper return values for the uring_cmd hook.
>>>
>>> The only problem I see with that is that IOU_OK is defined under
>>> io_uring/io_uring.h and not include/linux/io_uring.h so the #include
>>> macro is kinda ugly:
>>>
>>>   #include "../../io_uring/io_uring.h"
>>>
>>> I'm not sure I want to submit that upstream looking like that.  Are
>>> you okay with leaving the return code as 0 for now and changing it at
>>> a later date?  I'm trying to keep this patchset relatively small since
>>> we are in the -rcX stage, but if you're okay with a simple cut-n-paste
>>> of the enum to linux/io_uring.h I can do that.
>>
>> Ugh yes, that should move into the general domain. Yeah I'm fine with it
>> as it is, we can fix that up (and them nvme as well) at a later point.
> 
> Okay, sounds good, I'll leave it as-is.  Is it okay to still add your ACK?

Yep, all things considered, for 6.0 I think that's the way to go.

-- 
Jens Axboe

Re: [PATCH 3/3] /dev/null: add IORING_OP_URING_CMD support

[Paul Moore]

On Mon, Aug 22, 2022 at 7:25 PM Jens Axboe <axboe@kernel.dk> wrote:
>
> On 8/22/22 5:19 PM, Paul Moore wrote:
> > On Mon, Aug 22, 2022 at 7:13 PM Jens Axboe <axboe@kernel.dk> wrote:
> >> On 8/22/22 5:09 PM, Paul Moore wrote:
> >>> On Mon, Aug 22, 2022 at 6:36 PM Jens Axboe <axboe@kernel.dk> wrote:
> >>>> On 8/22/22 3:21 PM, Paul Moore wrote:
> >>>>> This patch adds support for the io_uring command pass through, aka
> >>>>> IORING_OP_URING_CMD, to the /dev/null driver.  As with all of the
> >>>>> /dev/null functionality, the implementation is just a simple sink
> >>>>> where commands go to die, but it should be useful for developers who
> >>>>> need a simple IORING_OP_URING_CMD test device that doesn't require
> >>>>> any special hardware.
> >>>>>
> >>>>> Cc: Arnd Bergmann <arnd@arndb.de>
> >>>>> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> >>>>> Signed-off-by: Paul Moore <paul@paul-moore.com>
> >>>>> ---
> >>>>>  drivers/char/mem.c |    6 ++++++
> >>>>>  1 file changed, 6 insertions(+)
> >>>>>
> >>>>> diff --git a/drivers/char/mem.c b/drivers/char/mem.c
> >>>>> index 84ca98ed1dad..32a932a065a6 100644
> >>>>> --- a/drivers/char/mem.c
> >>>>> +++ b/drivers/char/mem.c
> >>>>> @@ -480,6 +480,11 @@ static ssize_t splice_write_null(struct pipe_inode_info *pipe, struct file *out,
> >>>>>       return splice_from_pipe(pipe, out, ppos, len, flags, pipe_to_null);
> >>>>>  }
> >>>>>
> >>>>> +static int uring_cmd_null(struct io_uring_cmd *ioucmd, unsigned int issue_flags)
> >>>>> +{
> >>>>> +     return 0;
> >>>>> +}
> >>>>
> >>>> This would be better as:
> >>>>
> >>>>         return IOU_OK;
> >>>>
> >>>> using the proper return values for the uring_cmd hook.
> >>>
> >>> The only problem I see with that is that IOU_OK is defined under
> >>> io_uring/io_uring.h and not include/linux/io_uring.h so the #include
> >>> macro is kinda ugly:
> >>>
> >>>   #include "../../io_uring/io_uring.h"
> >>>
> >>> I'm not sure I want to submit that upstream looking like that.  Are
> >>> you okay with leaving the return code as 0 for now and changing it at
> >>> a later date?  I'm trying to keep this patchset relatively small since
> >>> we are in the -rcX stage, but if you're okay with a simple cut-n-paste
> >>> of the enum to linux/io_uring.h I can do that.
> >>
> >> Ugh yes, that should move into the general domain. Yeah I'm fine with it
> >> as it is, we can fix that up (and them nvme as well) at a later point.
> >
> > Okay, sounds good, I'll leave it as-is.  Is it okay to still add your ACK?
>
> Yep, all things considered, for 6.0 I think that's the way to go.

Great, thanks.

-- 
paul-moore.com

Re: [PATCH 3/3] /dev/null: add IORING_OP_URING_CMD support

[Greg Kroah-Hartman]

On Mon, Aug 22, 2022 at 05:21:19PM -0400, Paul Moore wrote:
> This patch adds support for the io_uring command pass through, aka
> IORING_OP_URING_CMD, to the /dev/null driver.  As with all of the
> /dev/null functionality, the implementation is just a simple sink
> where commands go to die, but it should be useful for developers who
> need a simple IORING_OP_URING_CMD test device that doesn't require
> any special hardware.
> 
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Signed-off-by: Paul Moore <paul@paul-moore.com>
> ---
>  drivers/char/mem.c |    6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/char/mem.c b/drivers/char/mem.c
> index 84ca98ed1dad..32a932a065a6 100644
> --- a/drivers/char/mem.c
> +++ b/drivers/char/mem.c
> @@ -480,6 +480,11 @@ static ssize_t splice_write_null(struct pipe_inode_info *pipe, struct file *out,
>  	return splice_from_pipe(pipe, out, ppos, len, flags, pipe_to_null);
>  }
>  
> +static int uring_cmd_null(struct io_uring_cmd *ioucmd, unsigned int issue_flags)
> +{
> +	return 0;

If a callback just returns 0, that implies it is not needed at all and
can be removed and then you are back at the original file before your
commit :)

thanks,

greg k-h

Re: [PATCH 3/3] /dev/null: add IORING_OP_URING_CMD support

[Jens Axboe]

On 8/23/22 12:51 AM, Greg Kroah-Hartman wrote:
> On Mon, Aug 22, 2022 at 05:21:19PM -0400, Paul Moore wrote:
>> This patch adds support for the io_uring command pass through, aka
>> IORING_OP_URING_CMD, to the /dev/null driver.  As with all of the
>> /dev/null functionality, the implementation is just a simple sink
>> where commands go to die, but it should be useful for developers who
>> need a simple IORING_OP_URING_CMD test device that doesn't require
>> any special hardware.
>>
>> Cc: Arnd Bergmann <arnd@arndb.de>
>> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>> Signed-off-by: Paul Moore <paul@paul-moore.com>
>> ---
>>  drivers/char/mem.c |    6 ++++++
>>  1 file changed, 6 insertions(+)
>>
>> diff --git a/drivers/char/mem.c b/drivers/char/mem.c
>> index 84ca98ed1dad..32a932a065a6 100644
>> --- a/drivers/char/mem.c
>> +++ b/drivers/char/mem.c
>> @@ -480,6 +480,11 @@ static ssize_t splice_write_null(struct pipe_inode_info *pipe, struct file *out,
>>  	return splice_from_pipe(pipe, out, ppos, len, flags, pipe_to_null);
>>  }
>>  
>> +static int uring_cmd_null(struct io_uring_cmd *ioucmd, unsigned int issue_flags)
>> +{
>> +	return 0;
> 
> If a callback just returns 0, that implies it is not needed at all and
> can be removed and then you are back at the original file before your
> commit :)

In theory you are correct, but the empty hook is needed so that
submitting an io_uring cmd to the file type is attempted. If not it's
just errored upfront.

Paul, is it strictly needed to test the selinux uring cmd policy? If the
operation would've been attempted but null doesn't support it, you'd get
-1/EOPNOTSUPP - and supposedly you'd get EACCES/EPERM or something if
it's filtered?

-- 
Jens Axboe

Re: [PATCH 3/3] /dev/null: add IORING_OP_URING_CMD support

[Paul Moore]

On Tue, Aug 23, 2022 at 9:33 AM Jens Axboe <axboe@kernel.dk> wrote:
> On 8/23/22 12:51 AM, Greg Kroah-Hartman wrote:
> > On Mon, Aug 22, 2022 at 05:21:19PM -0400, Paul Moore wrote:
> >> This patch adds support for the io_uring command pass through, aka
> >> IORING_OP_URING_CMD, to the /dev/null driver.  As with all of the
> >> /dev/null functionality, the implementation is just a simple sink
> >> where commands go to die, but it should be useful for developers who
> >> need a simple IORING_OP_URING_CMD test device that doesn't require
> >> any special hardware.
> >>
> >> Cc: Arnd Bergmann <arnd@arndb.de>
> >> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> >> Signed-off-by: Paul Moore <paul@paul-moore.com>
> >> ---
> >>  drivers/char/mem.c |    6 ++++++
> >>  1 file changed, 6 insertions(+)
> >>
> >> diff --git a/drivers/char/mem.c b/drivers/char/mem.c
> >> index 84ca98ed1dad..32a932a065a6 100644
> >> --- a/drivers/char/mem.c
> >> +++ b/drivers/char/mem.c
> >> @@ -480,6 +480,11 @@ static ssize_t splice_write_null(struct pipe_inode_info *pipe, struct file *out,
> >>      return splice_from_pipe(pipe, out, ppos, len, flags, pipe_to_null);
> >>  }
> >>
> >> +static int uring_cmd_null(struct io_uring_cmd *ioucmd, unsigned int issue_flags)
> >> +{
> >> +    return 0;
> >
> > If a callback just returns 0, that implies it is not needed at all and
> > can be removed and then you are back at the original file before your
> > commit :)
>
> In theory you are correct, but the empty hook is needed so that
> submitting an io_uring cmd to the file type is attempted. If not it's
> just errored upfront.
>
> Paul, is it strictly needed to test the selinux uring cmd policy? If the
> operation would've been attempted but null doesn't support it, you'd get
> -1/EOPNOTSUPP - and supposedly you'd get EACCES/EPERM or something if
> it's filtered?

I haven't built a kernel without this patch to test, but yes, you are
probably correct that it wouldn't be strictly necessary, but
considering the extreme simplicity of this addition, what is the real
harm here?  Wouldn't it be nice to have a rather simple
IORING_OP_URING_CMD target?

Also, just so we are clear, I didn't mark this patch with the
stable/fixes tags because I don't believe this should go into -stable;
while I believe it is a nice addition, it is definitely not critical.

-- 
paul-moore.com

Re: [PATCH 3/3] /dev/null: add IORING_OP_URING_CMD support

[Greg Kroah-Hartman]

On Mon, Aug 22, 2022 at 05:21:19PM -0400, Paul Moore wrote:
> This patch adds support for the io_uring command pass through, aka
> IORING_OP_URING_CMD, to the /dev/null driver.  As with all of the
> /dev/null functionality, the implementation is just a simple sink
> where commands go to die, but it should be useful for developers who
> need a simple IORING_OP_URING_CMD test device that doesn't require
> any special hardware.

Also, shouldn't you document this somewhere?

At least in the code itself saying "this is here so that /dev/null works
as a io_uring sink" or something like that?  Otherwise it just looks
like it does nothing at all.

thanks,

greg k-h

Re: [PATCH 3/3] /dev/null: add IORING_OP_URING_CMD support

[Paul Moore]

On Tue, Aug 23, 2022 at 2:52 AM Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
> On Mon, Aug 22, 2022 at 05:21:19PM -0400, Paul Moore wrote:
> > This patch adds support for the io_uring command pass through, aka
> > IORING_OP_URING_CMD, to the /dev/null driver.  As with all of the
> > /dev/null functionality, the implementation is just a simple sink
> > where commands go to die, but it should be useful for developers who
> > need a simple IORING_OP_URING_CMD test device that doesn't require
> > any special hardware.
>
> Also, shouldn't you document this somewhere?
>
> At least in the code itself saying "this is here so that /dev/null works
> as a io_uring sink" or something like that?  Otherwise it just looks
> like it does nothing at all.

What about read_null() and write_null()?  I can definitely add a
comment (there is no /dev/null documentation in the kernel source tree
that I can see), but there is clearly precedence for /dev/null having
"do nothing" file_operations functions.  If nothing else, it's pretty
much in the *name*; we're adding the "uring_cmd_null()" member
function to the "null_fops" struct for the "null" device ... come on
Greg :)

-- 
paul-moore.com

Re: [PATCH 3/3] /dev/null: add IORING_OP_URING_CMD support

[Greg Kroah-Hartman]

On Tue, Aug 23, 2022 at 01:02:08PM -0400, Paul Moore wrote:
> On Tue, Aug 23, 2022 at 2:52 AM Greg Kroah-Hartman
> <gregkh@linuxfoundation.org> wrote:
> > On Mon, Aug 22, 2022 at 05:21:19PM -0400, Paul Moore wrote:
> > > This patch adds support for the io_uring command pass through, aka
> > > IORING_OP_URING_CMD, to the /dev/null driver.  As with all of the
> > > /dev/null functionality, the implementation is just a simple sink
> > > where commands go to die, but it should be useful for developers who
> > > need a simple IORING_OP_URING_CMD test device that doesn't require
> > > any special hardware.
> >
> > Also, shouldn't you document this somewhere?
> >
> > At least in the code itself saying "this is here so that /dev/null works
> > as a io_uring sink" or something like that?  Otherwise it just looks
> > like it does nothing at all.
> 
> What about read_null() and write_null()?  I can definitely add a
> comment (there is no /dev/null documentation in the kernel source tree
> that I can see), but there is clearly precedence for /dev/null having
> "do nothing" file_operations functions.

Yes, they should "do nothing".  write_null() does report that it
consumed everything, why doesn't this function have to also do that?

thanks,

greg k-h

Re: [PATCH 3/3] /dev/null: add IORING_OP_URING_CMD support

[Paul Moore]

On Wed, Aug 24, 2022 at 2:10 AM Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
> On Tue, Aug 23, 2022 at 01:02:08PM -0400, Paul Moore wrote:
> > On Tue, Aug 23, 2022 at 2:52 AM Greg Kroah-Hartman
> > <gregkh@linuxfoundation.org> wrote:
> > > On Mon, Aug 22, 2022 at 05:21:19PM -0400, Paul Moore wrote:
> > > > This patch adds support for the io_uring command pass through, aka
> > > > IORING_OP_URING_CMD, to the /dev/null driver.  As with all of the
> > > > /dev/null functionality, the implementation is just a simple sink
> > > > where commands go to die, but it should be useful for developers who
> > > > need a simple IORING_OP_URING_CMD test device that doesn't require
> > > > any special hardware.
> > >
> > > Also, shouldn't you document this somewhere?
> > >
> > > At least in the code itself saying "this is here so that /dev/null works
> > > as a io_uring sink" or something like that?  Otherwise it just looks
> > > like it does nothing at all.
> >
> > What about read_null() and write_null()?  I can definitely add a
> > comment (there is no /dev/null documentation in the kernel source tree
> > that I can see), but there is clearly precedence for /dev/null having
> > "do nothing" file_operations functions.
>
> Yes, they should "do nothing".

Right, I don't think anyone was disputing that.  You were asking for a
comment for the new function that effectively says "this function does
nothing", which seems a little silly given the simplicity of the
function, the name, and the context of it all.

> write_null() does report that it
> consumed everything, why doesn't this function have to also do that?

Because a file write (file_operations->write) and a
IORING_OP_URING_CMD (file_operations->uring_cmd) are fundamentally
different operations; uring_cmd_null() returns 0, which is the success
return code for this file op (not to mention a significant number of
kernel functions that return an int).

-- 
paul-moore.com

Re: [PATCH 0/3] LSM hooks for IORING_OP_URING_CMD

[Paul Moore]

On Mon, Aug 22, 2022 at 5:21 PM Paul Moore <paul@paul-moore.com> wrote:
>
> This patchset includes three patches: one to add a new LSM hook for
> the IORING_OP_URING_CMD operation, one to add the SELinux
> implementation for the new hook, and one to enable
> IORING_OP_URING_CMD for /dev/null.  The last patch, the /dev/null
> support, is obviously not critical but it makes testing so much
> easier and I believe is in keeping with the general motivation behind
> /dev/null.
>
> Luis' patch has already been vetted by Jens and the io_uring folks,
> so the only new bits are the SELinux implementation and the trivial
> /dev/null implementation of IORING_OP_URING_CMD.  Assuming no one
> has any objections over the next few days, I'll plan on sending this
> up to Linus during the v6.0-rcX cycle.
>
> I believe Casey is also currently working on Smack support for the
> IORING_OP_URING_CMD hook, and as soon as he is ready I can add it
> to this patchset (or Casey can send it up himself).
>
> -Paul
>
> ---
>
> Luis Chamberlain (1):
>       lsm,io_uring: add LSM hooks for the new uring_cmd file op
>
> Paul Moore (2):
>       selinux: implement the security_uring_cmd() LSM hook
>       /dev/null: add IORING_OP_URING_CMD support
>
>
>  drivers/char/mem.c                  |  6 ++++++
>  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 ++++
>  security/selinux/hooks.c            | 24 ++++++++++++++++++++++++
>  security/selinux/include/classmap.h |  2 +-
>  8 files changed, 49 insertions(+), 1 deletion(-)

FYI, I just merged this into lsm/stable-6.0 and once the automated
testing completes and we sort out the Smack patch I'll send this up to
Linus.

-- 
paul-moore.com

[PATCH 0/3] LSM hooks for IORING_OP_URING_CMD

[Paul Moore]

This patchset includes three patches: one to add a new LSM hook for
the IORING_OP_URING_CMD operation, one to add the SELinux
implementation for the new hook, and one to enable
IORING_OP_URING_CMD for /dev/null.  The last patch, the /dev/null
support, is obviously not critical but it makes testing so much
easier and I believe is in keeping with the general motivation behind
/dev/null.

Luis' patch has already been vetted by Jens and the io_uring folks,
so the only new bits are the SELinux implementation and the trivial
/dev/null implementation of IORING_OP_URING_CMD.  Assuming no one
has any objections over the next few days, I'll plan on sending this
up to Linus during the v6.0-rcX cycle.

I believe Casey is also currently working on Smack support for the
IORING_OP_URING_CMD hook, and as soon as he is ready I can add it
to this patchset (or Casey can send it up himself).

-Paul

---

Luis Chamberlain (1):
      lsm,io_uring: add LSM hooks for the new uring_cmd file op

Paul Moore (2):
      /dev/null: add IORING_OP_URING_CMD support
      selinux: implement the security_uring_cmd() LSM hook


 drivers/char/mem.c                  |  6 ++++++
 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 ++++
 security/selinux/hooks.c            | 24 ++++++++++++++++++++++++
 security/selinux/include/classmap.h |  2 +-
 8 files changed, 49 insertions(+), 1 deletion(-)

Re: [PATCH 0/3] LSM hooks for IORING_OP_URING_CMD

[Paul Moore]

On Mon, Aug 22, 2022 at 5:14 PM Paul Moore <paul@paul-moore.com> wrote:
>
> This patchset includes three patches: one to add a new LSM hook for
> the IORING_OP_URING_CMD operation, one to add the SELinux
> implementation for the new hook, and one to enable
> IORING_OP_URING_CMD for /dev/null.  The last patch, the /dev/null
> support, is obviously not critical but it makes testing so much
> easier and I believe is in keeping with the general motivation behind
> /dev/null.
>
> Luis' patch has already been vetted by Jens and the io_uring folks,
> so the only new bits are the SELinux implementation and the trivial
> /dev/null implementation of IORING_OP_URING_CMD.  Assuming no one
> has any objections over the next few days, I'll plan on sending this
> up to Linus during the v6.0-rcX cycle.
>
> I believe Casey is also currently working on Smack support for the
> IORING_OP_URING_CMD hook, and as soon as he is ready I can add it
> to this patchset (or Casey can send it up himself).
>
> -Paul

Forgive me, I spaced and hit send on this patchset posting a *second*
too soon and didn't cancel it in time so the cover letter leaked out.
A proper patchset posting will be coming in just another minute ...

-- 
paul-moore.com