| Message ID | 455b2279-2e08-4d00-9784-be56d8ee42e3@moroto.mountain (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915/gt: Prevent error pointer dereference | expand |
Hi Dan, On Wed, Sep 13, 2023 at 11:17:41AM +0300, Dan Carpenter wrote: > Move the check for "if (IS_ERR(obj))" in front of the call to > i915_gem_object_set_cache_coherency() which dereferences "obj". > Otherwise it will lead to a crash. > > Fixes: 43aa755eae2c ("drm/i915/mtl: Update cache coherency setting for context structure") > Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org> > --- > drivers/gpu/drm/i915/gt/intel_lrc.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c > index 957d0aeb0c02..c378cc7c953c 100644 > --- a/drivers/gpu/drm/i915/gt/intel_lrc.c > +++ b/drivers/gpu/drm/i915/gt/intel_lrc.c > @@ -1094,6 +1094,9 @@ __lrc_alloc_state(struct intel_context *ce, struct intel_engine_cs *engine) > I915_BO_ALLOC_PM_VOLATILE); > if (IS_ERR(obj)) { > obj = i915_gem_object_create_shmem(engine->i915, context_size); > + if (IS_ERR(obj)) > + return ERR_CAST(obj); > + that's correct! When the workaround was added later it wasn't checking whether obj had a valid value or not, leading to a potential segfault. Thanks for fixing it! Reviewed-by: Andi Shyti <andi.shyti@linux.intel.com> Andi > /* > * Wa_22016122933: For Media version 13.0, all Media GT shared > * memory needs to be mapped as WC on CPU side and UC (PAT > @@ -1102,8 +1105,6 @@ __lrc_alloc_state(struct intel_context *ce, struct intel_engine_cs *engine) > if (intel_gt_needs_wa_22016122933(engine->gt)) > i915_gem_object_set_cache_coherency(obj, I915_CACHE_NONE); > } > - if (IS_ERR(obj)) > - return ERR_CAST(obj); > > vma = i915_vma_instance(obj, &engine->gt->ggtt->vm, NULL); > if (IS_ERR(vma)) {
Hi Dan, > Possible regressions > > • igt@gem_ccs@ctrl-surf-copy-new-ctx: > > □ shard-mtlp: NOTRUN -> SKIP > • igt@gen9_exec_parse@allowed-all: > > □ shard-apl: PASS -> INCOMPLETE I believe these failures are not caused by this patch. I had to retrigger the tests because the BAT tests failed at first (Jani, actually did it). I applied your patch in drm-intel-gt-nex. Thanks! Andi
diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c index 957d0aeb0c02..c378cc7c953c 100644 --- a/drivers/gpu/drm/i915/gt/intel_lrc.c +++ b/drivers/gpu/drm/i915/gt/intel_lrc.c @@ -1094,6 +1094,9 @@ __lrc_alloc_state(struct intel_context *ce, struct intel_engine_cs *engine) I915_BO_ALLOC_PM_VOLATILE); if (IS_ERR(obj)) { obj = i915_gem_object_create_shmem(engine->i915, context_size); + if (IS_ERR(obj)) + return ERR_CAST(obj); + /* * Wa_22016122933: For Media version 13.0, all Media GT shared * memory needs to be mapped as WC on CPU side and UC (PAT @@ -1102,8 +1105,6 @@ __lrc_alloc_state(struct intel_context *ce, struct intel_engine_cs *engine) if (intel_gt_needs_wa_22016122933(engine->gt)) i915_gem_object_set_cache_coherency(obj, I915_CACHE_NONE); } - if (IS_ERR(obj)) - return ERR_CAST(obj); vma = i915_vma_instance(obj, &engine->gt->ggtt->vm, NULL); if (IS_ERR(vma)) {
Move the check for "if (IS_ERR(obj))" in front of the call to i915_gem_object_set_cache_coherency() which dereferences "obj". Otherwise it will lead to a crash. Fixes: 43aa755eae2c ("drm/i915/mtl: Update cache coherency setting for context structure") Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org> --- drivers/gpu/drm/i915/gt/intel_lrc.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)