Re: [PATCH 1/1] io_uring: prevent reg-wait speculations

[Pavel Begunkov <asml.silence@gmail.com>]

On 11/19/24 01:29, Pavel Begunkov wrote:
> With *ENTER_EXT_ARG_REG instead of passing a user pointer with arguments
> for the waiting loop the user can specify an offset into a pre-mapped
> region of memory, in which case the
> [offset, offset + sizeof(io_uring_reg_wait)) will be intepreted as the
> argument.

Jann, do mind taking a look? I hope there is some clever trick with
masks we can use instead of the barrier, it seems expensive.

The byte offset user pases is 0 based and we add it to the base
kernel address:

if (unlikely(check_add_overflow(offset, sizeof(struct ...), &end) ||
	     end > ctx->cq_wait_size))
	return ERR_PTR(-EFAULT);

barrier_nospec();
return ctx->cq_wait_arg + offset;

Here in particular we know the structure size, but I also wonder how
to do it right if size is variable.


> As we address a kernel array using a user given index, it'd be a subject
> to speculation type of exploits.
> 
> Fixes: d617b3147d54c ("io_uring: restore back registered wait arguments")
> Fixes: aa00f67adc2c0 ("io_uring: add support for fixed wait regions")
> Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
> ---
>   io_uring/io_uring.c | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/io_uring/io_uring.c b/io_uring/io_uring.c
> index da8fd460977b..3a3e4fca1545 100644
> --- a/io_uring/io_uring.c
> +++ b/io_uring/io_uring.c
> @@ -3207,6 +3207,7 @@ static struct io_uring_reg_wait *io_get_ext_arg_reg(struct io_ring_ctx *ctx,
>   		     end > ctx->cq_wait_size))
>   		return ERR_PTR(-EFAULT);
>   
> +	barrier_nospec();
>   	return ctx->cq_wait_arg + offset;
>   }
>   

-- 
Pavel Begunkov