Re: [PATCH RFC] io_uring: Fix addr usage on get_unmapped_area()

[Helge Deller <deller@gmx.de>]

On 6/25/25 22:03, Peter Xu wrote:
> mm_get_unmapped_area() is an internal mm API to calculate virtual addresses
> for mmap().  Here, addr parameter should normally be passed over from an
> userspace as hint address.  When with MAP_FIXED, it's required to use the
> address or fail the mmap().
> 
> When reviewing existing mm_get_unmapped_area() users, I stumbled upon this
> use case, where addr will be adjusted to io_uring_validate_mmap_request().
> That internally uses io_region_get_ptr() to fetch the kernel address that
> io_uring used, by either page_address() or vmap() from io_region_init_ptr()
> calls.
> 
> Here, the io_mapped_region.ptr isn't a valid user address, hence passing it
> over to mm_get_unmapped_area() is misleading if not wrong.
> 
> The problem should be about parisc having issues with cache aliasing when
> both io_uring kernel and the userspace may map the same pages.  Here what
> matters should be pgoff rather than the address hint.  

The whole aliasing is quite fragile on parisc, and if I remember that
code it sometimes does depend on the address too.
I really want to test your patch completely, before it should go in.
I will try to test during the next few days.

Thanks!
Helge

> Simplify the code to
> keep addr=0, while setup pgoff only to make sure the VA to be calculated
> will satisfy VIPT's cache aliasing demand.
> 
> Cc: Helge Deller <deller@gmx.de>
> Cc: Jens Axboe <axboe@kernel.dk>
> Cc: Jason Gunthorpe <jgg@nvidia.com>
> Cc: io-uring@vger.kernel.org
> Signed-off-by: Peter Xu <peterx@redhat.com>
> ---
> Marking this as RFC because I don't have parisc hence no test done, but
> raise this issue.
> ---
>   io_uring/memmap.c | 14 +++++---------
>   1 file changed, 5 insertions(+), 9 deletions(-)
> 
> diff --git a/io_uring/memmap.c b/io_uring/memmap.c
> index 725dc0bec24c..8b74894489bc 100644
> --- a/io_uring/memmap.c
> +++ b/io_uring/memmap.c
> @@ -371,21 +371,17 @@ unsigned long io_uring_get_unmapped_area(struct file *filp, unsigned long addr,
>   	 * kernel memory *and* userspace memory. To achieve that:
>   	 * - use a NULL file pointer to reference physical memory, and
>   	 * - use the kernel virtual address of the shared io_uring context
> -	 *   (instead of the userspace-provided address, which has to be 0UL
> -	 *   anyway).
> -	 * - use the same pgoff which the get_unmapped_area() uses to
> -	 *   calculate the page colouring.
> +	 *   to calculate pgoff, which will be used later in parisc va
> +	 *   allocator to calculate VIPT-safe aliasing va.
>   	 * For architectures without such aliasing requirements, the
> -	 * architecture will return any suitable mapping because addr is 0.
> +	 * architecture will return any suitable mapping because pgoff is 0.
>   	 */
>   	filp = NULL;
>   	flags |= MAP_SHARED;
> -	pgoff = 0;	/* has been translated to ptr above */
>   #ifdef SHM_COLOUR
> -	addr = (uintptr_t) ptr;
> -	pgoff = addr >> PAGE_SHIFT;
> +	pgoff = (uintptr_t)ptr >> PAGE_SHIFT;
>   #else
> -	addr = 0UL;
> +	pgoff = 0;
>   #endif
>   	return mm_get_unmapped_area(current->mm, filp, addr, len, pgoff, flags);
>   }