Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] Using remap_pfn_range() to map normal refcounted shmem folios creates special PTEs, violating MM invariants. --
commit 2da717e72d7319305e74d66b3b963f132ff12737 Author: Lizhi Hou <[email protected]> accel/amdxdna: Fix unsafe use of handle_mm_fault() This commit removes the unsafe use of handle_mm_fault() from the mmap callback since the VMA is not yet linked. For shmem BOs, it replaces the faulting loop with direct calls to remap_pfn_range(), and for imported BOs it marks the mapping as invalid to be faulted in later. > diff --git a/drivers/accel/amdxdna/amdxdna_gem.c > b/drivers/accel/amdxdna/amdxdna_gem.c > index 3f05fa8811af1..7a5b0ad3235fd 100644 > --- a/drivers/accel/amdxdna/amdxdna_gem.c > +++ b/drivers/accel/amdxdna/amdxdna_gem.c [ ... ] > @@ -490,16 +491,10 @@ static int amdxdna_insert_pages(struct amdxdna_gem_obj > *abo, [ ... ] > + ret = drm_gem_shmem_mmap(&abo->base, vma); > + if (ret) { > + XDNA_ERR(xdna, "Failed shmem mmap %d", ret); > + return ret; > } > > - do { > - vm_fault_t fault_ret; > + for (i = 0; i < num_pages; i++) { > > - fault_ret = handle_mm_fault(vma, vma->vm_start + offset, > - FAULT_FLAG_WRITE, NULL); > - if (fault_ret & VM_FAULT_ERROR) { > - XDNA_ERR(xdna, "Fault in page failed"); > - amdxdna_mark_mapp_invalid(abo, vma); > - break; > + ret = remap_pfn_range(vma, vma->vm_start + (i << PAGE_SHIFT), > + page_to_pfn(abo->base.pages[vma->vm_pgoff > + i]), > + PAGE_SIZE, vma->vm_page_prot); [Severity: High] Does using remap_pfn_range() here to map normal refcounted shmem folios create special PTEs? Since remap_pfn_range() is designed for raw PFN mappings, it sets the special bit on the resulting page table entries. If this is applied to normal refcounted folios from the shmem BO, it violates the memory management invariant that normal folios must not have special PTEs. When the special bit is set, vm_normal_page() returns NULL, which hides these pages from page table walkers, GUP (get_user_pages), and mapcount-based refcount management. Can this break reverse mapping (rmap) and lead to memory management corruption? > + if (ret) { > + if (i > 0) > + zap_special_vma_range(vma, vma->vm_start, i << > PAGE_SHIFT); > + dma_resv_lock(to_gobj(abo)->resv, NULL); > + drm_gem_shmem_put_pages_locked(&abo->base); > + dma_resv_unlock(to_gobj(abo)->resv); > + return ret; > } > - > - offset += PAGE_SIZE; > - } while (--num_pages); > + } > > return 0; > } -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
