Re: [PATCH] drm/ttm: clamp the prefault window to the buffer object

On Thu, Aug 06, 2026 at 12:43:56PM +0900, Baul Lee wrote:
> ttm_bo_vm_fault_reserved() derives two page indices from the caller's
> mmap(2) arguments and bounds only one of them:
> 
> 	page_offset = ((address - vma->vm_start) >> PAGE_SHIFT) +
> 		vma->vm_pgoff - drm_vma_node_start(&bo->base.vma_node);
> 	page_last = vma_pages(vma) + vma->vm_pgoff -
> 		drm_vma_node_start(&bo->base.vma_node);
> 
> 	if (unlikely(page_offset >= PFN_UP(bo->base.size)))
> 		return VM_FAULT_SIGBUS;
> 
> bo->base.size appears once in the function, bounding page_offset on
> entry.  page_last comes straight from vma_pages(vma) and is the loop
> terminator:
> 
> 	if (unlikely(++page_offset >= page_last))
> 		break;
> 
> so the object size never bounds it.  For an object of N pages, a fault
> on the last in-object page passes the entry test with page_offset
> N - 1, and the prefault loop then walks N..N+14, reading
> ttm->pages[page_offset] or
> ttm_bo_io_mem_pfn(bo, page_offset) and installing each frame with
> vmf_insert_pfn_prot().
> 
> page_last exceeds the object whenever the VMA is longer than it.
> drm_gem_mmap_obj() rejects that on the DRM node, but the fbdev path
> reaches the object function through drm_gem_prime_mmap(), which does
> not.  It is also exceeded by a mapping no longer than the object taken
> at a nonzero file offset, so the handler needs its own bound.
> 
> With a 128-page object mapped 192 pages long, one read fault at index
> N - 1 leaves the fifteen frames after the object readable through the
> mapping; on a fresh mapping, reading index N without first faulting
> N - 1 is SIGBUS.  For a system-memory placement the page array is
> over-read as well:
> 
>   BUG: KASAN: slab-out-of-bounds in ttm_bo_vm_fault_reserved+0x248/0x57c
>   Read of size 8 at addr ffff0000078aac00 by task e1/219
>    __asan_load8+0x84/0xb0
>    ttm_bo_vm_fault_reserved+0x248/0x57c
>    ttm_bo_vm_fault+0xe4/0x140
>    __do_fault+0x6c/0x2f0
> 
> Clamp page_last to the object.
> 
> Discovered by XBOW, triaged by Baul Lee <[email protected]>

You are going to want an Assisted-by tag here as XBOW is an AI tool?

> 
> Fixes: ba4e7d973dd0 ("drm: Add the TTM GPU memory manager subsystem.")

This won't apply to ba4e7d973dd0. More below.

> Cc: [email protected]
> Signed-off-by: Baul Lee <[email protected]>
> ---
>  drivers/gpu/drm/ttm/ttm_bo_vm.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/gpu/drm/ttm/ttm_bo_vm.c b/drivers/gpu/drm/ttm/ttm_bo_vm.c
> index a80510489c45..14ebf6ee3c47 100644
> --- a/drivers/gpu/drm/ttm/ttm_bo_vm.c
> +++ b/drivers/gpu/drm/ttm/ttm_bo_vm.c
> @@ -212,6 +212,7 @@ vm_fault_t ttm_bo_vm_fault_reserved(struct vm_fault *vmf,
>  		vma->vm_pgoff - drm_vma_node_start(&bo->base.vma_node);
>  	page_last = vma_pages(vma) + vma->vm_pgoff -
>  		drm_vma_node_start(&bo->base.vma_node);
> +	page_last = min_t(unsigned long, page_last, PFN_UP(bo->base.size));

This looks correct but maybe to make backporting easier all the way to
ba4e7d973dd0, we do this instead...

diff --git a/drivers/gpu/drm/ttm/ttm_bo_vm.c b/drivers/gpu/drm/ttm/ttm_bo_vm.c
index a80510489c45..3529371a37d5 100644
--- a/drivers/gpu/drm/ttm/ttm_bo_vm.c
+++ b/drivers/gpu/drm/ttm/ttm_bo_vm.c
@@ -274,7 +274,8 @@ vm_fault_t ttm_bo_vm_fault_reserved(struct vm_fault *vmf,
                }

                address += PAGE_SIZE;
-               if (unlikely(++page_offset >= page_last))
+               if (unlikely(++page_offset >= page_last ||
+                            page_offset >= PFN_UP(bo->base.size)))
                        break;
        }
        return ret;

The above code git blame show this line was last modified in
ba4e7d973dd0.

Then in non-fixes patch, do it like you have it here.

Matt

>  
>  	if (unlikely(page_offset >= PFN_UP(bo->base.size)))
>  		return VM_FAULT_SIGBUS;
> -- 
> 2.50.1 (Apple Git-155)