[PATCH v2] drm/gem: Acquire references on GEM handles for framebuffers
Thomas Zimmermann
tzimmermann at suse.de
Mon Jun 30 11:34:57 UTC 2025
Hi
Am 30.06.25 um 10:49 schrieb Christian König:
> On 30.06.25 10:36, Thomas Zimmermann wrote:
>> 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 at amd.com>
>> Signed-off-by: Thomas Zimmermann <tzimmermann at suse.de>
> Reviewed-by: Christian König <christian.koenig at amd.com>
Thanks a lot
>
> But I strongly suggest to let the different CI systems take a look as well, we already had to much fun with that.
I can wait a bit longer for reports, but the patch fixes a regression in
v6.15. I'd rather see it merged soon-ish.
Best regards
Thomas
>
> Regards,
> Christian.
>
>> Link: https://elixir.bootlin.com/linux/v6.15/source/drivers/gpu/drm/drm_gem.c#L241 # [1]
>> Cc: Thomas Zimmermann <tzimmermann at suse.de>
>> Cc: Anusha Srivatsa <asrivats at redhat.com>
>> Cc: Christian König <christian.koenig at amd.com>
>> Cc: Maarten Lankhorst <maarten.lankhorst at linux.intel.com>
>> Cc: Maxime Ripard <mripard at kernel.org>
>> Cc: Sumit Semwal <sumit.semwal at linaro.org>
>> Cc: "Christian König" <christian.koenig at amd.com>
>> Cc: linux-media at vger.kernel.org
>> Cc: dri-devel at lists.freedesktop.org
>> Cc: linaro-mm-sig at lists.linaro.org
>> Cc: <stable at vger.kernel.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(-)
>>
>> diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
>> index 19d50d254fe6..bc505d938b3e 100644
>> --- a/drivers/gpu/drm/drm_gem.c
>> +++ b/drivers/gpu/drm/drm_gem.c
>> @@ -213,6 +213,35 @@ void drm_gem_private_object_fini(struct drm_gem_object *obj)
>> }
>> 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.
>> @@ -243,8 +272,14 @@ static void drm_gem_object_exported_dma_buf_free(struct drm_gem_object *obj)
>> }
>> }
>>
>> -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;
>> @@ -269,6 +304,7 @@ drm_gem_object_handle_put_unlocked(struct drm_gem_object *obj)
>> 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
>> @@ -390,8 +426,8 @@ drm_gem_handle_create_tail(struct drm_file *file_priv,
>> 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
>> diff --git a/drivers/gpu/drm/drm_gem_framebuffer_helper.c b/drivers/gpu/drm/drm_gem_framebuffer_helper.c
>> index 618ce725cd75..c60d0044d036 100644
>> --- a/drivers/gpu/drm/drm_gem_framebuffer_helper.c
>> +++ b/drivers/gpu/drm/drm_gem_framebuffer_helper.c
>> @@ -100,7 +100,7 @@ void drm_gem_fb_destroy(struct drm_framebuffer *fb)
>> 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);
>> @@ -183,8 +183,10 @@ int drm_gem_fb_init_with_funcs(struct drm_device *dev,
>> 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)
>> @@ -194,22 +196,22 @@ int drm_gem_fb_init_with_funcs(struct drm_device *dev,
>> 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]);
>> }
>> return ret;
>> }
>> diff --git a/drivers/gpu/drm/drm_internal.h b/drivers/gpu/drm/drm_internal.h
>> index 442eb31351dd..f7b414a813ae 100644
>> --- 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_device *dev);
>>
>> /* 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);
--
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)
More information about the dri-devel
mailing list