Re: [PATCH 2/3] drm/amdgpu: fix VM update overrun on non-4K page kernels

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [Critical] Unhandled error path leaks BOs and leaves dangling pointers to a stack-allocated list, leading to stack corruption.
- [High] Sub-page offset is dropped when mapping contiguous system memory on non-4K page kernels, causing data corruption.
--

--- Patch [2]: [PATCH 2/3] drm/amdgpu: fix VM update overrun on non-4K page kernels ---
commit 250ebbca695a7882625e4e4f1a9f6af7c33fde76
Author: Junrui Luo <moonafterrain@outlook.com>
drm/amdgpu: fix VM update overrun on non-4K page kernels

This commit fixes a VM update overrun bug in amdgpu_vm_update_range() on
systems where the CPU page size is larger than the GPU page size. It clamps
num_entries against the rounded-up value during the contiguity scan. This
prevents an issue where an unprivileged caller could trigger a BUG_ON by
mapping memory with a non-multiple GPU page count.

> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> index dc6a9d7dd0b28..365a1c4a45275 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> @@ -1193,8 +1193,9 @@ int amdgpu_vm_update_range(struct amdgpu_device *adev, struct amdgpu_vm *vm,
>  				}
>  				if (!contiguous)
>  					count--;
> -				num_entries = count *
> -					AMDGPU_GPU_PAGES_IN_CPU_PAGE;
> +				num_entries = min(count *
> +						  AMDGPU_GPU_PAGES_IN_CPU_PAGE,
> +						  num_entries);
>  			}
>  
>  			if (!contiguous) {

[Severity: High]
This is a pre-existing issue, but does this code drop the sub-page offset
when mapping contiguous memory?

In amdgpu_vm_update_range(), if a user maps system memory where
PAGE_SIZE > AMDGPU_GPU_PAGE_SIZE at a sub-CPU-page offset and the pages are
physically contiguous, addr is set to pages_addr[cursor.start >> PAGE_SHIFT].

Because params.pages_addr is set to NULL, the sub-page offset
(cursor.start & ~PAGE_MASK) is dropped. Could this cause the GPU to map
memory starting from the beginning of the CPU page instead of the requested
offset, returning incorrect data to the user?

[Severity: Critical]
This is a pre-existing issue, but does the error_free path in this function
leave dangling pointers to the stack-allocated params.tlb_flush_waitlist?

During amdgpu_vm_ptes_update() later in this loop, replaced page table
entries are added to params.tlb_flush_waitlist, which is allocated on the
stack in amdgpu_vm_update_range().

If a subsequent operation fails, the code jumps to error_free:

		r = amdgpu_vm_ptes_update(&params, start, tmp, addr, flags);
		if (r)
			goto error_free;

This bypasses the call to amdgpu_vm_pt_free_list() which would normally
process and free the list.

When the function returns, its stack frame is reclaimed, but the
amdgpu_vm_bo_base entries in the software tree still have their vm_status
list nodes pointing to this reclaimed stack memory.

When the VM is later destroyed, amdgpu_vm_pt_free_root() traverses the tree
and calls list_del() on these entries. Could this write to the dangling stack
pointers and cause arbitrary stack corruption?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260806-amdgpu-fixes-v1-0-ce247012d4da@outlook.com?part=2