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) <[email protected]> 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-dacf19cce...@kernel.org?part=10
