Re: [PATCH] io_uring/bpf-ops: reject re-registration of an already-bound ops

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

Woraphat Khiaodaeng <worapat.kd2@gmail.com> writes:

> io_install_bpf() only rejects a second registration on the ctx side
> (ctx->bpf_ops) and sets the per-map back-pointer ops->priv
> unconditionally. The struct_ops link path never advances a map past
> BPF_STRUCT_OPS_STATE_READY, so the same io_uring_bpf_ops map can be
> registered more than once, and bpf_io_reg() re-resolves the target ring
> via fget(ops->ring_fd) on every call. A caller can therefore point the
> same ring_fd at a different io_ring_ctx between two BPF_LINK_CREATE
> calls.
>
> The second registration passes the ctx->bpf_ops check (the new ctx has
> none) and overwrites ops->priv, orphaning the first ctx. Teardown
> (io_eject_bpf()/bpf_io_unreg()) only reaches a ctx through ops->priv, so
> the orphaned ctx is never torn down: its ctx->loop_step keeps pointing
> into the struct_ops trampoline, which is freed once the map is gone. A
> later io_uring_enter() on the orphaned ring then calls the dangling
> ctx->loop_step from io_run_loop() -- a use-after-free of freed
> executable memory, reachable by a task with CAP_BPF + CAP_PERFMON.
>
> Reject registration when ops->priv is already set, as hid_bpf_reg()
> does for its struct_ops.
>
> Fixes: 98f37634b12b ("io_uring/bpf-ops: implement bpf ops registration")
> Signed-off-by: Woraphat Khiaodaeng <worapat.kd2@gmail.com>

Feel free to add,

Reviewed-by: Gabriel Krisman Bertazi <krisman@suse.de>

can you send a testcase to add to liburing?

> ---
>  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 5a50f0675..cf2bd068e 100644
> --- a/io_uring/bpf-ops.c
> +++ b/io_uring/bpf-ops.c
> @@ -168,6 +168,8 @@ static int io_install_bpf(struct io_ring_ctx *ctx, struct io_uring_bpf_ops *ops)
>  
>  	if (ctx->bpf_ops)
>  		return -EBUSY;
> +	if (ops->priv)
> +		return -EBUSY;
>  	if (WARN_ON_ONCE(!ops->loop_step))
>  		return -EINVAL;
>  
> -- 
> 2.43.0
>

-- 
Gabriel Krisman Bertazi