Re: [PATCH] drm/i915: check before removing mm notifier

[Nirmoy Das <nirmoy.das@linux.intel.com>]

[-- Attachment #1: Type: text/plain, Size: 1599 bytes --]

Hi Rodrigo,

On 2/19/2024 9:12 PM, Rodrigo Vivi wrote:
> On Mon, Feb 19, 2024 at 01:50:47PM +0100, Nirmoy Das wrote:
>> Error in mmu_interval_notifier_insert() can leave a NULL
>> notifier.mm pointer. Catch that and return early.
>>
>> Cc: Andi Shyti<andi.shyti@linux.intel.com>
>> Cc: Shawn Lee<shawn.c.lee@intel.com>
>> Signed-off-by: Nirmoy Das<nirmoy.das@intel.com>
>> ---
>>   drivers/gpu/drm/i915/gem/i915_gem_userptr.c | 3 +++
>>   1 file changed, 3 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_userptr.c b/drivers/gpu/drm/i915/gem/i915_gem_userptr.c
>> index 0e21ce9d3e5a..61abfb505766 100644
>> --- a/drivers/gpu/drm/i915/gem/i915_gem_userptr.c
>> +++ b/drivers/gpu/drm/i915/gem/i915_gem_userptr.c
>> @@ -349,6 +349,9 @@ i915_gem_userptr_release(struct drm_i915_gem_object *obj)
>>   {
>>   	GEM_WARN_ON(obj->userptr.page_ref);
>>   
>> +	if (!obj->userptr.notifier.mm)
>> +		return;
>> +
> hmmm... right, it looks that we need this protection. But...
>
> I mean, feel free to use
> Reviewed-by: Rodrigo Vivi<rodrigo.vivi@intel.com>
>
> for this patch,
>
> but I believe that if this mmu insert failed we might have other
> deeper problems like when checking i915_gem_object_is_userptr() ?
>
> No?!

We are returning an error if mmu insert fails while creating a userptr 
object  so the obj struct is only available to obj cleanup methods.

As far as I see, i915_gem_object_is_userptr() should not happen on such obj struct.

Thanks,
Nirmoy

>>   	mmu_interval_notifier_remove(&obj->userptr.notifier);
>>   	obj->userptr.notifier.mm = NULL;
>>   }
>> -- 
>> 2.42.0
>>

[-- Attachment #2: Type: text/html, Size: 2645 bytes --]