FROMLIST: drm/drm_crtc: ensure dma_fence_ops remain valid during device unbind
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.
Link: https://sashiko.dev/#/patchset/20260618-linux-drm_crtc_fix2-v1-1-c03e77b36f34@linaro.org?part=1
Fixes: 6d6003c4b6 ("drm/fence: add fence timeline to drm_crtc")
Cc: stable@vger.kernel.org
Reviewed-by: Philipp Stanner <phasta@kernel.org>
Signed-off-by: André Draszik <andre.draszik@linaro.org>
Link: https://lore.kernel.org/all/20260721-linux-drm_crtc_fix2-v3-1-afa8c71506e6@linaro.org/
Bug: 521531791
Test: TH
Change-Id: Ib7607bfbf5f619c8c4d19d306dff7288063177f1
Signed-off-by: André Draszik <draszik@google.com>
This commit is contained in:
1 parent
2530770c26
commit
bbea9e696f
1 file changed
+12
-3
@@ -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.
|
||||
|
||||
Reference in new issue
Block a user