* Re: [PATCH v3 1/2] drm/drm_crtc: ensure dma_fence_ops remain valid during device unbind
2026-07-21 8:21 ` [PATCH v3 1/2] drm/drm_crtc: ensure dma_fence_ops remain valid during device unbind André Draszik
@ 2026-07-21 11:20 ` Philipp Stanner
2026-08-03 12:26 ` Christian König
2026-09-15 8:18 ` Tvrtko Ursulin
2026-10-20 7:14 ` Philipp Stanner
2 siblings, 1 reply; 9+ messages in thread
From: Philipp Stanner @ 2026-07-21 11:20 UTC (permalink / raw)
To: André Draszik, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Simona Vetter, Sumit Semwal,
Christian König, Tvrtko Ursulin, Boris Brezillon,
Philipp Stanner, Danilo Krummrich, Sean Paul, Gustavo Padovan
Cc: dri-devel, linux-kernel, linux-media, linaro-mm-sig,
Peter Griffin, Tudor Ambarus, Juan Yescas, kernel-team,
Simona Vetter, stable
On Tue, 2026-07-21 at 09:21 +0100, André Draszik wrote:
> In [1], sashiko reported the following issue:
>
> === snip ===
> Looking at how these fences are managed, drm_crtc_create_fence()
> creates a dma_fence without taking a reference to the drm_device or
> drm_crtc. Because the sync_file framework exposes this fence to
> userspace, the fence can outlive the CRTC.
>
> The dma_fence contract requires that data accessed by dma_fence_ops
> (like get_driver_name) must remain valid for an RCU grace period after
> the fence is signaled. However, drm_crtc_cleanup() and the subsequent
> freeing of the device do not wait for an RCU grace period via
> synchronize_rcu().
>
> If userspace calls ioctl(SYNC_IOC_FILE_INFO) concurrently with a device
> hot-unplug:
>
> CPU1 (Userspace)
> sync_file_get_name()
> ops = rcu_dereference(fence->ops);
> if (!dma_fence_test_signaled_flag())
> // Preempted or delayed here
nit: no one will be preempted here since the RCU read lock must be
held. The Sashiko tool misses the point, which is simply that someone
illegally frees up stuff that might be still in use, with or without
delay or preemption, that's all irrelevant for the issue.
Anyways, thanks for fixing this:
>
>
[…]
> Link: https://sashiko.dev/#/patchset/20260618-linux-drm_crtc_fix2-v1-1-c03e77b36f34@linaro.org?part=1
> Fixes: 6d6003c4b613 ("drm/fence: add fence timeline to drm_crtc")
> Cc: stable@vger.kernel.org
> Signed-off-by: André Draszik <andre.draszik@linaro.org>
Reviewed-by: Philipp Stanner <phasta@kernel.org>
>
> ---
> v3:
> - Philipp: update kerneldoc, add Fixes:
>
> v2: new patch
> ---
> drivers/gpu/drm/drm_crtc.c | 15 ++++++++++++---
> 1 file changed, 12 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> index 63ead8ba6756..e8e80c936852 100644
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
> @@ -493,14 +493,23 @@ EXPORT_SYMBOL(__drmm_crtc_alloc_with_planes);
> * drm_crtc_cleanup - Clean up the core crtc usage
> * @crtc: CRTC to cleanup
> *
> - * This function cleans up @crtc and removes it from the DRM mode setting
> - * core. Note that the function does *not* free the crtc structure itself,
> - * this is the responsibility of the caller.
> + * This function cleans up @crtc and removes it from the DRM mode setting core,
> + * after first waiting an RCU grace period to ensure @crtc->dev can safely be
> + * dereferenced by our dma_fence_ops.
> + *
> + * Note that the function does *not* free the crtc structure itself, this is the
> + * responsibility of the caller.
> */
> void drm_crtc_cleanup(struct drm_crtc *crtc)
> {
> struct drm_device *dev = crtc->dev;
>
> + /* Ensure our dma_fence_ops remain valid for an RCU grace period after
> + * the fence is signaled. This is necessary because our dma_fence_ops
> + * dereference crtc->dev.
> + */
> + synchronize_rcu();
> +
> /* Note that the crtc_list is considered to be static; should we
> * remove the drm_crtc at runtime we would have to decrement all
> * the indices on the drm_crtc after us in the crtc_list.
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH v3 1/2] drm/drm_crtc: ensure dma_fence_ops remain valid during device unbind
2026-07-21 11:20 ` Philipp Stanner
@ 2026-08-03 12:26 ` Christian König
0 siblings, 0 replies; 9+ messages in thread
From: Christian König @ 2026-08-03 12:26 UTC (permalink / raw)
To: phasta, André Draszik, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Simona Vetter, Sumit Semwal,
Tvrtko Ursulin, Boris Brezillon, Danilo Krummrich, Sean Paul,
Gustavo Padovan
Cc: dri-devel, linux-kernel, linux-media, linaro-mm-sig,
Peter Griffin, Tudor Ambarus, Juan Yescas, kernel-team,
Simona Vetter, stable
On 7/21/26 13:20, Philipp Stanner wrote:
> On Tue, 2026-07-21 at 09:21 +0100, André Draszik wrote:
>> In [1], sashiko reported the following issue:
>>
>> === snip ===
>> Looking at how these fences are managed, drm_crtc_create_fence()
>> creates a dma_fence without taking a reference to the drm_device or
>> drm_crtc. Because the sync_file framework exposes this fence to
>> userspace, the fence can outlive the CRTC.
>>
>> The dma_fence contract requires that data accessed by dma_fence_ops
>> (like get_driver_name) must remain valid for an RCU grace period after
>> the fence is signaled. However, drm_crtc_cleanup() and the subsequent
>> freeing of the device do not wait for an RCU grace period via
>> synchronize_rcu().
>>
>> If userspace calls ioctl(SYNC_IOC_FILE_INFO) concurrently with a device
>> hot-unplug:
>>
>> CPU1 (Userspace)
>> sync_file_get_name()
>> ops = rcu_dereference(fence->ops);
>> if (!dma_fence_test_signaled_flag())
>> // Preempted or delayed here
>
> nit: no one will be preempted here since the RCU read lock must be
> held. The Sashiko tool misses the point, which is simply that someone
> illegally frees up stuff that might be still in use, with or without
> delay or preemption, that's all irrelevant for the issue.
>
> Anyways, thanks for fixing this:
Seconded.
>
>>
>>
>
> […]
>
>> Link: https://sashiko.dev/#/patchset/20260618-linux-drm_crtc_fix2-v1-1-c03e77b36f34@linaro.org?part=1
>> Fixes: 6d6003c4b613 ("drm/fence: add fence timeline to drm_crtc")
>> Cc: stable@vger.kernel.org
>> Signed-off-by: André Draszik <andre.draszik@linaro.org>
>
> Reviewed-by: Philipp Stanner <phasta@kernel.org>
Reviewed-by: Christian König <christian.koenig@amd.com> for both patches.
>
>>
>> ---
>> v3:
>> - Philipp: update kerneldoc, add Fixes:
>>
>> v2: new patch
>> ---
>> drivers/gpu/drm/drm_crtc.c | 15 ++++++++++++---
>> 1 file changed, 12 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
>> index 63ead8ba6756..e8e80c936852 100644
>> --- a/drivers/gpu/drm/drm_crtc.c
>> +++ b/drivers/gpu/drm/drm_crtc.c
>> @@ -493,14 +493,23 @@ EXPORT_SYMBOL(__drmm_crtc_alloc_with_planes);
>> * drm_crtc_cleanup - Clean up the core crtc usage
>> * @crtc: CRTC to cleanup
>> *
>> - * This function cleans up @crtc and removes it from the DRM mode setting
>> - * core. Note that the function does *not* free the crtc structure itself,
>> - * this is the responsibility of the caller.
>> + * This function cleans up @crtc and removes it from the DRM mode setting core,
>> + * after first waiting an RCU grace period to ensure @crtc->dev can safely be
>> + * dereferenced by our dma_fence_ops.
>> + *
>> + * Note that the function does *not* free the crtc structure itself, this is the
>> + * responsibility of the caller.
>> */
>> void drm_crtc_cleanup(struct drm_crtc *crtc)
>> {
>> struct drm_device *dev = crtc->dev;
>>
>> + /* Ensure our dma_fence_ops remain valid for an RCU grace period after
>> + * the fence is signaled. This is necessary because our dma_fence_ops
>> + * dereference crtc->dev.
>> + */
>> + synchronize_rcu();
>> +
>> /* Note that the crtc_list is considered to be static; should we
>> * remove the drm_crtc at runtime we would have to decrement all
>> * the indices on the drm_crtc after us in the crtc_list.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v3 1/2] drm/drm_crtc: ensure dma_fence_ops remain valid during device unbind
2026-07-21 8:21 ` [PATCH v3 1/2] drm/drm_crtc: ensure dma_fence_ops remain valid during device unbind André Draszik
2026-07-21 11:20 ` Philipp Stanner
@ 2026-09-15 8:18 ` Tvrtko Ursulin
2026-10-20 7:14 ` Philipp Stanner
2 siblings, 0 replies; 9+ messages in thread
From: Tvrtko Ursulin @ 2026-09-15 8:18 UTC (permalink / raw)
To: André Draszik, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Simona Vetter, Sumit Semwal,
Christian König, Boris Brezillon, Philipp Stanner,
Danilo Krummrich, Sean Paul, Gustavo Padovan
Cc: dri-devel, linux-kernel, linux-media, linaro-mm-sig,
Peter Griffin, Tudor Ambarus, Juan Yescas, kernel-team,
Simona Vetter, stable
On 21/07/2026 09:21, André Draszik wrote:
> In [1], sashiko reported the following issue:
>
> === snip ===
> Looking at how these fences are managed, drm_crtc_create_fence()
> creates a dma_fence without taking a reference to the drm_device or
> drm_crtc. Because the sync_file framework exposes this fence to
> userspace, the fence can outlive the CRTC.
>
> The dma_fence contract requires that data accessed by dma_fence_ops
> (like get_driver_name) must remain valid for an RCU grace period after
> the fence is signaled. However, drm_crtc_cleanup() and the subsequent
> freeing of the device do not wait for an RCU grace period via
> synchronize_rcu().
>
> If userspace calls ioctl(SYNC_IOC_FILE_INFO) concurrently with a device
> hot-unplug:
>
> CPU1 (Userspace)
> sync_file_get_name()
> ops = rcu_dereference(fence->ops);
> if (!dma_fence_test_signaled_flag())
> // Preempted or delayed here
>
> CPU2 (Driver Teardown)
> Signals the fence (setting fence->ops = NULL)
> Destroys and frees the CRTC without waiting for an RCU grace period
>
> CPU1 (Resumes)
> ops->get_driver_name(fence) -> drm_crtc_fence_get_driver_name()
> crtc = fence_to_crtc(fence); // Casts to the freed CRTC
> return crtc->dev->driver->name; // Use-after-free
>
> ...
>
> Does the CRTC or DRM device need to be kept alive for the RCU grace
> period, or should the fence hold a proper reference to prevent the
> use-after-free when get_driver_name() and get_timeline_name() access
> the freed CRTC structure?
> === snap ===
>
> I believe this to be a correct observation and this patch implements
> the suggestion of waiting for an RCU grace period before proceeding
> with destruction of the drm_crtc, so that get_driver_name() and
> get_timeline_name() can still work.
Presumably these fence are exportable via syncobj and/or sync file? If
so, your analysis seems correct to me. Fix as well looks it works to
close the vulnerability, but the part I cannot help with, and which will
need someone more familiar with the CRTC code and the associated object
lifetimes, is to comment on whether a RCU delay in drm_crtc_cleanup() is
acceptable or it must be made async one way or the other.
Regards,
Tvrtko
> Link: https://sashiko.dev/#/patchset/20260618-linux-drm_crtc_fix2-v1-1-c03e77b36f34@linaro.org?part=1
> Fixes: 6d6003c4b613 ("drm/fence: add fence timeline to drm_crtc")
> Cc: stable@vger.kernel.org
> Signed-off-by: André Draszik <andre.draszik@linaro.org>
>
> ---
> v3:
> - Philipp: update kerneldoc, add Fixes:
>
> v2: new patch
> ---
> drivers/gpu/drm/drm_crtc.c | 15 ++++++++++++---
> 1 file changed, 12 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> index 63ead8ba6756..e8e80c936852 100644
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
> @@ -493,14 +493,23 @@ EXPORT_SYMBOL(__drmm_crtc_alloc_with_planes);
> * drm_crtc_cleanup - Clean up the core crtc usage
> * @crtc: CRTC to cleanup
> *
> - * This function cleans up @crtc and removes it from the DRM mode setting
> - * core. Note that the function does *not* free the crtc structure itself,
> - * this is the responsibility of the caller.
> + * This function cleans up @crtc and removes it from the DRM mode setting core,
> + * after first waiting an RCU grace period to ensure @crtc->dev can safely be
> + * dereferenced by our dma_fence_ops.
> + *
> + * Note that the function does *not* free the crtc structure itself, this is the
> + * responsibility of the caller.
> */
> void drm_crtc_cleanup(struct drm_crtc *crtc)
> {
> struct drm_device *dev = crtc->dev;
>
> + /* Ensure our dma_fence_ops remain valid for an RCU grace period after
> + * the fence is signaled. This is necessary because our dma_fence_ops
> + * dereference crtc->dev.
> + */
> + synchronize_rcu();
> +
> /* Note that the crtc_list is considered to be static; should we
> * remove the drm_crtc at runtime we would have to decrement all
> * the indices on the drm_crtc after us in the crtc_list.
>
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH v3 1/2] drm/drm_crtc: ensure dma_fence_ops remain valid during device unbind
2026-07-21 8:21 ` [PATCH v3 1/2] drm/drm_crtc: ensure dma_fence_ops remain valid during device unbind André Draszik
2026-07-21 11:20 ` Philipp Stanner
2026-09-15 8:18 ` Tvrtko Ursulin
@ 2026-10-20 7:14 ` Philipp Stanner
2026-10-20 11:19 ` Christian König
2 siblings, 1 reply; 9+ messages in thread
From: Philipp Stanner @ 2026-10-20 7:14 UTC (permalink / raw)
To: André Draszik, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Simona Vetter, Sumit Semwal,
Christian König, Tvrtko Ursulin, Boris Brezillon,
Philipp Stanner, Danilo Krummrich, Sean Paul, Gustavo Padovan
Cc: dri-devel, linux-kernel, linux-media, linaro-mm-sig,
Peter Griffin, Tudor Ambarus, Juan Yescas, kernel-team,
Simona Vetter, stable
On Tue, 2026-07-21 at 09:21 +0100, André Draszik wrote:
>
[…]
>
>
> ---
> v3:
> - Philipp: update kerneldoc, add Fixes:
>
> v2: new patch
> ---
> drivers/gpu/drm/drm_crtc.c | 15 ++++++++++++---
> 1 file changed, 12 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> index 63ead8ba6756..e8e80c936852 100644
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
> @@ -493,14 +493,23 @@ EXPORT_SYMBOL(__drmm_crtc_alloc_with_planes);
> * drm_crtc_cleanup - Clean up the core crtc usage
> * @crtc: CRTC to cleanup
> *
> - * This function cleans up @crtc and removes it from the DRM mode setting
> - * core. Note that the function does *not* free the crtc structure itself,
> - * this is the responsibility of the caller.
> + * This function cleans up @crtc and removes it from the DRM mode setting core,
> + * after first waiting an RCU grace period to ensure @crtc->dev can safely be
> + * dereferenced by our dma_fence_ops.
> + *
> + * Note that the function does *not* free the crtc structure itself, this is the
> + * responsibility of the caller.
> */
> void drm_crtc_cleanup(struct drm_crtc *crtc)
> {
> struct drm_device *dev = crtc->dev;
>
> + /* Ensure our dma_fence_ops remain valid for an RCU grace period after
> + * the fence is signaled. This is necessary because our dma_fence_ops
> + * dereference crtc->dev.
> + */
> + synchronize_rcu();
> +
> /* Note that the crtc_list is considered to be static; should we
> * remove the drm_crtc at runtime we would have to decrement all
> * the indices on the drm_crtc after us in the crtc_list.
Seems we forgot [1] this patch. Sorry about that.
This fix also looks fine to me (for now) since it's what the dma_fence
contract currently demands.
I'm a bit nervous about a synchronize_rcu() being in every cleanup
path, but I suspect it's the most reasonable thing to do now.
Let's wait a day or two to see if anyone has fundamental objections.
Could someone with some CRTC knowledge give it an R-b? Otherwise I'd
push it this week.
Regards,
Philipp
[1] https://lore.kernel.org/dri-devel/73fc12734386c9bebc9cf6d3cea4c5f8e125255e.camel@mailbox.org/T/#thttps://lore.kernel.org/dri-devel/73fc12734386c9bebc9cf6d3cea4c5f8e125255e.camel@mailbox.org/T/#t
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH v3 1/2] drm/drm_crtc: ensure dma_fence_ops remain valid during device unbind
2026-10-20 7:14 ` Philipp Stanner
@ 2026-10-20 11:19 ` Christian König
0 siblings, 0 replies; 9+ messages in thread
From: Christian König @ 2026-10-20 11:19 UTC (permalink / raw)
To: phasta, André Draszik, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Simona Vetter, Sumit Semwal,
Tvrtko Ursulin, Boris Brezillon, Danilo Krummrich, Sean Paul,
Gustavo Padovan
Cc: dri-devel, linux-kernel, linux-media, linaro-mm-sig,
Peter Griffin, Tudor Ambarus, Juan Yescas, kernel-team,
Simona Vetter, stable
On 10/20/26 09:14, Philipp Stanner wrote:
> On Tue, 2026-07-21 at 09:21 +0100, André Draszik wrote:
>>
>
> […]
>
>>
>>
>> ---
>> v3:
>> - Philipp: update kerneldoc, add Fixes:
>>
>> v2: new patch
>> ---
>> drivers/gpu/drm/drm_crtc.c | 15 ++++++++++++---
>> 1 file changed, 12 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
>> index 63ead8ba6756..e8e80c936852 100644
>> --- a/drivers/gpu/drm/drm_crtc.c
>> +++ b/drivers/gpu/drm/drm_crtc.c
>> @@ -493,14 +493,23 @@ EXPORT_SYMBOL(__drmm_crtc_alloc_with_planes);
>> * drm_crtc_cleanup - Clean up the core crtc usage
>> * @crtc: CRTC to cleanup
>> *
>> - * This function cleans up @crtc and removes it from the DRM mode setting
>> - * core. Note that the function does *not* free the crtc structure itself,
>> - * this is the responsibility of the caller.
>> + * This function cleans up @crtc and removes it from the DRM mode setting core,
>> + * after first waiting an RCU grace period to ensure @crtc->dev can safely be
>> + * dereferenced by our dma_fence_ops.
>> + *
>> + * Note that the function does *not* free the crtc structure itself, this is the
>> + * responsibility of the caller.
>> */
>> void drm_crtc_cleanup(struct drm_crtc *crtc)
>> {
>> struct drm_device *dev = crtc->dev;
>>
>> + /* Ensure our dma_fence_ops remain valid for an RCU grace period after
>> + * the fence is signaled. This is necessary because our dma_fence_ops
>> + * dereference crtc->dev.
>> + */
>> + synchronize_rcu();
>> +
>> /* Note that the crtc_list is considered to be static; should we
>> * remove the drm_crtc at runtime we would have to decrement all
>> * the indices on the drm_crtc after us in the crtc_list.
>
>
> Seems we forgot [1] this patch. Sorry about that.
>
> This fix also looks fine to me (for now) since it's what the dma_fence
> contract currently demands.
>
> I'm a bit nervous about a synchronize_rcu() being in every cleanup
> path, but I suspect it's the most reasonable thing to do now.
>
> Let's wait a day or two to see if anyone has fundamental objections.
> Could someone with some CRTC knowledge give it an R-b? Otherwise I'd
> push it this week.
I just picked them up and I'm about to push them to drm-misc-fixes.
Regards,
Christian.
>
>
> Regards,
> Philipp
>
>
> [1] https://lore.kernel.org/dri-devel/73fc12734386c9bebc9cf6d3cea4c5f8e125255e.camel@mailbox.org/T/#thttps://lore.kernel.org/dri-devel/73fc12734386c9bebc9cf6d3cea4c5f8e125255e.camel@mailbox.org/T/#t
^ permalink raw reply [flat|nested] 9+ messages in thread