Re: [PATCH] drm/shmem-helper: Fix BUG_ON() on mmap(PROT_WRITE, MAP_PRIVATE)

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, May 20, 2024 at 12:05:14PM +0200, Jacek Lawrynowicz wrote:
> From: "Wachowski, Karol" <karol.wachowski@xxxxxxxxx>
> 
> Lack of check for copy-on-write (COW) mapping in drm_gem_shmem_mmap
> allows users to call mmap with PROT_WRITE and MAP_PRIVATE flag
> causing a kernel panic due to BUG_ON in vmf_insert_pfn_prot:
> BUG_ON((vma->vm_flags & VM_PFNMAP) && is_cow_mapping(vma->vm_flags));
> 
> Return -EINVAL early if COW mapping is detected.
> 
> This bug affects all drm drivers using default shmem helpers.
> It can be reproduced by this simple example:
> void *ptr = mmap(0, size, PROT_WRITE, MAP_PRIVATE, fd, mmap_offset);
> ptr[0] = 0;
> 
> Fixes: 2194a63a818d ("drm: Add library for shmem backed GEM objects")
> Cc: Noralf Trønnes <noralf@xxxxxxxxxxx>
> Cc: Eric Anholt <eric@xxxxxxxxxx>
> Cc: Rob Herring <robh@xxxxxxxxxx>
> Cc: Maarten Lankhorst <maarten.lankhorst@xxxxxxxxxxxxxxx>
> Cc: Maxime Ripard <mripard@xxxxxxxxxx>
> Cc: Thomas Zimmermann <tzimmermann@xxxxxxx>
> Cc: David Airlie <airlied@xxxxxxxxx>
> Cc: Daniel Vetter <daniel@xxxxxxxx>
> Cc: dri-devel@xxxxxxxxxxxxxxxxxxxxx
> Cc: <stable@xxxxxxxxxxxxxxx> # v5.2+
> Signed-off-by: Wachowski, Karol <karol.wachowski@xxxxxxxxx>
> Signed-off-by: Jacek Lawrynowicz <jacek.lawrynowicz@xxxxxxxxxxxxxxx>

Excellent catch!

Reviewed-by: Daniel Vetter <daniel.vetter@xxxxxxxx>

I reviewed the other helpers, and ttm/vram helpers already block this with
the check in ttm_bo_mmap_obj.

But the dma helpers does not, because the remap_pfn_range that underlies
the various dma_mmap* function (at least on most platforms) allows some
limited use of cow. But it makes no sense at all to all that only for
gpu buffer objects backed by specific allocators.

Would you be up for the 2nd patch that also adds this check to
drm_gem_dma_mmap, so that we have a consistent uapi?

I'll go ahead and apply this one to drm-misc-fixes meanwhile.

Thanks, Sima

> ---
>  drivers/gpu/drm/drm_gem_shmem_helper.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c b/drivers/gpu/drm/drm_gem_shmem_helper.c
> index 177773bcdbfd..885a62c2e1be 100644
> --- a/drivers/gpu/drm/drm_gem_shmem_helper.c
> +++ b/drivers/gpu/drm/drm_gem_shmem_helper.c
> @@ -611,6 +611,9 @@ int drm_gem_shmem_mmap(struct drm_gem_shmem_object *shmem, struct vm_area_struct
>  		return ret;
>  	}
>  
> +	if (is_cow_mapping(vma->vm_flags))
> +		return -EINVAL;
> +
>  	dma_resv_lock(shmem->base.resv, NULL);
>  	ret = drm_gem_shmem_get_pages(shmem);
>  	dma_resv_unlock(shmem->base.resv);
> -- 
> 2.45.1
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch




[Index of Archives]     [Linux Kernel]     [Kernel Development Newbies]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Hiking]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux