accel/amdxdna: Fix locally exploitable BUG_ON in amdxdna_insert_pages()

[PATCH V1] accel/amdxdna: Fix locally exploitable BUG_ON in amdxdna_insert_pages()

Posted by Lizhi Hou 2 months ago

In amdxdna_insert_pages(), vm_flags_mod() sets VM_MIXEDMAP and clears
VM_PFNMAP. If an unprivileged userspace process mmaps a non-imported GEM
object and then calls madvise(MADV_DONTNEED), the PTEs will be
successfully cleared because VM_MIXEDMAP allows this (unlike VM_PFNMAP).

When userspace subsequently accesses the memory, drm_gem_shmem_fault()
handles the page fault and attempts to map the backing shmem page via
vmf_insert_pfn() which calls vmf_insert_pfn_prot(). Because the backing
shmem page is normal system memory (pfn_valid(pfn) is true) and the VMA
now has VM_MIXEDMAP set, won't this predictably trigger the explicit
assertion BUG_ON((vma->vm_flags & VM_MIXEDMAP) && pfn_valid(pfn))

Fix by removing the vm_flags_mod() call and replacing the vm_insert_pages()
pre-population with the handle_mm_fault() loop that was already used for
the import (dma-buf) path.

Fixes: e486147c912f ("accel/amdxdna: Add BO import and export")
Signed-off-by: Lizhi Hou <lizhi.hou@amd.com>
---
 drivers/accel/amdxdna/amdxdna_gem.c | 27 ++++++++-------------------
 1 file changed, 8 insertions(+), 19 deletions(-)

diff --git a/drivers/accel/amdxdna/amdxdna_gem.c b/drivers/accel/amdxdna/amdxdna_gem.c
index aec69d5f463e..1c63eff0a4a8 100644
--- a/drivers/accel/amdxdna/amdxdna_gem.c
+++ b/drivers/accel/amdxdna/amdxdna_gem.c
@@ -467,25 +467,17 @@ static int amdxdna_insert_pages(struct amdxdna_gem_obj *abo,
 			XDNA_ERR(xdna, "Failed shmem mmap %d", ret);
 			return ret;
 		}
-
-		/* The buffer is based on memory pages. Fix the flag. */
-		vm_flags_mod(vma, VM_MIXEDMAP, VM_PFNMAP);
-		ret = vm_insert_pages(vma, vma->vm_start, abo->base.pages,
-				      &num_pages);
+	} else {
+		vma->vm_private_data = NULL;
+		vma->vm_ops = NULL;
+		ret = dma_buf_mmap(abo->dma_buf, vma, 0);
 		if (ret) {
-			XDNA_ERR(xdna, "Failed insert pages %d", ret);
-			amdxdna_mark_mapp_invalid(abo, vma);
+			XDNA_ERR(xdna, "Failed to mmap dma buf %d", ret);
+			return ret;
 		}
 
-		return 0;
-	}
-
-	vma->vm_private_data = NULL;
-	vma->vm_ops = NULL;
-	ret = dma_buf_mmap(abo->dma_buf, vma, 0);
-	if (ret) {
-		XDNA_ERR(xdna, "Failed to mmap dma buf %d", ret);
-		return ret;
+		/* Drop the reference drm_gem_mmap_obj() acquired.*/
+		drm_gem_object_put(to_gobj(abo));
 	}
 
 	do {
@@ -502,9 +494,6 @@ static int amdxdna_insert_pages(struct amdxdna_gem_obj *abo,
 		offset += PAGE_SIZE;
 	} while (--num_pages);
 
-	/* Drop the reference drm_gem_mmap_obj() acquired.*/
-	drm_gem_object_put(to_gobj(abo));
-
 	return 0;
 }
 
-- 
2.34.1

Re: [PATCH V1] accel/amdxdna: Fix locally exploitable BUG_ON in amdxdna_insert_pages()

Posted by Max Zhen 2 months ago


On 7/31/2026 Fri 11:59, Lizhi Hou wrote:
> In amdxdna_insert_pages(), vm_flags_mod() sets VM_MIXEDMAP and clears
> VM_PFNMAP. If an unprivileged userspace process mmaps a non-imported GEM
> object and then calls madvise(MADV_DONTNEED), the PTEs will be
> successfully cleared because VM_MIXEDMAP allows this (unlike VM_PFNMAP).
> 
> When userspace subsequently accesses the memory, drm_gem_shmem_fault()
> handles the page fault and attempts to map the backing shmem page via
> vmf_insert_pfn() which calls vmf_insert_pfn_prot(). Because the backing
> shmem page is normal system memory (pfn_valid(pfn) is true) and the VMA
> now has VM_MIXEDMAP set, won't this predictably trigger the explicit
> assertion BUG_ON((vma->vm_flags & VM_MIXEDMAP) && pfn_valid(pfn))
> 
> Fix by removing the vm_flags_mod() call and replacing the vm_insert_pages()
> pre-population with the handle_mm_fault() loop that was already used for
> the import (dma-buf) path.
> 
> Fixes: e486147c912f ("accel/amdxdna: Add BO import and export")
> Signed-off-by: Lizhi Hou <lizhi.hou@amd.com>
Reviewed-by: Max Zhen <max.zhen@amd.com>
> ---
>   drivers/accel/amdxdna/amdxdna_gem.c | 27 ++++++++-------------------
>   1 file changed, 8 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/accel/amdxdna/amdxdna_gem.c b/drivers/accel/amdxdna/amdxdna_gem.c
> index aec69d5f463e..1c63eff0a4a8 100644
> --- a/drivers/accel/amdxdna/amdxdna_gem.c
> +++ b/drivers/accel/amdxdna/amdxdna_gem.c
> @@ -467,25 +467,17 @@ static int amdxdna_insert_pages(struct amdxdna_gem_obj *abo,
>   			XDNA_ERR(xdna, "Failed shmem mmap %d", ret);
>   			return ret;
>   		}
> -
> -		/* The buffer is based on memory pages. Fix the flag. */
> -		vm_flags_mod(vma, VM_MIXEDMAP, VM_PFNMAP);
> -		ret = vm_insert_pages(vma, vma->vm_start, abo->base.pages,
> -				      &num_pages);
> +	} else {
> +		vma->vm_private_data = NULL;
> +		vma->vm_ops = NULL;
> +		ret = dma_buf_mmap(abo->dma_buf, vma, 0);
>   		if (ret) {
> -			XDNA_ERR(xdna, "Failed insert pages %d", ret);
> -			amdxdna_mark_mapp_invalid(abo, vma);
> +			XDNA_ERR(xdna, "Failed to mmap dma buf %d", ret);
> +			return ret;
>   		}
>   
> -		return 0;
> -	}
> -
> -	vma->vm_private_data = NULL;
> -	vma->vm_ops = NULL;
> -	ret = dma_buf_mmap(abo->dma_buf, vma, 0);
> -	if (ret) {
> -		XDNA_ERR(xdna, "Failed to mmap dma buf %d", ret);
> -		return ret;
> +		/* Drop the reference drm_gem_mmap_obj() acquired.*/
> +		drm_gem_object_put(to_gobj(abo));
>   	}
>   
>   	do {
> @@ -502,9 +494,6 @@ static int amdxdna_insert_pages(struct amdxdna_gem_obj *abo,
>   		offset += PAGE_SIZE;
>   	} while (--num_pages);
>   
> -	/* Drop the reference drm_gem_mmap_obj() acquired.*/
> -	drm_gem_object_put(to_gobj(abo));
> -
>   	return 0;
>   }
>

Re: [PATCH V1] accel/amdxdna: Fix locally exploitable BUG_ON in amdxdna_insert_pages()

Posted by Lizhi Hou 2 months ago

Applied to drm-misc-fixes.

On 7/31/26 13:59, Max Zhen wrote:
>
>
> On 7/31/2026 Fri 11:59, Lizhi Hou wrote:
>> In amdxdna_insert_pages(), vm_flags_mod() sets VM_MIXEDMAP and clears
>> VM_PFNMAP. If an unprivileged userspace process mmaps a non-imported GEM
>> object and then calls madvise(MADV_DONTNEED), the PTEs will be
>> successfully cleared because VM_MIXEDMAP allows this (unlike VM_PFNMAP).
>>
>> When userspace subsequently accesses the memory, drm_gem_shmem_fault()
>> handles the page fault and attempts to map the backing shmem page via
>> vmf_insert_pfn() which calls vmf_insert_pfn_prot(). Because the backing
>> shmem page is normal system memory (pfn_valid(pfn) is true) and the VMA
>> now has VM_MIXEDMAP set, won't this predictably trigger the explicit
>> assertion BUG_ON((vma->vm_flags & VM_MIXEDMAP) && pfn_valid(pfn))
>>
>> Fix by removing the vm_flags_mod() call and replacing the 
>> vm_insert_pages()
>> pre-population with the handle_mm_fault() loop that was already used for
>> the import (dma-buf) path.
>>
>> Fixes: e486147c912f ("accel/amdxdna: Add BO import and export")
>> Signed-off-by: Lizhi Hou <lizhi.hou@amd.com>
> Reviewed-by: Max Zhen <max.zhen@amd.com>
>> ---
>>   drivers/accel/amdxdna/amdxdna_gem.c | 27 ++++++++-------------------
>>   1 file changed, 8 insertions(+), 19 deletions(-)
>>
>> diff --git a/drivers/accel/amdxdna/amdxdna_gem.c 
>> b/drivers/accel/amdxdna/amdxdna_gem.c
>> index aec69d5f463e..1c63eff0a4a8 100644
>> --- a/drivers/accel/amdxdna/amdxdna_gem.c
>> +++ b/drivers/accel/amdxdna/amdxdna_gem.c
>> @@ -467,25 +467,17 @@ static int amdxdna_insert_pages(struct 
>> amdxdna_gem_obj *abo,
>>               XDNA_ERR(xdna, "Failed shmem mmap %d", ret);
>>               return ret;
>>           }
>> -
>> -        /* The buffer is based on memory pages. Fix the flag. */
>> -        vm_flags_mod(vma, VM_MIXEDMAP, VM_PFNMAP);
>> -        ret = vm_insert_pages(vma, vma->vm_start, abo->base.pages,
>> -                      &num_pages);
>> +    } else {
>> +        vma->vm_private_data = NULL;
>> +        vma->vm_ops = NULL;
>> +        ret = dma_buf_mmap(abo->dma_buf, vma, 0);
>>           if (ret) {
>> -            XDNA_ERR(xdna, "Failed insert pages %d", ret);
>> -            amdxdna_mark_mapp_invalid(abo, vma);
>> +            XDNA_ERR(xdna, "Failed to mmap dma buf %d", ret);
>> +            return ret;
>>           }
>>   -        return 0;
>> -    }
>> -
>> -    vma->vm_private_data = NULL;
>> -    vma->vm_ops = NULL;
>> -    ret = dma_buf_mmap(abo->dma_buf, vma, 0);
>> -    if (ret) {
>> -        XDNA_ERR(xdna, "Failed to mmap dma buf %d", ret);
>> -        return ret;
>> +        /* Drop the reference drm_gem_mmap_obj() acquired.*/
>> +        drm_gem_object_put(to_gobj(abo));
>>       }
>>         do {
>> @@ -502,9 +494,6 @@ static int amdxdna_insert_pages(struct 
>> amdxdna_gem_obj *abo,
>>           offset += PAGE_SIZE;
>>       } while (--num_pages);
>>   -    /* Drop the reference drm_gem_mmap_obj() acquired.*/
>> -    drm_gem_object_put(to_gobj(abo));
>> -
>>       return 0;
>>   }
>