Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues: - [High] TOCTOU race condition in DRM CRTC fence operations triggers a kernel panic via BUG_ON. -- commit 00a837e98e1d45bf6a3df4ddad52336211392885 Author: Philipp Stanner <[email protected]> dma-buf: dma-fence: Fix potential NULL pointer dereference This commit adds memory barriers to the fence signaling and name retrieval functions. It ensures that the signaled bit is set before the ops pointer is cleared, and that readers correctly order the ops pointer load before checking the signaled bit to safely handle decoupled fences. > diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c > index c7ea1e75d38a2..2e80b01499de8 100644 > --- a/drivers/dma-buf/dma-fence.c > +++ b/drivers/dma-buf/dma-fence.c [ ... ] > @@ -1170,6 +1182,12 @@ const char __rcu *dma_fence_driver_name(struct > dma_fence *fence) > > /* RCU protection is required for safe access to returned string */ > ops = rcu_dereference(fence->ops); > + /* > + * Fully order the dereference above with the flag check. Otherwise, > + * ops could be dereferenced as a NULL pointer. The barrier's > + * counterpart is in dma_fence_signal_timestamp_locked(). > + */ > + smp_rmb(); > if (!dma_fence_test_signaled_flag(fence)) > return (const char __rcu *)ops->get_driver_name(fence); [Severity: High] This is a pre-existing issue, but can this lead to a kernel panic during concurrent fence signaling? If a concurrent thread signals the fence after the flag is checked here, the ops pointer can be set to NULL. When the callback resolves to drm_crtc_fence_get_driver_name() in the drm crtc code, it calls fence_to_crtc(), which re-reads the ops pointer: drivers/gpu/drm/drm_crtc.c:fence_to_crtc() { BUG_ON(rcu_access_pointer(fence->ops) != &drm_crtc_fence_ops); return container_of(fence->extern_lock, struct drm_crtc, fence_lock); } Since the decoupling mechanism can set the ops pointer to NULL, won't this BUG_ON trigger and crash the system? Do the fence callbacks need to be updated to avoid re-reading the ops pointer? -- Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1
