Re: [PATCH v1 2/3] io_uring/rsrc: don't rely on user vaddr alignment

[David Hildenbrand <david@redhat.com>]

On 24.06.25 12:35, Pavel Begunkov wrote:
> There is no guaranteed alignment for user pointers, however the
> calculation of an offset of the first page into a folio after
> coalescing uses some weird bit mask logic, get rid of it.
> 
> Cc: stable@vger.kernel.org
> Reported-by: David Hildenbrand <david@redhat.com>
> Fixes: a8edbb424b139 ("io_uring/rsrc: enable multi-hugepage buffer coalescing")
> Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
> ---
>   io_uring/rsrc.c | 8 +++++++-
>   io_uring/rsrc.h | 1 +
>   2 files changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/io_uring/rsrc.c b/io_uring/rsrc.c
> index e83a294c718b..5132f8df600f 100644
> --- a/io_uring/rsrc.c
> +++ b/io_uring/rsrc.c
> @@ -734,6 +734,8 @@ bool io_check_coalesce_buffer(struct page **page_array, int nr_pages,
>   
>   	data->nr_pages_mid = folio_nr_pages(folio);
>   	data->folio_shift = folio_shift(folio);
> +	data->first_page_offset = page_array[0] - compound_head(page_array[0]);

Note: pointer arithmetic on "struct page" does not work reliably for 
very large folios (eg., 1 GiB hugetlb) in all configs 
(!CONFIG_SPARSEMEM_VMEMMAP)

I assume this can be

data->first_page_offset = folio_page_idx(folio, page_array[0]);


> +	data->first_page_offset <<= PAGE_SHIFT;
>   
>   	/*
>   	 * Check if pages are contiguous inside a folio, and all folios have
> @@ -830,7 +832,11 @@ static struct io_rsrc_node *io_sqe_buffer_register(struct io_ring_ctx *ctx,
>   	if (coalesced)
>   		imu->folio_shift = data.folio_shift;
>   	refcount_set(&imu->refs, 1);
> -	off = (unsigned long) iov->iov_base & ((1UL << imu->folio_shift) - 1);
> +
> +	off = (unsigned long)iov->iov_base & ~PAGE_MASK;
> +	if (coalesced)
> +		off += data.first_page_offset;
> +
>   	node->buf = imu;
>   	ret = 0;
>   
> diff --git a/io_uring/rsrc.h b/io_uring/rsrc.h
> index 0d2138f16322..d823554a8817 100644
> --- a/io_uring/rsrc.h
> +++ b/io_uring/rsrc.h
> @@ -49,6 +49,7 @@ struct io_imu_folio_data {
>   	unsigned int	nr_pages_mid;
>   	unsigned int	folio_shift;
>   	unsigned int	nr_folios;
> +	unsigned long	first_page_offset;

Heh, is it actually "first_folio_offset" ?

Alternatively, call it "first_folio_page_idx" or sth like that and leave 
the shift to the user.

-- 
Cheers,

David / dhildenb