From: Casey Schaufler <[email protected]>
To: Joel Granados <[email protected]>,
[email protected], [email protected], [email protected],
[email protected]
Cc: [email protected], [email protected],
[email protected], [email protected],
[email protected]
Subject: Re: [RFC v2 1/1] Use a fs callback to set security specific data
Date: Tue, 22 Nov 2022 07:18:24 -0800 [thread overview]
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>
On 11/22/2022 2:31 AM, Joel Granados wrote:
> Signed-off-by: Joel Granados <[email protected]>
> ---
> 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/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
> index f94b05c585cb..275826fe3c9e 100644
> --- a/drivers/nvme/host/core.c
> +++ b/drivers/nvme/host/core.c
> @@ -4,6 +4,7 @@
> * Copyright (c) 2011-2014, Intel Corporation.
> */
>
> +#include "linux/security.h"
> #include <linux/blkdev.h>
> #include <linux/blk-mq.h>
> #include <linux/blk-integrity.h>
> @@ -3308,6 +3309,13 @@ static int nvme_dev_release(struct inode *inode, struct file *file)
> return 0;
> }
>
> +int nvme_uring_cmd_sec(struct io_uring_cmd *ioucmd, struct security_uring_cmd *sec)
> +{
> + sec->flags = 0;
> + sec->flags = SECURITY_URING_CMD_TYPE_IOCTL;
> + return 0;
> +}
> +
> static const struct file_operations nvme_dev_fops = {
> .owner = THIS_MODULE,
> .open = nvme_dev_open,
> @@ -3315,6 +3323,7 @@ static const struct file_operations nvme_dev_fops = {
> .unlocked_ioctl = nvme_dev_ioctl,
> .compat_ioctl = compat_ptr_ioctl,
> .uring_cmd = nvme_dev_uring_cmd,
> + .uring_cmd_sec = nvme_uring_cmd_sec,
> };
>
> static ssize_t nvme_sysfs_reset(struct device *dev,
> @@ -3982,6 +3991,7 @@ static const struct file_operations nvme_ns_chr_fops = {
> .compat_ioctl = compat_ptr_ioctl,
> .uring_cmd = nvme_ns_chr_uring_cmd,
> .uring_cmd_iopoll = nvme_ns_chr_uring_cmd_iopoll,
> + .uring_cmd_sec = nvme_uring_cmd_sec,
> };
>
> static int nvme_add_ns_cdev(struct nvme_ns *ns)
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index e654435f1651..af743a2dd562 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -2091,6 +2091,7 @@ struct dir_context {
>
> struct iov_iter;
> struct io_uring_cmd;
> +struct security_uring_cmd;
>
> struct file_operations {
> struct module *owner;
> @@ -2136,6 +2137,7 @@ struct file_operations {
> int (*uring_cmd)(struct io_uring_cmd *ioucmd, unsigned int issue_flags);
> int (*uring_cmd_iopoll)(struct io_uring_cmd *, struct io_comp_batch *,
> unsigned int poll_flags);
> + int (*uring_cmd_sec)(struct io_uring_cmd *ioucmd, struct security_uring_cmd*);
> } __randomize_layout;
>
> struct inode_operations {
> diff --git a/include/linux/lsm_hook_defs.h b/include/linux/lsm_hook_defs.h
> index ec119da1d89b..6cef29bce373 100644
> --- a/include/linux/lsm_hook_defs.h
> +++ b/include/linux/lsm_hook_defs.h
> @@ -408,5 +408,6 @@ 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)
> +LSM_HOOK(int, 0, uring_cmd, struct io_uring_cmd *ioucmd,
> + int (*uring_cmd_sec)(struct io_uring_cmd *ioucmd, struct security_uring_cmd*))
I'm slow, and I'm sure the question has been covered elsewhere,
but I have real trouble understanding why you're sending a function
to fetch the security data rather than the data itself. Callbacks
are not usual for LSM interfaces. If multiple security modules have
uring_cmd hooks (e.g. SELinux and landlock) the callback has to be
called multiple times.
> #endif /* CONFIG_IO_URING */
> diff --git a/include/linux/security.h b/include/linux/security.h
> index ca1b7109c0db..146b1bbdc2e0 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -2065,10 +2065,20 @@ static inline int security_perf_event_write(struct perf_event *event)
> #endif /* CONFIG_PERF_EVENTS */
>
> #ifdef CONFIG_IO_URING
> +enum security_uring_cmd_type
> +{
> + SECURITY_URING_CMD_TYPE_IOCTL,
> +};
> +
> +struct security_uring_cmd {
> + u64 flags;
> +};
> #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);
> +extern int security_uring_cmd(struct io_uring_cmd *ioucmd,
> + int (*uring_cmd_sec)(struct io_uring_cmd *,
> + struct security_uring_cmd*));
> #else
> static inline int security_uring_override_creds(const struct cred *new)
> {
> @@ -2078,7 +2088,9 @@ static inline int security_uring_sqpoll(void)
> {
> return 0;
> }
> -static inline int security_uring_cmd(struct io_uring_cmd *ioucmd)
> +static inline int security_uring_cmd(struct io_uring_cmd *ioucmd,
> + int (*uring_cmd_sec)(struct io_uring_cmd *,
> + struct security_uring_cmd*))
> {
> return 0;
> }
> diff --git a/io_uring/uring_cmd.c b/io_uring/uring_cmd.c
> index e50de0b6b9f8..2f650b346756 100644
> --- a/io_uring/uring_cmd.c
> +++ b/io_uring/uring_cmd.c
> @@ -108,10 +108,11 @@ int io_uring_cmd(struct io_kiocb *req, unsigned int issue_flags)
> struct file *file = req->file;
> int ret;
>
> + //req->file->f_op->owner->ei_funcs
> if (!req->file->f_op->uring_cmd)
> return -EOPNOTSUPP;
>
> - ret = security_uring_cmd(ioucmd);
> + ret = security_uring_cmd(ioucmd, req->file->f_op->uring_cmd_sec);
> if (ret)
> return ret;
>
> diff --git a/security/security.c b/security/security.c
> index 79d82cb6e469..d3360a32f971 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -2667,8 +2667,9 @@ int security_uring_sqpoll(void)
> {
> return call_int_hook(uring_sqpoll, 0);
> }
> -int security_uring_cmd(struct io_uring_cmd *ioucmd)
> +int security_uring_cmd(struct io_uring_cmd *ioucmd,
> + int (*uring_cmd_sec)(struct io_uring_cmd *ioucmd, struct security_uring_cmd*))
> {
> - return call_int_hook(uring_cmd, 0, ioucmd);
> + return call_int_hook(uring_cmd, 0, ioucmd, uring_cmd_sec);
> }
> #endif /* CONFIG_IO_URING */
> 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*))
> {
> 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);
> +
> return avc_has_perm(&selinux_state, current_sid(), isec->sid,
> SECCLASS_IO_URING, IO_URING__CMD, &ad);
> +
> }
> #endif /* CONFIG_IO_URING */
>
next prev parent reply other threads:[~2022-11-22 15:18 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <CGME20221122103536eucas1p2a0bc5ebdf063715f063e5b6254d0b058@eucas1p2.samsung.com>
2022-11-22 10:31 ` [RFC v2 0/1] RFC on how to include LSM hooks for io_uring commands Joel Granados
[not found] ` <CGME20221122103536eucas1p28f1c88f2300e49942c789721fe70c428@eucas1p2.samsung.com>
2022-11-22 10:31 ` [RFC v2 1/1] Use a fs callback to set security specific data Joel Granados
2022-11-22 15:18 ` Casey Schaufler [this message]
2022-11-28 8:19 ` Joel Granados
2022-11-28 9:06 ` Joel Granados
2022-11-23 21:02 ` Paul Moore
2022-11-28 9:27 ` Joel Granados
2022-11-29 14:24 ` Christoph Hellwig
2022-11-30 21:29 ` Joel Granados
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=1afc3928-710e-9b0f-5b0a-cf2cf8d79cb9@schaufler-ca.com \
[email protected] \
[email protected] \
[email protected] \
[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