The previous patch caches the timeline name so that get_timeline_name() no
longer dereferences a scheduler that a userspace-held fence has outlived.
That is a targeted fix: the underlying reason the callback is reachable at
all is that both drm_sched fences implement .release, so dma_fence never
detaches their ops on signalling. get_driver_name() has the same exposure
for module unload.

Dropping the .release callbacks is the complete fix, but it requires
auditing every to_drm_sched_fence() caller (ops-detach makes it return NULL
for signalled fences), a different identity mechanism for
pvr_queue_fence_is_native(), and a rework of the shared allocation's
reference handling. Record that as a TODO entry so the cleanup is not lost.

Suggested-by: Philipp Stanner <[email protected]>
Signed-off-by: Jonghyuk Kim(MalHyuk) <[email protected]>
---
 Documentation/gpu/todo.rst | 39 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 39 insertions(+)

diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst
index 14cf37590fc7..284aeba3c752 100644
--- a/Documentation/gpu/todo.rst
+++ b/Documentation/gpu/todo.rst
@@ -990,6 +990,45 @@ Contact:
 
 Level: Beginner
 
+Detach the scheduler fence ops on signalling
+--------------------------------------------
+
+The dma-fence contract forbids touching driver-provided data - everything
+reachable through &dma_fence.ops - once a fence is signalled. dma_fence 
enforces
+that by detaching a fence's ops on signalling, but only for fences that carry
+neither a .release nor a .wait callback (see
+dma_fence_signal_timestamp_locked()).
+
+Both drm_sched fences implement .release, so their ops stay attached forever.
+That leaves the callbacks reachable on a long-signalled fence that userspace
+still holds through a sync_file or drm_syncobj, even after the scheduler is
+gone: get_timeline_name() used to dereference the freed &drm_sched_fence.sched
+(fixed by caching the name), and get_driver_name() can still return a string
+literal belonging to a module that has since been unloaded.
+
+Dropping the .release callbacks so that the ops are detached on signalling is
+the complete fix, and it is what the dma-fence rules ask for. It is not
+straightforward:
+
+Tasks:
+
+- Audit every to_drm_sched_fence() caller. Detaching the ops makes the helper
+  return NULL for a signalled fence, and callers such as
+  amdgpu_cs_p2_dependencies() and amdgpu_ctx_fence_time() dereference the 
result
+  unconditionally.
+- drm/imagination uses the ops pointer as an identity test in
+  pvr_queue_fence_is_native(); that needs a different mechanism.
+- Rework the reference handling. The scheduled and the finished fence share one
+  allocation, and the finished fence's .release currently drops the scheduled
+  fence's reference, so the callbacks cannot simply be deleted.
+
+Contact:
+
+- Philipp Stanner <[email protected]>
+- Christian König <[email protected]>
+
+Level: Advanced
+
 Outside DRM
 ===========
 
-- 
2.43.0

Reply via email to