Re: [PATCH] io_uring/query: cap user size passed to copy_struct_to_user

[Gabriel Krisman Bertazi]

Laxman Acharya <acharyalaxman8848@xxxxxxxxx> writes:

> From: Laxman Acharya Padhya <acharyalaxman8848@xxxxxxxxx>
>
> io_handle_query_entry() clamps hdr.size for the inbound copy_from_user()
> but keeps the original user value as usize. copy_struct_to_user() uses
> that usize and, when it is larger than the kernel result, clear_user()s
> the trailing bytes.
>
> As hdr.size is a __u32, a query can request nearly 4 GiB of zeroing,
> including on the error path where res_size stays 0. The interface is
> reachable without a ring via IORING_REGISTER_QUERY.
>
> Reject sizes larger than PAGE_SIZE, as recommended for copy_struct_*
> interfaces.
>
> Fixes: c265ae75f900 ("io_uring: introduce io_uring querying")
> Cc: stable@xxxxxxxxxxxxxxx # 6.18+
> Signed-off-by: Laxman Acharya Padhya <acharyalaxman8848@xxxxxxxxx>

Looks good. feel free to add:

Reviewed-by: Gabriel Krisman Bertazi <krisman@xxxxxxx>

> ---
>  io_uring/query.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/io_uring/query.c b/io_uring/query.c
> index 88a325736992..f2b5e19e5af6 100644
> --- a/io_uring/query.c
> +++ b/io_uring/query.c
> @@ -76,6 +76,9 @@ static int io_handle_query_entry(union io_query_data *data, void __user *uhdr,
>  
>  	if (copy_from_user(&hdr, uhdr, sizeof(hdr)))
>  		return -EFAULT;
> +	/* copy_struct_to_user() zeros up to usize bytes */
> +	if (hdr.size > PAGE_SIZE)
> +		return -E2BIG;
>  	usize = hdr.size;
>  	hdr.size = min(hdr.size, IO_MAX_QUERY_SIZE);
>  	udata = u64_to_user_ptr(hdr.query_data);
> -- 
> 2.51.2

-- 
Gabriel Krisman Bertazi