| Message ID | sspixcrejd5nbhijuo7o3z5agbgyq637wc7gph7ajrs3lnls6g@5ljanfmvccyf (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [v4] drm/i915: Fix NULL pointer dereference in capture_engine | expand |
Hi Eugene, On Thu, Nov 28, 2024 at 09:28:43PM +0000, 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> Reviewed-by: Andi Shyti <andi.shyti@linux.intel.com> Thanks, Andi
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. John.
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
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);
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(-)