On 15.07.25 15:56, Simona Vetter wrote:
On Tue, Jul 15, 2025 at 03:43:08PM +0200, Christian König wrote:
We are about to revert this patch. Not sure if backporting it makes sense at the moment.
I think it still makes sense, at least as an interim fix.
What I discussed with Thomas is that first we want to revert back from gem_bo->dma_buf to gem_bo->import_attach.dmabuf everywhere. And those patches are sprinkled over various branches/trees/releases. So it'll take a while to catch them all. Meanwhile these two patches at least take care of the worst fallout, they're already tested and in the pipeline - I don't think it makes sense to hold them up and wait.
It might take us until 6.17-rc1 until we've caught all the trees and made sure the backports of those reverts have happened, and I don't think we should wait that long.
Works for me.
I just wanted to point out that the patch backported might be reverted again.
Christian.
Cheers, Sima
Regards, Christian.
On 15.07.25 15:13, Greg Kroah-Hartman wrote:
6.15-stable review patch. If anyone has any objections, please let me know.
From: Thomas Zimmermann tzimmermann@suse.de
commit 5307dce878d4126e1b375587318955bd019c3741 upstream.
A GEM handle can be released while the GEM buffer object is attached to a DRM framebuffer. This leads to the release of the dma-buf backing the buffer object, if any. [1] Trying to use the framebuffer in further mode-setting operations leads to a segmentation fault. Most easily happens with driver that use shadow planes for vmap-ing the dma-buf during a page flip. An example is shown below.
[ 156.791968] ------------[ cut here ]------------ [ 156.796830] WARNING: CPU: 2 PID: 2255 at drivers/dma-buf/dma-buf.c:1527 dma_buf_vmap+0x224/0x430 [...] [ 156.942028] RIP: 0010:dma_buf_vmap+0x224/0x430 [ 157.043420] Call Trace: [ 157.045898] <TASK> [ 157.048030] ? show_trace_log_lvl+0x1af/0x2c0 [ 157.052436] ? show_trace_log_lvl+0x1af/0x2c0 [ 157.056836] ? show_trace_log_lvl+0x1af/0x2c0 [ 157.061253] ? drm_gem_shmem_vmap+0x74/0x710 [ 157.065567] ? dma_buf_vmap+0x224/0x430 [ 157.069446] ? __warn.cold+0x58/0xe4 [ 157.073061] ? dma_buf_vmap+0x224/0x430 [ 157.077111] ? report_bug+0x1dd/0x390 [ 157.080842] ? handle_bug+0x5e/0xa0 [ 157.084389] ? exc_invalid_op+0x14/0x50 [ 157.088291] ? asm_exc_invalid_op+0x16/0x20 [ 157.092548] ? dma_buf_vmap+0x224/0x430 [ 157.096663] ? dma_resv_get_singleton+0x6d/0x230 [ 157.101341] ? __pfx_dma_buf_vmap+0x10/0x10 [ 157.105588] ? __pfx_dma_resv_get_singleton+0x10/0x10 [ 157.110697] drm_gem_shmem_vmap+0x74/0x710 [ 157.114866] drm_gem_vmap+0xa9/0x1b0 [ 157.118763] drm_gem_vmap_unlocked+0x46/0xa0 [ 157.123086] drm_gem_fb_vmap+0xab/0x300 [ 157.126979] drm_atomic_helper_prepare_planes.part.0+0x487/0xb10 [ 157.133032] ? lockdep_init_map_type+0x19d/0x880 [ 157.137701] drm_atomic_helper_commit+0x13d/0x2e0 [ 157.142671] ? drm_atomic_nonblocking_commit+0xa0/0x180 [ 157.147988] drm_mode_atomic_ioctl+0x766/0xe40 [...] [ 157.346424] ---[ end trace 0000000000000000 ]---
Acquiring GEM handles for the framebuffer's GEM buffer objects prevents this from happening. The framebuffer's cleanup later puts the handle references.
Commit 1a148af06000 ("drm/gem-shmem: Use dma_buf from GEM object instance") triggers the segmentation fault easily by using the dma-buf field more widely. The underlying issue with reference counting has been present before.
v2:
- acquire the handle instead of the BO (Christian)
- fix comment style (Christian)
- drop the Fixes tag (Christian)
- rename err_ gotos
- add missing Link tag
Suggested-by: Christian König christian.koenig@amd.com Signed-off-by: Thomas Zimmermann tzimmermann@suse.de Link: https://elixir.bootlin.com/linux/v6.15/source/drivers/gpu/drm/drm_gem.c#L241 # [1] Cc: Thomas Zimmermann tzimmermann@suse.de Cc: Anusha Srivatsa asrivats@redhat.com Cc: Christian König christian.koenig@amd.com Cc: Maarten Lankhorst maarten.lankhorst@linux.intel.com Cc: Maxime Ripard mripard@kernel.org Cc: Sumit Semwal sumit.semwal@linaro.org Cc: "Christian König" christian.koenig@amd.com Cc: linux-media@vger.kernel.org Cc: dri-devel@lists.freedesktop.org Cc: linaro-mm-sig@lists.linaro.org Cc: stable@vger.kernel.org Reviewed-by: Christian König christian.koenig@amd.com Link: https://lore.kernel.org/r/20250630084001.293053-1-tzimmermann@suse.de Signed-off-by: Greg Kroah-Hartman gregkh@linuxfoundation.org
drivers/gpu/drm/drm_gem.c | 44 ++++++++++++++++++++++++--- drivers/gpu/drm/drm_gem_framebuffer_helper.c | 16 +++++---- drivers/gpu/drm/drm_internal.h | 2 + 3 files changed, 51 insertions(+), 11 deletions(-)
--- a/drivers/gpu/drm/drm_gem.c +++ b/drivers/gpu/drm/drm_gem.c @@ -212,6 +212,35 @@ void drm_gem_private_object_fini(struct } EXPORT_SYMBOL(drm_gem_private_object_fini); +static void drm_gem_object_handle_get(struct drm_gem_object *obj) +{
- struct drm_device *dev = obj->dev;
- drm_WARN_ON(dev, !mutex_is_locked(&dev->object_name_lock));
- if (obj->handle_count++ == 0)
drm_gem_object_get(obj);+}
+/**
- drm_gem_object_handle_get_unlocked - acquire reference on user-space handles
- @obj: GEM object
- Acquires a reference on the GEM buffer object's handle. Required
- to keep the GEM object alive. Call drm_gem_object_handle_put_unlocked()
- to release the reference.
- */
+void drm_gem_object_handle_get_unlocked(struct drm_gem_object *obj) +{
- struct drm_device *dev = obj->dev;
- guard(mutex)(&dev->object_name_lock);
- drm_WARN_ON(dev, !obj->handle_count); /* first ref taken in create-tail helper */
- drm_gem_object_handle_get(obj);
+} +EXPORT_SYMBOL(drm_gem_object_handle_get_unlocked);
/**
- drm_gem_object_handle_free - release resources bound to userspace handles
- @obj: GEM object to clean up.
@@ -242,8 +271,14 @@ static void drm_gem_object_exported_dma_ } } -static void -drm_gem_object_handle_put_unlocked(struct drm_gem_object *obj) +/**
- drm_gem_object_handle_put_unlocked - releases reference on user-space handles
- @obj: GEM object
- Releases a reference on the GEM buffer object's handle. Possibly releases
- the GEM buffer object and associated dma-buf objects.
- */
+void drm_gem_object_handle_put_unlocked(struct drm_gem_object *obj) { struct drm_device *dev = obj->dev; bool final = false; @@ -268,6 +303,7 @@ drm_gem_object_handle_put_unlocked(struc if (final) drm_gem_object_put(obj); } +EXPORT_SYMBOL(drm_gem_object_handle_put_unlocked); /*
- Called at device or object close to release the file's
@@ -389,8 +425,8 @@ drm_gem_handle_create_tail(struct drm_fi int ret; WARN_ON(!mutex_is_locked(&dev->object_name_lock));
- if (obj->handle_count++ == 0)
drm_gem_object_get(obj);
- drm_gem_object_handle_get(obj);
/* * Get the user-visible handle using idr. Preload and perform --- a/drivers/gpu/drm/drm_gem_framebuffer_helper.c +++ b/drivers/gpu/drm/drm_gem_framebuffer_helper.c @@ -99,7 +99,7 @@ void drm_gem_fb_destroy(struct drm_frame unsigned int i; for (i = 0; i < fb->format->num_planes; i++)
drm_gem_object_put(fb->obj[i]);
drm_gem_object_handle_put_unlocked(fb->obj[i]);drm_framebuffer_cleanup(fb); kfree(fb); @@ -182,8 +182,10 @@ int drm_gem_fb_init_with_funcs(struct dr if (!objs[i]) { drm_dbg_kms(dev, "Failed to lookup GEM object\n"); ret = -ENOENT;
goto err_gem_object_put;
goto err_gem_object_handle_put_unlocked;drm_gem_object_handle_get_unlocked(objs[i]);drm_gem_object_put(objs[i]);min_size = (height - 1) * mode_cmd->pitches[i] + drm_format_info_min_pitch(info, i, width) @@ -193,22 +195,22 @@ int drm_gem_fb_init_with_funcs(struct dr drm_dbg_kms(dev, "GEM object size (%zu) smaller than minimum size (%u) for plane %d\n", objs[i]->size, min_size, i);
drm_gem_object_put(objs[i]);
drm_gem_object_handle_put_unlocked(objs[i]); ret = -EINVAL;
goto err_gem_object_put;
goto err_gem_object_handle_put_unlocked;ret = drm_gem_fb_init(dev, fb, mode_cmd, objs, i, funcs); if (ret)
goto err_gem_object_put;
goto err_gem_object_handle_put_unlocked;return 0; -err_gem_object_put: +err_gem_object_handle_put_unlocked: while (i > 0) { --i;
drm_gem_object_put(objs[i]);
drm_gem_object_handle_put_unlocked(objs[i]);} --- a/drivers/gpu/drm/drm_internal.h +++ b/drivers/gpu/drm/drm_internal.h @@ -161,6 +161,8 @@ void drm_sysfs_lease_event(struct drm_de /* drm_gem.c */ int drm_gem_init(struct drm_device *dev); +void drm_gem_object_handle_get_unlocked(struct drm_gem_object *obj); +void drm_gem_object_handle_put_unlocked(struct drm_gem_object *obj); int drm_gem_handle_create_tail(struct drm_file *file_priv, struct drm_gem_object *obj, u32 *handlep);