public inbox for [email protected]
 help / color / mirror / Atom feed
From: Joel Granados <[email protected]>
To: Kanchan Joshi <[email protected]>
Cc: <[email protected]>, <[email protected]>, <[email protected]>,
	<[email protected]>,
	<[email protected]>
Subject: Re: [RFC 1/1] Use ioctl selinux callback io_uring commands that implement the ioctl op convention
Date: Thu, 17 Nov 2022 10:25:46 +0100	[thread overview]
Message-ID: <20221117092546.hjzbt52pprztt6dh@localhost> (raw)
In-Reply-To: <20221116173821.GC5094@test-zns>

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

On Wed, Nov 16, 2022 at 11:08:21PM +0530, Kanchan Joshi wrote:
> On Wed, Nov 16, 2022 at 01:50:51PM +0100, Joel Granados wrote:
> > Signed-off-by: Joel Granados <[email protected]>
> > ---
> > security/selinux/hooks.c | 15 +++++++++++++--
> > 1 file changed, 13 insertions(+), 2 deletions(-)
> > 
> > diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> > index f553c370397e..a3f37ae5a980 100644
> > --- a/security/selinux/hooks.c
> > +++ b/security/selinux/hooks.c
> > @@ -21,6 +21,7 @@
> >  *  Copyright (C) 2016 Mellanox Technologies
> >  */
> > 
> > +#include "linux/nvme_ioctl.h"
> > #include <linux/init.h>
> > #include <linux/kd.h>
> > #include <linux/kernel.h>
> > @@ -7005,12 +7006,22 @@ static int selinux_uring_cmd(struct io_uring_cmd *ioucmd)
> > 	struct inode *inode = file_inode(file);
> > 	struct inode_security_struct *isec = selinux_inode(inode);
> > 	struct common_audit_data ad;
> > +	const struct cred *cred = current_cred();
> > 
> > 	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);
> > +	switch (ioucmd->cmd_op) {
> > +	case NVME_URING_CMD_IO:
> > +	case NVME_URING_CMD_IO_VEC:
> > +	case NVME_URING_CMD_ADMIN:
> > +	case NVME_URING_CMD_ADMIN_VEC:
> 
> We do not have to spell out these opcodes here.
> How about this instead:
> 
> +       /*
> +        * nvme uring-cmd continue to follow the ioctl format, so reuse what
> +        * we do for ioctl.
> +        */
> +       if(_IOC_TYPE(ioucmd->cmd_op) == 'N')
> +               return ioctl_has_perm(cred, file, FILE__IOCTL, (u16) ioucmd->cmd_op);
> +       else
> +               return avc_has_perm(&selinux_state, current_sid(), isec->sid,
> +                                   SECCLASS_IO_URING, IO_URING__CMD, &ad);
> +       }
> +
> 
> Now, if we write the above fragment this way -
> 
> if (__IOC_TYPE(ioucmd->cmd_op) != 0)
> 	reuse_what_is_done_for_ioctl;
> else
> 	current_check;

This is even cleaner. I really like this solution.
> 
> That will be bit more generic and can support more opcodes than nvme.
> ublk will continue to fall into else case, but something else (of
> future) may go into the if-part and be as fine-granular as ioctl hook
> has been.
I also see that this is the case. Since the io_uring command does not
have a predefined structure another solution for the non ioctl io_uring
commands needs to be found.

> Although we defined new nvme opcodes to be used with uring-cmd, it is
> also possible that some other provider decides to work with existing
> ioctl-opcode packaged inside uring-cmd and turns it async. It's just
> another implmentation choice.
> 
> Not so nice with the above could be that driver-type being 0 seems
> under conflict already. The table in this page:
> https://www.kernel.org/doc/html/latest/userspace-api/ioctl/ioctl-number.html
> But that is first four out of many others. So those four will fall into
> else-part (if ever we get there) and everything else will go into the
> if-part.
Agreed, the fact that we might have these crashes also seems to be
another CON for using the ioctl approach

> 
> Let's see whether Paul considers all this an improvement from what is
> present now.



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

      parent reply	other threads:[~2022-11-17  9:26 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <CGME20221116125430eucas1p2f2969a4a795614ce3b8c06f9ea3be36f@eucas1p2.samsung.com>
2022-11-16 12:50 ` [RFC 0/1] RFC on how to include LSM hooks for io_uring commands Joel Granados
     [not found]   ` <CGME20221116125431eucas1p1dfd03b80863fce674a7c662660c94092@eucas1p1.samsung.com>
2022-11-16 12:50     ` [RFC 1/1] Use ioctl selinux callback io_uring commands that implement the ioctl op convention Joel Granados
2022-11-16 17:38       ` Kanchan Joshi
2022-11-16 19:21         ` Paul Moore
2022-11-17  9:40           ` Joel Granados
2022-11-17 22:10             ` Paul Moore
2022-11-21 19:53               ` Luis Chamberlain
2022-11-21 21:05                 ` Paul Moore
2022-11-22 11:18                   ` Joel Granados
2022-11-22 14:04                   ` Ming Lei
2022-11-28 10:13                     ` Joel Granados
2022-11-28 10:59                       ` Ming Lei
2022-11-22 11:15                 ` Joel Granados
2022-11-17  9:25         ` Joel Granados [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20221117092546.hjzbt52pprztt6dh@localhost \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox