Re: [Intel-gfx] [PATCH 19/27] drm/i915: Fix bug in user proto-context creation that leaked contexts

Matthew Brost
2021-09-21T19:33:14Z

On Mon, Sep 20, 2021 at 03:57:06PM -0700, John Harrison wrote:
> On 8/20/2021 15:44, Matthew Brost wrote:
> > Set number of engines before attempting to create contexts so the
> > function free_engines can clean up properly.
> >
> > Fixes: d4433c7600f7 ("drm/i915/gem: Use the proto-context to handle create parameters (v5)")
> > Signed-off-by: Matthew Brost <matthew.brost@intel.com>
> > Cc: <stable@vger.kernel.org>
> > ---
> > drivers/gpu/drm/i915/gem/i915_gem_context.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c b/drivers/gpu/drm/i915/gem/i915_gem_context.c
> > index dbaeb924a437..bcaaf514876b 100644
> > --- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
> > +++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
> > @@ -944,6 +944,7 @@ static struct i915_gem_engines *user_engines(struct i915_gem_context *ctx,
> > unsigned int n;
> > e = alloc_engines(num_engines);
> This can return null when out of memory. There needs to be an early exit
> check before dereferencing a null pointer. Not sure if that is a worse bug
> or not than leaking memory! Either way, it would be good to fix that too.
>

Indeed there is another bug. Will fix that one too.

Matt

> John.
>
> > + e->num_engines = num_engines;
> > for (n = 0; n < num_engines; n++) {
> > struct intel_context *ce;
> > int ret;
> > @@ -977,7 +978,6 @@ static struct i915_gem_engines *user_engines(struct i915_gem_context *ctx,
> > goto free_engines;
> > }
> > }
> > - e->num_engines = num_engines;
> > return e;
>