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

Tvrtko Ursulin
2021-09-21T14:49:59Z

On 20/09/2021 23:57, 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.

Pull out from the series and send a fix standalone ASAP? Also suggest
adding author and reviewer to cc for typically quicker turnaround time.

Regards,

Tvrtko

> 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;
>