[PATCH v2 2/2] drm/shmem-helper: Don't remove the offset in vm_area_struct pgoff

[Steven Price]

On 23/02/2021 15:51, Neil Roberts wrote:
> When mmapping the shmem, it would previously adjust the pgoff in the
> vm_area_struct to remove the fake offset that is added to be able to
> identify the buffer. This patch removes the adjustment and makes the
> fault handler use the vm_fault address to calculate the page offset
> instead. Although using this address is apparently discouraged, several
> DRM drivers seem to be doing it anyway.
> 
> The problem with removing the pgoff is that it prevents
> drm_vma_node_unmap from working because that searches the mapping tree
> by address. That doesn't work because all of the mappings are at offset
> 0. drm_vma_node_unmap is being used by the shmem helpers when purging
> the buffer.
> 
> This fixes a bug in Panfrost which is using drm_gem_shmem_purge. Without
> this the mapping for the purged buffer can still be accessed which might
> mean it would access random pages from other buffers
> 
> v2: Don't check whether the unsigned page_offset is less than 0.
> 
> Cc: stable at vger.kernel.org
> Fixes: 17acb9f35ed7 ("drm/shmem: Add madvise state and purge helpers")
> Signed-off-by: Neil Roberts <nroberts at igalia.com>

Reviewed-by: Steven Price <steven.price at arm.com>

> ---
>   drivers/gpu/drm/drm_gem_shmem_helper.c | 11 ++++++-----
>   1 file changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c b/drivers/gpu/drm/drm_gem_shmem_helper.c
> index b26139b1dc35..5b5c095e86a9 100644
> --- a/drivers/gpu/drm/drm_gem_shmem_helper.c
> +++ b/drivers/gpu/drm/drm_gem_shmem_helper.c
> @@ -527,15 +527,19 @@ static vm_fault_t drm_gem_shmem_fault(struct vm_fault *vmf)
>   	loff_t num_pages = obj->size >> PAGE_SHIFT;
>   	vm_fault_t ret;
>   	struct page *page;
> +	pgoff_t page_offset;
> +
> +	/* We don't use vmf->pgoff since that has the fake offset */
> +	page_offset = (vmf->address - vma->vm_start) >> PAGE_SHIFT;
>   
>   	mutex_lock(&shmem->pages_lock);
>   
> -	if (vmf->pgoff >= num_pages ||
> +	if (page_offset >= num_pages ||
>   	    WARN_ON_ONCE(!shmem->pages) ||
>   	    shmem->madv < 0) {
>   		ret = VM_FAULT_SIGBUS;
>   	} else {
> -		page = shmem->pages[vmf->pgoff];
> +		page = shmem->pages[page_offset];
>   
>   		ret = vmf_insert_page(vma, vmf->address, page);
>   	}
> @@ -591,9 +595,6 @@ int drm_gem_shmem_mmap(struct drm_gem_object *obj, struct vm_area_struct *vma)
>   	struct drm_gem_shmem_object *shmem;
>   	int ret;
>   
> -	/* Remove the fake offset */
> -	vma->vm_pgoff -= drm_vma_node_start(&obj->vma_node);
> -
>   	if (obj->import_attach) {
>   		/* Drop the reference drm_gem_mmap_obj() acquired.*/
>   		drm_gem_object_put(obj);
>