Skip to content

Commit

Permalink
drm/i915: Fix a VMA UAF for multi-gt platform
Browse files Browse the repository at this point in the history
Ensure correct handling of closed VMAs on multi-gt platforms to prevent
Use-After-Free. Currently, when GT0 goes idle, closed VMAs that are
exclusively added to GT0's closed_vma link (gt->closed_vma) and
subsequently freed by i915_vma_parked(), which assumes the entire GPU is
idle. However, on platforms with multiple GTs, such as MTL, GT1 may
remain active while GT0 is idle. This causes GT0 to mistakenly consider
the closed VMAs in its closed_vma list as unnecessary, potentially
leading to Use-After-Free issues if a job for GT1 attempts to access a
freed VMA.

Although we do take a wakeref for GT0 but it happens later, after
evaluating VMAs. To mitigate this, it is necessary to hold a GT0 wakeref
early.

v2: Use gt id to detect multi-tile(Andi)
    Fix the incorrect error path.
v3: Add more comment(Andi)
    Use the new gt var when possible(Andrzej)

Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com>
Cc: Chris Wilson <chris.p.wilson@intel.com>
Cc: Andi Shyti <andi.shyti@linux.intel.com>
Cc: Andrzej Hajda <andrzej.hajda@intel.com>
Cc: Sushma Venkatesh Reddy <sushma.venkatesh.reddy@intel.com>
Signed-off-by: Nirmoy Das <nirmoy.das@intel.com>
Tested-by: Andi Shyti <andi.shyti@linux.intel.com>
Reviewed-by: Andi Shyti <andi.shyti@linux.intel.com>
Reviewed-by: Andrzej Hajda <andrzej.hajda@intel.com>
Tested-by: Sushma Venkatesh Reddy <sushma.venkatesh.reddy@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20230608110103.777594-1-andi.shyti@linux.intel.com
  • Loading branch information
Nirmoy Das committed Jun 10, 2023
1 parent 2433584 commit f56fe3e
Showing 1 changed file with 19 additions and 2 deletions.
21 changes: 19 additions & 2 deletions drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
Original file line number Diff line number Diff line change
Expand Up @@ -2692,6 +2692,7 @@ static int
eb_select_engine(struct i915_execbuffer *eb)
{
struct intel_context *ce, *child;
struct intel_gt *gt;
unsigned int idx;
int err;

Expand All @@ -2715,10 +2716,17 @@ eb_select_engine(struct i915_execbuffer *eb)
}
}
eb->num_batches = ce->parallel.number_children + 1;
gt = ce->engine->gt;

for_each_child(ce, child)
intel_context_get(child);
intel_gt_pm_get(ce->engine->gt);
intel_gt_pm_get(gt);
/*
* Keep GT0 active on MTL so that i915_vma_parked() doesn't
* free VMAs while execbuf ioctl is validating VMAs.
*/
if (gt->info.id)
intel_gt_pm_get(to_gt(gt->i915));

if (!test_bit(CONTEXT_ALLOC_BIT, &ce->flags)) {
err = intel_context_alloc_state(ce);
Expand Down Expand Up @@ -2757,7 +2765,10 @@ eb_select_engine(struct i915_execbuffer *eb)
return err;

err:
intel_gt_pm_put(ce->engine->gt);
if (gt->info.id)
intel_gt_pm_put(to_gt(gt->i915));

intel_gt_pm_put(gt);
for_each_child(ce, child)
intel_context_put(child);
intel_context_put(ce);
Expand All @@ -2770,6 +2781,12 @@ eb_put_engine(struct i915_execbuffer *eb)
struct intel_context *child;

i915_vm_put(eb->context->vm);
/*
* This works in conjunction with eb_select_engine() to prevent
* i915_vma_parked() from interfering while execbuf validates vmas.
*/
if (eb->gt->info.id)
intel_gt_pm_put(to_gt(eb->gt->i915));
intel_gt_pm_put(eb->gt);
for_each_child(eb->context, child)
intel_context_put(child);
Expand Down

0 comments on commit f56fe3e

Please sign in to comment.