[PATCH] drm/gem-dma: bound the mmap against the object size

Baul Lee <[email protected]>
Newsgroups org.freedesktop.lists.dri-devel,org.kernel.vger.linux-kernel,org.kernel.vger.stable
Message-ID <[email protected]>
drm_gem_dma_mmap() passes the caller's request, not the object, as the
size of the buffer being mapped:

	ret = dma_mmap_wc(drm_dev_dma_dev(dma_obj->base.dev), vma,
			  dma_obj->vaddr, dma_obj->dma_addr,
			  vma->vm_end - vma->vm_start);

dma_direct_mmap() derives its page count from that size, so count and
user_count are the same number and the bounds test degenerates into
"vm_pgoff must be zero":

	unsigned long user_count = vma_pages(vma);
	unsigned long count = PAGE_ALIGN(size) >> PAGE_SHIFT;
	...
	if (vma->vm_pgoff >= count || user_count > count - vma->vm_pgoff)
		return -ENXIO;

remap_pfn_range() then installs writable PTEs for every frame the caller
asked for, starting at the object and running past its end.

On the DRM node that is unreachable: drm_gem_mmap_obj() rejects a VMA
larger than the object before the object's mmap function runs.  The fbdev
emulation does not go through it.  drm_fbdev_dma_fb_mmap() calls
drm_gem_prime_mmap(), which invokes obj->funcs->mmap() directly, so
/dev/fb0 accepts a length that /dev/dri/card0 refuses for the same object.

The framebuffer is contiguous CMA inside system DRAM, so the excess
mapping covers kernel-owned RAM, readable and writable, at an offset the
caller picks.  On arm64 a 1 GiB mapping of an 8294400-byte framebuffer
object was accepted, and writes through it landed in the private memory
of another unprivileged process.  Nothing is logged: every access comes
from userspace through a PTE the kernel installed.  open() and mmap() on
/dev/fb0 are enough, so any member of group video reaches it.

Pass the object size to the DMA layer, which makes the dma_direct_mmap()
test meaningful, and check the VMA length the way drm_gem_mmap_obj() does
so both nodes reject the same request.

Fixes: b79fe9abd58b ("drm/fbdev-dma: Implement fbdev emulation for GEM DMA helpers")
Cc: [email protected]
Signed-off-by: Baul Lee <[email protected]>
---
 drivers/gpu/drm/drm_gem_dma_helper.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/drm_gem_dma_helper.c b/drivers/gpu/drm/drm_gem_dma_helper.c
--- a/drivers/gpu/drm/drm_gem_dma_helper.c
+++ b/drivers/gpu/drm/drm_gem_dma_helper.c
@@ -531,6 +531,9 @@ int drm_gem_dma_mmap(struct drm_gem_dma_object *dma_obj, struct vm_area_struct *
 	struct drm_gem_object *obj = &dma_obj->base;
 	int ret;

+	if (obj->size < vma->vm_end - vma->vm_start)
+		return -EINVAL;
+
 	/*
 	 * Clear the VM_PFNMAP flag that was set by drm_gem_mmap(), and set the
 	 * vm_pgoff (used as a fake buffer offset by DRM) to 0 as we want to map
@@ -543,12 +546,12 @@ int drm_gem_dma_mmap(struct drm_gem_dma_object *dma_obj, struct vm_area_struct *
 		vma->vm_page_prot = vm_get_page_prot(vma->vm_flags);

 		ret = dma_mmap_pages(drm_dev_dma_dev(dma_obj->base.dev),
-				     vma, vma->vm_end - vma->vm_start,
+				     vma, obj->size,
 				     virt_to_page(dma_obj->vaddr));
 	} else {
 		ret = dma_mmap_wc(drm_dev_dma_dev(dma_obj->base.dev), vma,
 				  dma_obj->vaddr, dma_obj->dma_addr,
-				  vma->vm_end - vma->vm_start);
+				  obj->size);
 	}
 	if (ret)
 		drm_gem_vm_close(vma);
-- 
2.50.1 (Apple Git-155)
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.