drm/xe/ct: prevent UAF in send_recv()

Ensure we serialize with completion side to prevent UAF with fence going
out of scope on the stack, since we have no clue if it will fire after
the timeout before we can erase from the xa. Also we have some dependent
loads and stores for which we need the correct ordering, and we lack the
needed barriers. Fix this by grabbing the ct->lock after the wait, which
is also held by the completion side.

v2 (Badal):
 - Also print done after acquiring the lock and seeing timeout.

Fixes: dd08ebf6c352 ("drm/xe: Introduce a new DRM driver for Intel GPUs")
Signed-off-by: Matthew Auld <[email protected]>
Cc: Matthew Brost <[email protected]>
Cc: Badal Nilawar <[email protected]>
Cc: <[email protected]> # v6.8+
Reviewed-by: Badal Nilawar <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]

diff --git a/drivers/gpu/drm/xe/xe_guc_ct.c b/drivers/gpu/drm/xe/xe_guc_ct.c
index 4b95f75b15461197bf82b783c1924f2fe6490546..44263b3cd8c7a8b33506ae4414009067be278ff7 100644 (file)
--- a/drivers/gpu/drm/xe/xe_guc_ct.c
+++ b/drivers/gpu/drm/xe/xe_guc_ct.c
@@ -903,16 +903,26 @@ retry_same_fence:
        }
 
        ret = wait_event_timeout(ct->g2h_fence_wq, g2h_fence.done, HZ);
+
+       /*
+        * Ensure we serialize with completion side to prevent UAF with fence going out of scope on
+        * the stack, since we have no clue if it will fire after the timeout before we can erase
+        * from the xa. Also we have some dependent loads and stores below for which we need the
+        * correct ordering, and we lack the needed barriers.
+        */
+       mutex_lock(&ct->lock);
        if (!ret) {
-               xe_gt_err(gt, "Timed out wait for G2H, fence %u, action %04x",
-                         g2h_fence.seqno, action[0]);
+               xe_gt_err(gt, "Timed out wait for G2H, fence %u, action %04x, done %s",
+                         g2h_fence.seqno, action[0], str_yes_no(g2h_fence.done));
                xa_erase_irq(&ct->fence_lookup, g2h_fence.seqno);
+               mutex_unlock(&ct->lock);
                return -ETIME;
        }
 
        if (g2h_fence.retry) {
                xe_gt_dbg(gt, "H2G action %#x retrying: reason %#x\n",
                          action[0], g2h_fence.reason);
+               mutex_unlock(&ct->lock);
                goto retry;
        }
        if (g2h_fence.fail) {
@@ -921,7 +931,12 @@ retry_same_fence:
                ret = -EIO;
        }
 
-       return ret > 0 ? response_buffer ? g2h_fence.response_len : g2h_fence.response_data : ret;
+       if (ret > 0)
+               ret = response_buffer ? g2h_fence.response_len : g2h_fence.response_data;
+
+       mutex_unlock(&ct->lock);
+
+       return ret;
 }
 
 /**