Re: [PATCH V1 1/1] accel/amdxdna: Fix unsafe use of handle_mm_fault()

[sashiko-bot@kernel.org]

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 <lizhi.hou@amd.com>

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/20260909163211.950709-1-lizhi.hou@amd.com?part=1