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 --]
prev 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