Re: [PATCH v4] drm/i915: Fix NULL pointer dereference in capture_engine

[Andi Shyti <andi.shyti@linux.intel.com>]

On Thu, Nov 28, 2024 at 06:32:28PM -0800, John Harrison wrote:
> On 11/28/2024 13:28, Eugene Kobyak wrote:
> > When the intel_context structure contains NULL,
> > it raises a NULL pointer dereference error in drm_info().
> > 
> > Fixes: e8a3319c31a1 ("drm/i915: Allow error capture without a request")
> > Closes: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/12309
> > Cc: John Harrison <John.C.Harrison@Intel.com>
> > Cc: <stable@vger.kernel.org> # v6.3+
> > Signed-off-by: Eugene Kobyak <eugene.kobyak@intel.com>
> > ---
> > v2:
> >    - return drm_info to separate condition
> > v3:
> >    - create separate condition which generate string if intel_context exist
> > v4:
> >    - rollback and add check intel_context in log condition
> >   drivers/gpu/drm/i915/i915_gpu_error.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c
> > index 135ded17334e..56f05a18870c 100644
> > --- a/drivers/gpu/drm/i915/i915_gpu_error.c
> > +++ b/drivers/gpu/drm/i915/i915_gpu_error.c
> > @@ -1643,7 +1643,7 @@ capture_engine(struct intel_engine_cs *engine,
> >   		return NULL;
> >   	intel_engine_get_hung_entity(engine, &ce, &rq);
> > -	if (rq && !i915_request_started(rq))
> > +	if (rq && !i915_request_started(rq) && ce)
> >   		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);
> But now you don't get a message when there is a request without the context
> pointer. Which can clearly happen because otherwise you wouldn't have hit a
> null pointer dereference in the first place.

do we need the guc_id at all?

Andi