Intel-XE Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/xe: Don't keep stale pointer to bo->ggtt_node
@ 2024-09-06 22:03 Michal Wajdeczko
  2024-09-06 22:36 ` Rodrigo Vivi
                   ` (8 more replies)
  0 siblings, 9 replies; 11+ messages in thread
From: Michal Wajdeczko @ 2024-09-06 22:03 UTC (permalink / raw)
  To: intel-xe; +Cc: Michal Wajdeczko, Rodrigo Vivi

When we fail to map a BO in the GGTT, we release our GGTT node
placeholder, but leave stale bo->ggtt_node pointer to it, which
triggers an assert immediately followed by a crash, due to UAF:

[ ] xe 0000:00:02.0: [drm] Assertion `bo->ggtt_node->base.size == bo->size` failed!
[ ] WARNING: CPU: 4 PID: 126 at drivers/gpu/drm/xe/xe_ggtt.c:689 xe_ggtt_remove_bo+0x1d9/0x250 [xe]
[ ] RIP: 0010:xe_ggtt_remove_bo+0x1d9/0x250 [xe]
[ ] Call Trace:
[ ]  <TASK>
[ ]  ? __warn+0x88/0x190
[ ]  ? xe_ggtt_remove_bo+0x1d9/0x250 [xe]
[ ]  ? report_bug+0x1c3/0x1d0
[ ]  ? handle_bug+0x42/0x70
[ ]  ? exc_invalid_op+0x14/0x70
[ ]  ? asm_exc_invalid_op+0x16/0x20
[ ]  ? xe_ggtt_remove_bo+0x1d9/0x250 [xe]
[ ]  ? xe_ggtt_remove_bo+0x1d9/0x250 [xe]
[ ]  xe_ttm_bo_destroy+0x11f/0x260 [xe]
[ ]  ? ttm_bo_release+0x31c/0x350 [ttm]
[ ]  ? __mutex_unlock_slowpath+0x35/0x270
[ ]  __xe_bo_create_locked+0x4a0/0x550 [xe]
[ ]  ? mark_held_locks+0x49/0x80
[ ]  xe_bo_create_pin_map_at+0x37/0x200 [xe]
[ ]  xe_bo_create_pin_map+0x11/0x20 [xe]

While around, for similar reason, also don't keep an error pointer
if we fail to allocate ggtt_node placeholder.

Fixes: 34e804220f69 ("drm/xe: Make xe_ggtt_node struct independent")
Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 drivers/gpu/drm/xe/xe_ggtt.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/xe/xe_ggtt.c b/drivers/gpu/drm/xe/xe_ggtt.c
index f3fca5565d32..2895f154654c 100644
--- a/drivers/gpu/drm/xe/xe_ggtt.c
+++ b/drivers/gpu/drm/xe/xe_ggtt.c
@@ -619,16 +619,19 @@ static int __xe_ggtt_insert_bo_at(struct xe_ggtt *ggtt, struct xe_bo *bo,
 	bo->ggtt_node = xe_ggtt_node_init(ggtt);
 	if (IS_ERR(bo->ggtt_node)) {
 		err = PTR_ERR(bo->ggtt_node);
+		bo->ggtt_node = NULL;
 		goto out;
 	}
 
 	mutex_lock(&ggtt->lock);
 	err = drm_mm_insert_node_in_range(&ggtt->mm, &bo->ggtt_node->base, bo->size,
 					  alignment, 0, start, end, 0);
-	if (err)
+	if (err) {
 		xe_ggtt_node_fini(bo->ggtt_node);
-	else
+		bo->ggtt_node = NULL;
+	} else {
 		xe_ggtt_map_bo(ggtt, bo);
+	}
 	mutex_unlock(&ggtt->lock);
 
 	if (!err && bo->flags & XE_BO_FLAG_GGTT_INVALIDATE)
-- 
2.43.0


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

end of thread, other threads:[~2024-09-09 17:50 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-09-06 22:03 [PATCH] drm/xe: Don't keep stale pointer to bo->ggtt_node Michal Wajdeczko
2024-09-06 22:36 ` Rodrigo Vivi
2024-09-07  0:35 ` ✓ CI.Patch_applied: success for " Patchwork
2024-09-07  0:35 ` ✗ CI.checkpatch: warning " Patchwork
2024-09-07  0:36 ` ✓ CI.KUnit: success " Patchwork
2024-09-07  0:48 ` ✓ CI.Build: " Patchwork
2024-09-07  0:51 ` ✓ CI.Hooks: " Patchwork
2024-09-07  0:52 ` ✓ CI.checksparse: " Patchwork
2024-09-07  1:08 ` ✓ CI.BAT: " Patchwork
2024-09-09 15:53 ` ✗ CI.FULL: failure " Patchwork
2024-09-09 17:50   ` Michal Wajdeczko

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