Re: [PATCH] io_uring: fix missing submitter_task ownership check in bpf_io_reg()

[Gabriel Krisman Bertazi <krisman@suse.de>]

Ali Raza <elirazamumtaz@gmail.com> writes:

> bpf_io_reg() installs a BPF struct_ops loop_step on any io_uring ring
> the caller holds a file descriptor for.  io_uring_ctx_get_file() only
> validates that the fd resolves to an io_uring file; it does not verify
> the caller has authority over the ring's submitter_task.
>
> A parallel path in io_uring_register() already enforces this:
>
>     if (ctx->submitter_task && ctx->submitter_task != current)
>         return -EEXIST;  /* register.c:733 */

How is this a protection?  I thought ctx->submitter_task is about
IORING_SETUP_SINGLE_ISSUER. there is no permission or capability over
it against other processes.

> Without the equivalent check in bpf_io_reg(), a local user with
> CAP_PERFMON can exploit IORING_SETUP_R_DISABLED -- which defers

I'd argue this is a non-issue.  If you have CAP_PERFMON, you are able to
mess with the process in many ways beyond this.  Otherwise, how a
process would be able to get the fd in the first place?

> submitter_task assignment until IORING_REGISTER_ENABLE_RINGS -- to
> install a loop_step on a ring before a more-privileged process becomes
> its submitter_task.  The loop_step then executes in the privileged
> process's task context and can issue arbitrary io_uring operations
> (IORING_OP_WRITE, IORING_OP_READ, IORING_OP_SPLICE) against that
> process's open file table.  This provides a cross-privilege io_uring
> execution primitive that can serve as a component in a privilege
> escalation chain when combined with a vector that induces a privileged
> process to adopt an attacker-controlled ring.
>
> Affected: v7.1-rc1+ with CONFIG_IO_URING_BPF_OPS=y.
> Requires: IORING_SETUP_DEFER_TASKRUN | IORING_SETUP_SINGLE_ISSUER.
>
> Add the ownership check in io_install_bpf(), which is called under
> uring_lock, matching the locking context of the register.c check.
>
> Signed-off-by: Ali Raza <elirazamumtaz@gmail.com>
> Cc: Pavel Begunkov <asml.silence@gmail.com>
> ---
>  io_uring/bpf-ops.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/io_uring/bpf-ops.c b/io_uring/bpf-ops.c
> index 937e48bef40b..cac11c929297 100644
> --- a/io_uring/bpf-ops.c
> +++ b/io_uring/bpf-ops.c
> @@ -162,6 +162,8 @@ static int io_install_bpf(struct io_ring_ctx *ctx, struct io_uring_bpf_ops *ops)
>  		return -EOPNOTSUPP;
>  	if (!(ctx->flags & IORING_SETUP_DEFER_TASKRUN))
>  		return -EOPNOTSUPP;
> +	if (ctx->submitter_task && ctx->submitter_task != current)
> +		return -EPERM;
>  
>  	if (ctx->bpf_ops)
>  		return -EBUSY;
>
> ---
> base-commit: bea8d77e45a8b77f2beca1affc9aa7ed28f39b17
> change-id: 20260422-master-d96fe0e8bb3c
>
> Best regards,
> --  
>
> Ali Raza <elirazamumtaz@gmail.com>
>

-- 
Gabriel Krisman Bertazi