| Message ID | 5a42bhkoaoed2mky6343qz35pdyzfz6pqgyczrywqgwb4ezipv@be2aiql3bmih (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915: Fixed NULL pointer dereference in capture_engine | expand |
Hi Eugene, On Tue, Nov 19, 2024 at 05:16:44PM +0000, Eugene Kobyak wrote: > When the intel_context structure contains NULL, > it raises a NULL pointer dereference error in drm_info(). > > This patch aims to resolve issue: > https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/12309 > > Signed-off-by: Eugene Kobyak <eugene.kobyak@intel.com> Fixes: e8a3319c31a1 ("drm/i915: Allow error capture without a request") Cc: John Harrison <John.C.Harrison@Intel.com> Cc: <stable@vger.kernel.org> # v6.3+ > --- > drivers/gpu/drm/i915/i915_gpu_error.c | 10 ++++++---- > 1 file changed, 6 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c > index 135ded17334e..b00651ad8515 100644 > --- a/drivers/gpu/drm/i915/i915_gpu_error.c > +++ b/drivers/gpu/drm/i915/i915_gpu_error.c > @@ -1643,11 +1643,13 @@ capture_engine(struct intel_engine_cs *engine, > return NULL; > > intel_engine_get_hung_entity(engine, &ce, &rq); > - if (rq && !i915_request_started(rq)) > - drm_info(&engine->gt->i915->drm, "Got hung context on %s with active request %lld:%lld [0x%04X] not yet started\n", > - engine->name, rq->fence.context, rq->fence.seqno, ce->guc_id.id); > - > if (rq) { > + if (!i915_request_started(rq)) { why are you breaking the if here? > + u16 guc_id = ce ? ce->guc_id.id : 0; good catch! Andi > + drm_info(&engine->gt->i915->drm, "Got hung context on %s with active request %lld:%lld [0x%04X] not yet started\n", > + engine->name, rq->fence.context, rq->fence.seqno, guc_id); > + } > capture = intel_engine_coredump_add_request(ee, rq, ATOMIC_MAYFAIL); > i915_request_put(rq); > } else if (ce) { > -- > 2.34.1
Hi Eugene, > > intel_engine_get_hung_entity(engine, &ce, &rq); > > - if (rq && !i915_request_started(rq)) > > - drm_info(&engine->gt->i915->drm, "Got hung context on %s with active request %lld:%lld [0x%04X] not yet started\n", > > - engine->name, rq->fence.context, rq->fence.seqno, ce->guc_id.id); > > - > > if (rq) { > > + if (!i915_request_started(rq)) { > > why are you breaking the if here? Just to be clear, this is not a binding comment as you are merging to "if (rq)". But I prefer the previous style as the line of this drm_info() is already too long and with one more level of indentation is even longer. In any case: Reviewed-by: Andi Shyti <andi.shyti@linux.intel.com> and now I'm really cc'eing John. Andi > > + u16 guc_id = ce ? ce->guc_id.id : 0; > > good catch! > > Andi > > > + drm_info(&engine->gt->i915->drm, "Got hung context on %s with active request %lld:%lld [0x%04X] not yet started\n", > > + engine->name, rq->fence.context, rq->fence.seqno, guc_id); > > + } > > capture = intel_engine_coredump_add_request(ee, rq, ATOMIC_MAYFAIL); > > i915_request_put(rq); > > } else if (ce) { > > -- > > 2.34.1
diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c index 135ded17334e..b00651ad8515 100644 --- a/drivers/gpu/drm/i915/i915_gpu_error.c +++ b/drivers/gpu/drm/i915/i915_gpu_error.c @@ -1643,11 +1643,13 @@ capture_engine(struct intel_engine_cs *engine, return NULL; intel_engine_get_hung_entity(engine, &ce, &rq); - if (rq && !i915_request_started(rq)) - drm_info(&engine->gt->i915->drm, "Got hung context on %s with active request %lld:%lld [0x%04X] not yet started\n", - engine->name, rq->fence.context, rq->fence.seqno, ce->guc_id.id); - if (rq) { + if (!i915_request_started(rq)) { + u16 guc_id = ce ? ce->guc_id.id : 0; + + drm_info(&engine->gt->i915->drm, "Got hung context on %s with active request %lld:%lld [0x%04X] not yet started\n", + engine->name, rq->fence.context, rq->fence.seqno, guc_id); + } capture = intel_engine_coredump_add_request(ee, rq, ATOMIC_MAYFAIL); i915_request_put(rq); } else if (ce) {
When the intel_context structure contains NULL, it raises a NULL pointer dereference error in drm_info(). This patch aims to resolve issue: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/12309 Signed-off-by: Eugene Kobyak <eugene.kobyak@intel.com> --- drivers/gpu/drm/i915/i915_gpu_error.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-)