Re: [PATCH 10/39] infiniband: update hfi1 to use remap_vmalloc_range()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [Medium] RTAIL mmap command loses VM_DONTEXPAND when mapped via dma_mmap_coherent() on IOMMU-DMA paths.

Pre-existing issues:
- [High] Incomplete restriction of memory mapping permissions allows unintended read or write access.
--

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

infiniband: update hfi1 to use remap_vmalloc_range()

This patch converts 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 also updates flag handling and sets the virtual kernel address
for various mapping types.

> 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
[ ... ]
> @@ -366,7 +361,7 @@ static int hfi1_file_mmap(struct file *fp, struct vm_area_struct *vma)
>  		 */
>  		memlen = PAGE_ALIGN(uctxt->sc->credits * PIO_BLOCK_SIZE);
>  		flags &= ~VM_MAYREAD;
> -		flags |= VM_DONTCOPY | VM_DONTEXPAND;
> +		flags |= VM_DONTCOPY;

[Severity: High]
This is a pre-existing issue, but does this incomplete restriction of memory
permissions in hfi1_file_mmap() allow unintended read access to PIO buffers?

When a user calls mmap() on PIO_BUFS requesting PROT_READ, the driver clears
VM_MAYREAD from the flags but fails to check and reject the call if VM_READ
is already set. Because vma->vm_page_prot is computed before this callback
based on the initial VM_READ flag, the PTEs will be populated with read
permissions. This potentially allows reading of sensitive data written by
other subcontexts.

[ ... ]
>  	case RTAIL:
[ ... ]
>  		memlen = PAGE_SIZE;
>  		memvirt = (void *)hfi1_rcvhdrtail_kvaddr(uctxt);
>  		memdma = uctxt->rcvhdrqtailaddr_dma;
>  		flags &= ~VM_MAYWRITE;
>  		break;

[Severity: Medium]
Is it intentional that the VM_DONTEXPAND flag was removed from the RTAIL
path here?

The RTAIL path was converted to rely on dma_mmap_coherent(), but the
VM_DONTEXPAND flag was dropped. When a user maps the RTAIL buffer and then
calls mremap() to expand the VMA, the expansion could succeed incorrectly
because dma_mmap_coherent() does not set VM_DONTEXPAND on IOMMU-DMA paths.
When expanded on an IOMMU-DMA path, the new PTEs will not be populated,
leading to a SIGBUS or page fault on access.

Note that the commit message explicitly documented retaining VM_DONTEXPAND
for PIO_CRED because of this exact dma_mmap_coherent() behavior, but it
appears it was not retained for RTAIL.

[ ... ]
>  	case SUBCTXT_EGRBUF:
> -		memaddr = (u64)uctxt->subctxt_rcvegrbuf;
> +		memvirt = uctxt->subctxt_rcvegrbuf;
>  		memlen = uctxt->egrbufs.size * uctxt->subctxt_cnt;
> -		flags |= VM_IO | VM_DONTEXPAND;
>  		flags &= ~VM_MAYWRITE;
> -		vmf = 1;
> +		is_vmalloc = 1;
>  		break;

[Severity: High]
This is a pre-existing issue, but does this incomplete restriction of memory
permissions in hfi1_file_mmap() allow unintended write access to the shared
subcontext eager receive buffers?

Similar to the PIO_BUFS case above, when a user calls mmap() on
SUBCTXT_EGRBUF requesting PROT_WRITE, the driver clears VM_MAYWRITE from the
flags but fails to check and reject the call if VM_WRITE is already set.

Because vma->vm_page_prot is computed before this callback based on the
initial VM_WRITE flag, the PTEs will be populated with write permissions.
Write access to read-only eager buffers allows a process to corrupt or spoof
incoming network packets for other subcontexts sharing the hardware context.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260908-b4-mmap-prepare-vma-flag-sanify-v1-0-dacf19cce22b@kernel.org?part=10