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

[Casey Schaufler <casey@schaufler-ca.com>]

On 7/13/2022 5:05 PM, Luis Chamberlain wrote:
> 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.

tl;dr - Yuck. Again.

You're passing the complexity of uring-cmd directly into each
and every security module. SELinux, AppArmor, Smack, BPF and
every other LSM now needs to know the gory details of everything
that might be in any arbitrary subsystem so that it can make a
wild guess about what to do. And I thought ioctl was hard to deal
with.

Look at what Paul Moore did for the existing io_uring code.
Carry that forward into your passthrough implementation.
No, I don't think that waving security away because we haven't
proposed a fix for your flawed design is acceptable. Sure, we
can help.

>
> [0] https://lkml.kernel.org/r/8adf55db-7bab-f59d-d612-ed906b948d19@schaufler-ca.com
>
> Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
> ---
>  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 eafa1d2489fd..4e94755098f1 100644
> --- a/include/linux/lsm_hook_defs.h
> +++ b/include/linux/lsm_hook_defs.h
> @@ -406,4 +406,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 91c8146649f5..b681cfce6190 100644
> --- a/include/linux/lsm_hooks.h
> +++ b/include/linux/lsm_hooks.h
> @@ -1575,6 +1575,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 4d0baf30266e..421856919b1e 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -2053,6 +2053,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)
>  {
> @@ -2062,6 +2063,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 0a421ed51e7e..5e666aa7edb8 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>
>  
> @@ -82,6 +83,10 @@ int io_uring_cmd(struct io_kiocb *req, unsigned int issue_flags)
>  	struct file *file = req->file;
>  	int ret;
>  
> +	ret = security_uring_cmd(ioucmd);
> +	if (ret)
> +		return ret;
> +
>  	if (!req->file->f_op->uring_cmd)
>  		return -EOPNOTSUPP;
>  
> diff --git a/security/security.c b/security/security.c
> index f85afb02ea1c..ad7d7229bd72 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -2655,4 +2655,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 */