Intel-XE Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v5 0/4] Fixing some dma-fence use-after-free
@ 2025-06-09 11:03 Tvrtko Ursulin
  2025-06-09 11:03 ` [PATCH v5 1/4] dma-fence: Add safe access helpers and document the rules Tvrtko Ursulin
                   ` (11 more replies)
  0 siblings, 12 replies; 14+ messages in thread
From: Tvrtko Ursulin @ 2025-06-09 11:03 UTC (permalink / raw)
  To: dri-devel
  Cc: kernel-dev, Rob Clark, Sumit Semwal, Gustavo Padovan,
	Christian König, Matthew Brost, Lucas De Marchi,
	Rodrigo Vivi, amd-gfx, intel-xe, intel-gfx, linux-media,
	linaro-mm-sig, Tvrtko Ursulin

Hi all,

tl;dr;
Xe and probably some other drivers can tear down the internal state referenced
by an exported sync_file fence which then causes a null pointer derefences on
accessing said fence.

IGT that exploits the problem:
https://patchwork.freedesktop.org/patch/642709/?series=146211&rev=2

It seems there is a consensus this is a known problem with the dma-fence design,
where internal state shouldn't really be accessed after the fence has been
signaled. However currently the code is mostly unaware of that hence the use-
after-free potential.

To fix it, between the option of adding more reference counting and trying to
"revoke" the fence, suggestion is to focus on the later.

Reference to the recent discussion:
https://lore.kernel.org/dri-devel/20250418164246.72426-1-tvrtko.ursulin@igalia.com/

This series therefore attempts to implement a solution along those lines.

Most of the description what and how can be found in:
 "dma-fence: Add safe access helpers and document the rules"

The series only addreses the runtime use-after-free scenarios,
such as the above explained situation with the xe driver. For now the module
unload problem is deliberately left for later. (Although again, some of the
patches merged earlier do make it safer, and will make future improvements
easier due fewer accesses to fence->ops.)

Final patch in the series is the one which makes xe compliant with the rules
and API proposed earlier in the series. It does so by ensuring there is at least
one RCU grace period between fences being signaled and driver allocated memory
accessible from xe fences getting freed. Which couples with the earlier (from
the series) refactors which added dma_fence_access_begin/end() protection to
the relevant call sites.

If this approach is acceptable the next steps will be to see if any other
drivers will need similar changes. And also to discuss whether we want to go a
step futher and later move to SRCU, so code would be protected against module
unload as well.

v2:
 * Dropped module unload handling.
 * Proposing real API instead of hacks.

v3:
 * Dropped the dma_fence_is_array|chain ops to flags conversion.
 * Dropped the i915 cleanup patch which is now independent.
 * Squashed dma-fence helpers with internal usage patches.
 * Restored missing hunk in "dma-fence: Use a flag for 64-bit seqnos".
 * Removed the AMDGPU_JOB_GET_TIMELINE_NAME macro.
 * Applied some r-b tags.

v4:
 * Tidied 64-bit seqno flags patch and fixed for amdgpu user queues which landed
   since.
 * Adjusted some dma-fence tracepoints to avoid asserts.
 * Protected tracepoints in dma_fence_wait_timeout() with the safe access
   annotations.
 * Dropped driver/timeline helper usage from amdgpu_trace.h.
 * Dropped signaled fence protection from i915 timeline name vfunc.

v5:
 * Rebased after prep patches merged.
 * Move dma_fence_access_end() earlier in dma_fence_release(). (Christian)

Tvrtko Ursulin (4):
  dma-fence: Add safe access helpers and document the rules
  sync_file: Protect access to driver and timeline name
  drm/i915: Protect access to driver and timeline name
  drm/xe: Make dma-fences compliant with the safe access rules

 drivers/dma-buf/dma-fence.c                  | 82 +++++++++++++++++++-
 drivers/dma-buf/sync_file.c                  |  6 ++
 drivers/gpu/drm/i915/gt/intel_gt_requests.c  |  2 +
 drivers/gpu/drm/i915/i915_request.c          | 17 +---
 drivers/gpu/drm/i915/i915_sw_fence.c         |  2 +
 drivers/gpu/drm/xe/xe_guc_exec_queue_types.h |  2 +
 drivers/gpu/drm/xe/xe_guc_submit.c           |  7 +-
 drivers/gpu/drm/xe/xe_hw_fence.c             |  3 +
 include/linux/dma-fence.h                    | 32 ++++++--
 include/trace/events/dma_fence.h             | 38 ++++++++-
 10 files changed, 162 insertions(+), 29 deletions(-)

-- 
2.48.0


^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2025-06-10 13:27 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-06-09 11:03 [PATCH v5 0/4] Fixing some dma-fence use-after-free Tvrtko Ursulin
2025-06-09 11:03 ` [PATCH v5 1/4] dma-fence: Add safe access helpers and document the rules Tvrtko Ursulin
2025-06-10 13:27   ` Christian König
2025-06-09 11:03 ` [PATCH v5 2/4] sync_file: Protect access to driver and timeline name Tvrtko Ursulin
2025-06-09 11:03 ` [PATCH v5 3/4] drm/i915: " Tvrtko Ursulin
2025-06-09 11:03 ` [PATCH v5 4/4] drm/xe: Make dma-fences compliant with the safe access rules Tvrtko Ursulin
2025-06-09 12:59 ` ✓ CI.Patch_applied: success for Fixing some dma-fence use-after-free Patchwork
2025-06-09 12:59 ` ✗ CI.checkpatch: warning " Patchwork
2025-06-09 13:01 ` ✓ CI.KUnit: success " Patchwork
2025-06-09 13:11 ` ✓ CI.Build: " Patchwork
2025-06-09 13:14 ` ✓ CI.Hooks: " Patchwork
2025-06-09 13:15 ` ✗ CI.checksparse: warning " Patchwork
2025-06-09 14:05 ` ✓ Xe.CI.BAT: success " Patchwork
2025-06-09 15:10 ` ✗ Xe.CI.Full: failure " Patchwork

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox