Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues: - [Critical] Use-After-Free in get_driver_name() and get_timeline_name() due to dma_fence RCU contract violation during CRTC teardown. -- commit 4e03eb31730ff05ffb65d7d539f01df4c6b89099 Author: André Draszik <[email protected]> drm/drm_crtc: fix race with dma_fence_signal() in ::get_driver_name() This patch removes a BUG_ON() in drm_crtc_fence_get_driver_name() that was triggering because the dma_fence_ops get detached upon signaling. > diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c > index 63ead8ba67564..31c8636e74677 100644 > --- a/drivers/gpu/drm/drm_crtc.c > +++ b/drivers/gpu/drm/drm_crtc.c [ ... ] > @@ -154,14 +157,6 @@ static void drm_crtc_crc_fini(struct drm_crtc *crtc) > #endif > } > > -static const struct dma_fence_ops drm_crtc_fence_ops; > - > -static struct drm_crtc *fence_to_crtc(struct dma_fence *fence) > -{ > - BUG_ON(rcu_access_pointer(fence->ops) != &drm_crtc_fence_ops); > - return container_of(fence->extern_lock, struct drm_crtc, fence_lock); > -} > - > static const char *drm_crtc_fence_get_driver_name(struct dma_fence *fence) > { > struct drm_crtc *crtc = fence_to_crtc(fence); [Severity: Critical] This is a pre-existing issue, but does removing this BUG_ON() unmask a use-after-free during device teardown? 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 While the BUG_ON() was causing spurious panics during normal operation, it seems it was also inadvertently preventing this use-after-free from proceeding silently. 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? > > return crtc->dev->driver->name; > } -- Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1
