Re: [PATCH v2 11/40] infiniband: update hfi1 to use remap_vmalloc_range()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The patch breaks mmap for the EVENTS region for all dynamic contexts because remap_vmalloc_range() strictly requires the base address of the vmalloc allocation, but the patch passes a pointer with an offset.
--

commit b52cc1703a348bc2ace5ff6752750d7fd8d2282e
Author: Lorenzo Stoakes (ARM) <ljs@kernel.org>

infiniband: update hfi1 to use remap_vmalloc_range()

This commit updates the hfi1 infiniband driver to use remap_vmalloc_range()
for mapping chip memory from vmalloc'd ranges instead of using a custom
fault handler. It aims to prevent exposing kernel virtual addresses and
cleans up VM flags.

> diff --git a/drivers/infiniband/hw/hfi1/file_ops.c b/drivers/infiniband/hw/hfi1/file_ops.c
> --- a/drivers/infiniband/hw/hfi1/file_ops.c
> +++ b/drivers/infiniband/hw/hfi1/file_ops.c
> @@ -476,15 +472,10 @@ static int hfi1_file_mmap(struct file *fp, struct vm_area_struct *vma)
>  		 * Use the page where this context's flags are. User level
>  		 * knows where it's own bitmap is within the page.
>  		 */
> -		memaddr = (unsigned long)
> -			(dd->events + uctxt_offset(uctxt)) & PAGE_MASK;
> +		memvirt = dd->events + uctxt_offset(uctxt);
> +		memvirt = (void *)(((uintptr_t)memvirt) & PAGE_MASK);
>  		memlen = PAGE_SIZE;
> -		/*
> -		 * v3.7 removes VM_RESERVED but the effect is kept by
> -		 * using VM_IO.
> -		 */
> -		flags |= VM_IO | VM_DONTEXPAND;
> -		vmf = 1;
> +		is_vmalloc = 1;
>  		break;
>  	case STATUS:

[ ... ]

> @@ -559,12 +546,10 @@ static int hfi1_file_mmap(struct file *fp, struct vm_area_struct *vma)
>  	}
>  
>  	vm_flags_reset(vma, flags);
> -	mmap_cdbg(ctxt, subctxt, type, mapio, vmf, memaddr, memvirt, memdma, 
> +	mmap_cdbg(ctxt, subctxt, type, mapio, is_vmalloc, memaddr, memvirt, memdma,
>  		  memlen, vma);
> -	if (vmf) {
> -		vma->vm_pgoff = PFN_DOWN(memaddr);
> -		vma->vm_ops = &vm_ops;
> -		ret = 0;
> +	if (is_vmalloc) {
> +		ret = remap_vmalloc_range(vma, memvirt, 0);

[Severity: High]
Will this fail for secondary contexts where uctxt_offset(uctxt) > 0?

When mapping the EVENTS region for any context > 0, memvirt is calculated as
an offset from the vmalloc base address (dd->events). However,
remap_vmalloc_range() strictly requires the exact base address of the
vmalloc allocation. 

Because remap_vmalloc_range() expects the provided address to exactly match
the area base address, won't it unconditionally return -EINVAL when passed
this interior pointer? 

If so, does this mean userspace applications attempting to mmap the EVENTS
page for any dynamic context will receive an unexpected -EINVAL error,
breaking core functionality for secondary contexts?

>  	} else if (memdma) {
>  		ret = dma_mmap_coherent(&dd->pcidev->dev, vma,
>  					memvirt, memdma, memlen);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260914-b4-mmap-prepare-vma-flag-sanify-v2-0-7d9781ed5361@kernel.org?part=11