On Tue, Nov 22, 2022 at 5:35 AM Joel Granados <j.granados@xxxxxxxxxxx> wrote:
>
> Signed-off-by: Joel Granados <j.granados@xxxxxxxxxxx>
> ---
> drivers/nvme/host/core.c | 10 ++++++++++
> include/linux/fs.h | 2 ++
> include/linux/lsm_hook_defs.h | 3 ++-
> include/linux/security.h | 16 ++++++++++++++--
> io_uring/uring_cmd.c | 3 ++-
> security/security.c | 5 +++--
> security/selinux/hooks.c | 16 +++++++++++++++-
> 7 files changed, 48 insertions(+), 7 deletions(-)
...
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index f553c370397e..9fe3a230c671 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -21,6 +21,8 @@
> * Copyright (C) 2016 Mellanox Technologies
> */
>
> +#include "linux/nvme_ioctl.h"
> +#include "linux/security.h"
> #include <linux/init.h>
> #include <linux/kd.h>
> #include <linux/kernel.h>
> @@ -6999,18 +7001,30 @@ static int selinux_uring_sqpoll(void)
> * IORING_OP_URING_CMD against the device/file specified in @ioucmd.
> *
> */
> -static int selinux_uring_cmd(struct io_uring_cmd *ioucmd)
> +static int selinux_uring_cmd(struct io_uring_cmd *ioucmd,
> + int (*uring_cmd_sec)(struct io_uring_cmd *ioucmd, struct security_uring_cmd*))
> {
As we discussed in the previous thread, and Casey mentioned already,
passing a function pointer for the LSM to call isn't a great practice.
When it was proposed we hadn't really thought of any alternatives, but
if we can't find a good scalar value to compare somewhere, I think
inspecting the file_operations::owner::name string to determine the
target is preferable to the function pointer approach described here.
Although I really would like to see us find, or create, some sort of
scalar token ID we could use instead. I fear that doing a lot of
strcmp()'s to identify the uring command target is going to be a
problem (one strcmp() for each possible target, multiplied by the
number of LSMs which implement a io_uring command hook).
> struct file *file = ioucmd->file;
> 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();
> + struct security_uring_cmd sec_uring = {0};
> + int ret;
>
> ad.type = LSM_AUDIT_DATA_FILE;
> ad.u.file = file;
>
> + ret = uring_cmd_sec(ioucmd, &sec_uring);
> + if (ret)
> + return ret;
> +
> + if (sec_uring.flags & SECURITY_URING_CMD_TYPE_IOCTL)
> + return ioctl_has_perm(cred, file, FILE__IOCTL, (u16) ioucmd->cmd_op);
As mentioned previously, we'll need a SELinux policy capability here
to preserve the SECCLASS_IO_URING/IO_URING__CMD access check for
existing users/policies. I expect the logic would look something like
this (of course the details are dependent on how we identify the
target module/device/etc.):
if (polcap_foo && uring_tgt) {
switch (uring_tgt) {
case NVME:
return avc_has_perm(...);
default:
WARN();
return avc_has_perm(SECCLASS_IO_URING, IO_URING__CMD);
}
} else
return avc_has_perm(SECCLASS_IO_URING, IO_URING__CMD);
> return avc_has_perm(&selinux_state, current_sid(), isec->sid,
> SECCLASS_IO_URING, IO_URING__CMD, &ad);
> +
> }
> #endif /* CONFIG_IO_URING */
--
paul-moore.com
