Skip to content

Commit

Permalink
drm/i915: mark requests for GuC virtual engines to avoid use-after-free
Browse files Browse the repository at this point in the history
[ Upstream commit 5eefc53 ]

References to i915_requests may be trapped by userspace inside a
sync_file or dmabuf (dma-resv) and held indefinitely across different
proceses. To counter-act the memory leaks, we try to not to keep
references from the request past their completion.
On the other side on fence release we need to know if rq->engine
is valid and points to hw engine (true for non-virtual requests).
To make it possible extra bit has been added to rq->execution_mask,
for marking virtual engines.

Fixes: bcb9aa4 ("Revert "drm/i915: Hold reference to intel_context over life of i915_request"")
Signed-off-by: Chris Wilson <chris.p.wilson@linux.intel.com>
Signed-off-by: Andrzej Hajda <andrzej.hajda@intel.com>
Reviewed-by: Andi Shyti <andi.shyti@linux.intel.com>
Signed-off-by: Andi Shyti <andi.shyti@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20230821153035.3903006-1-andrzej.hajda@intel.com
(cherry picked from commit 2804106)
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
  • Loading branch information
ahajda authored and gregkh committed Sep 19, 2023
1 parent 33da1bc commit 7fb464d
Show file tree
Hide file tree
Showing 3 changed files with 6 additions and 5 deletions.
1 change: 1 addition & 0 deletions drivers/gpu/drm/i915/gt/intel_engine_types.h
Expand Up @@ -58,6 +58,7 @@ struct i915_perf_group;

typedef u32 intel_engine_mask_t;
#define ALL_ENGINES ((intel_engine_mask_t)~0ul)
#define VIRTUAL_ENGINES BIT(BITS_PER_TYPE(intel_engine_mask_t) - 1)

struct intel_hw_status_page {
struct list_head timelines;
Expand Down
3 changes: 3 additions & 0 deletions drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
Expand Up @@ -5470,6 +5470,9 @@ guc_create_virtual(struct intel_engine_cs **siblings, unsigned int count,

ve->base.flags = I915_ENGINE_IS_VIRTUAL;

BUILD_BUG_ON(ilog2(VIRTUAL_ENGINES) < I915_NUM_ENGINES);
ve->base.mask = VIRTUAL_ENGINES;

intel_context_init(&ve->context, &ve->base);

for (n = 0; n < count; n++) {
Expand Down
7 changes: 2 additions & 5 deletions drivers/gpu/drm/i915/i915_request.c
Expand Up @@ -134,9 +134,7 @@ static void i915_fence_release(struct dma_fence *fence)
i915_sw_fence_fini(&rq->semaphore);

/*
* Keep one request on each engine for reserved use under mempressure
* do not use with virtual engines as this really is only needed for
* kernel contexts.
* Keep one request on each engine for reserved use under mempressure.
*
* We do not hold a reference to the engine here and so have to be
* very careful in what rq->engine we poke. The virtual engine is
Expand Down Expand Up @@ -166,8 +164,7 @@ static void i915_fence_release(struct dma_fence *fence)
* know that if the rq->execution_mask is a single bit, rq->engine
* can be a physical engine with the exact corresponding mask.
*/
if (!intel_engine_is_virtual(rq->engine) &&
is_power_of_2(rq->execution_mask) &&
if (is_power_of_2(rq->execution_mask) &&
!cmpxchg(&rq->engine->request_pool, NULL, rq))
return;

Expand Down

0 comments on commit 7fb464d

Please sign in to comment.