On Tue, 21 May 2024 at 14:38, Daniel Vetter daniel@ffwll.ch wrote:

On Mon, May 20, 2024 at 12:05:14PM +0200, Jacek Lawrynowicz wrote:

...

From: "Wachowski, Karol" karol.wachowski@intel.com

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@tronnes.org Cc: Eric Anholt eric@anholt.net Cc: Rob Herring robh@kernel.org Cc: Maarten Lankhorst maarten.lankhorst@linux.intel.com Cc: Maxime Ripard mripard@kernel.org Cc: Thomas Zimmermann tzimmermann@suse.de Cc: David Airlie airlied@gmail.com Cc: Daniel Vetter daniel@ffwll.ch Cc: dri-devel@lists.freedesktop.org Cc: stable@vger.kernel.org # v5.2+ Signed-off-by: Wachowski, Karol karol.wachowski@intel.com Signed-off-by: Jacek Lawrynowicz jacek.lawrynowicz@linux.intel.com

Excellent catch!

Reviewed-by: Daniel Vetter daniel.vetter@ffwll.ch

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.

Forgot to add: A testcase in igt would also be really lovely.

https://dri.freedesktop.org/docs/drm/gpu/drm-uapi.html#validating-changes-wi... -Sima

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