All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/gem-dma: fix double GEM object put on the mmap error path
@ 2026-08-05 12:44 Baul Lee
  2026-08-05 13:07 ` sashiko-bot
  0 siblings, 1 reply; 2+ messages in thread
From: Baul Lee @ 2026-08-05 12:44 UTC (permalink / raw)
  To: maarten.lankhorst, mripard, tzimmermann, airlied, simona
  Cc: dri-devel, linux-kernel, stable, Baul Lee

drm_gem_dma_mmap() drops a GEM reference when the DMA mapping fails:

	if (ret)
		drm_gem_vm_close(vma);

drm_gem_vm_close() puts vma->vm_private_data.  drm_gem_mmap_obj() has
already pointed that at the object, and puts it again when the callback
returns an error:

	drm_gem_object_get(obj);
	vma->vm_private_data = obj;
	...
	ret = obj->funcs->mmap(obj, vma);
	if (ret)
		goto err_drm_gem_object_put;

One get, two puts: a failing dma_mmap_wc() or dma_mmap_pages() underflows
the reference count and can free the object while the caller still holds
it.

The callee does not own that reference.  drm_gem_shmem_mmap() returns the
error and leaves the put to the caller, and both callers do it,
drm_gem_mmap_obj() as above and drm_gem_prime_mmap() from its own error
path.  Drop the call.

It was harmless until commit f49a51bfdc8e ("drm/shme-helpers: Fix
dma_buf_mmap forwarding bug") moved the vm_private_data assignment ahead
of the callback; before that the field was still NULL when the callback
ran and drm_gem_vm_close() put nothing.

exynos_drm_gem_mmap() and __tegra_gem_mmap() have the same error path.

Fixes: f49a51bfdc8e ("drm/shme-helpers: Fix dma_buf_mmap forwarding bug")
Cc: stable@vger.kernel.org
Signed-off-by: Baul Lee <baul.lee@xbow.com>
---
 drivers/gpu/drm/drm_gem_dma_helper.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/gpu/drm/drm_gem_dma_helper.c b/drivers/gpu/drm/drm_gem_dma_helper.c
index 1c00a71ab3c9..a34561efd1ae 100644
--- a/drivers/gpu/drm/drm_gem_dma_helper.c
+++ b/drivers/gpu/drm/drm_gem_dma_helper.c
@@ -550,8 +550,6 @@ int drm_gem_dma_mmap(struct drm_gem_dma_object *dma_obj, struct vm_area_struct *
 				  dma_obj->vaddr, dma_obj->dma_addr,
 				  vma->vm_end - vma->vm_start);
 	}
-	if (ret)
-		drm_gem_vm_close(vma);
 
 	return ret;
 }
-- 
2.50.1 (Apple Git-155)

^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH] drm/gem-dma: fix double GEM object put on the mmap error path
  2026-08-05 12:44 [PATCH] drm/gem-dma: fix double GEM object put on the mmap error path Baul Lee
@ 2026-08-05 13:07 ` sashiko-bot
  0 siblings, 0 replies; 2+ messages in thread
From: sashiko-bot @ 2026-08-05 13:07 UTC (permalink / raw)
  To: Baul Lee; +Cc: dri-devel

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Double-put causing Use-After-Free and memory leak on mmap paths in exynos_drm_gem_mmap()
--

--- Patch [1]: [PATCH] drm/gem-dma: fix double GEM object put on the mmap error path ---
commit 381f3566395605c2a5479094f795c8569a814b8a
Author: Baul Lee <baul.lee@xbow.com>

drm/gem-dma: fix double GEM object put on the mmap error path

This commit removes drm_gem_vm_close() from the error path of
drm_gem_dma_mmap() to prevent a double-put of the GEM object. The caller
drm_gem_mmap_obj() already drops the reference on error, so the extra
put resulted in an underflow and use-after-free.

> exynos_drm_gem_mmap() and __tegra_gem_mmap() have the same error path.

[Severity: High]
This isn't a bug introduced by this patch, but since the commit message
mentions exynos_drm_gem_mmap() having the same error path, looking at that
function:

drivers/gpu/drm/exynos/exynos_drm_gem.c:exynos_drm_gem_mmap() {
    ...
	ret = exynos_drm_gem_mmap_buffer(exynos_gem, vma);
	if (ret)
		goto err_close_vm;

	return ret;

err_close_vm:
	drm_gem_vm_close(vma);

	return ret;
}

Can this cause a double-put leading to a use-after-free?

When returning an error, this explicitly calls drm_gem_vm_close(vma) which
drops a reference. The caller drm_gem_mmap_obj() will see the error and drop
the reference again, causing a double-put.

[Severity: High]
This is also a pre-existing issue, but looking at the import path earlier in
the same function:

drivers/gpu/drm/exynos/exynos_drm_gem.c:exynos_drm_gem_mmap() {
    ...
	if (obj->import_attach)
		return dma_buf_mmap(obj->dma_buf, vma, 0);
    ...
}

Does this code leak the GEM object?

On success, dma_buf_mmap() replaces vma->vm_ops and vma->vm_private_data
with the exporter's state without dropping the surplus reference acquired
by drm_gem_mmap_obj(). This could cause a memory leak when the VMA is
torn down.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260805124442.57426-1-baul.lee@xbow.com?part=1

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2026-08-05 13:07 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-05 12:44 [PATCH] drm/gem-dma: fix double GEM object put on the mmap error path Baul Lee
2026-08-05 13:07 ` sashiko-bot

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.