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