Re: [PATCH v3 2/6] drm/gem-shmem: Test for existence of page in mmap fault handler

[Boris Brezillon <boris.brezillon@collabora.com>]

On Mon,  9 Feb 2026 14:27:11 +0100
Thomas Zimmermann <tzimmermann@suse.de> wrote:

> Not having a page pointer in the mmap fault handler is an error. Test
> for this situation and return VM_FAULT_SIGBUS if so. Also replace several
> lookups of the page with a local variable.
> 
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>

Reviewed-by: Boris Brezillon <boris.brezillon@collabora.com>

> ---
>  drivers/gpu/drm/drm_gem_shmem_helper.c | 24 ++++++++++++------------
>  1 file changed, 12 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c b/drivers/gpu/drm/drm_gem_shmem_helper.c
> index 5bced7db0f1f..3ee54c24e535 100644
> --- a/drivers/gpu/drm/drm_gem_shmem_helper.c
> +++ b/drivers/gpu/drm/drm_gem_shmem_helper.c
> @@ -574,31 +574,31 @@ static vm_fault_t drm_gem_shmem_fault(struct vm_fault *vmf)
>  {
>  	struct vm_area_struct *vma = vmf->vma;
>  	struct drm_gem_object *obj = vma->vm_private_data;
> +	struct drm_device *dev = obj->dev;
>  	struct drm_gem_shmem_object *shmem = to_drm_gem_shmem_obj(obj);
>  	loff_t num_pages = obj->size >> PAGE_SHIFT;
> -	vm_fault_t ret;
> +	vm_fault_t ret = VM_FAULT_SIGBUS;
>  	struct page **pages = shmem->pages;
> -	pgoff_t page_offset;
> +	pgoff_t page_offset = vmf->pgoff - vma->vm_pgoff; /* page offset within VMA */
> +	struct page *page;
>  	unsigned long pfn;
>  
> -	/* Offset to faulty address in the VMA. */
> -	page_offset = vmf->pgoff - vma->vm_pgoff;
> -
>  	dma_resv_lock(obj->resv, NULL);
>  
> -	if (page_offset >= num_pages ||
> -	    drm_WARN_ON_ONCE(obj->dev, !shmem->pages) ||
> -	    shmem->madv < 0) {
> -		ret = VM_FAULT_SIGBUS;
> +	if (page_offset >= num_pages || drm_WARN_ON_ONCE(dev, !shmem->pages) ||
> +	    shmem->madv < 0)
> +		goto out;
> +
> +	page = pages[page_offset];
> +	if (drm_WARN_ON_ONCE(dev, !page))
>  		goto out;
> -	}
>  
> -	if (drm_gem_shmem_try_map_pmd(vmf, vmf->address, pages[page_offset])) {
> +	if (drm_gem_shmem_try_map_pmd(vmf, vmf->address, page)) {
>  		ret = VM_FAULT_NOPAGE;
>  		goto out;
>  	}
>  
> -	pfn = page_to_pfn(pages[page_offset]);
> +	pfn = page_to_pfn(page);
>  	ret = vmf_insert_pfn(vma, vmf->address, pfn);
>  
>   out: