Re: [PATCH] drm/i915: Return NULL on error in active_instance

[Sebastian Brzezinka <sebastian.brzezinka@intel.com>]

On Wed Jun 24, 2026 at 12:24 PM CEST, Joonas Lahtinen wrote:
> Quoting Sebastian Brzezinka (2026-06-24 12:27:07)
>> Hi,
>> On Wed Jun 24, 2026 at 11:09 AM CEST, Joonas Lahtinen wrote:
>> > Avoid returning &node->base when node is NULL due to OOM
>> > during GFP_ATOMIC allocation.
>> >
>> > Discovered using AI-assisted static analysis confirmed by
>> > Intel Product Security.
>
> <SNIP>
>
>> > +++ b/drivers/gpu/drm/i915/i915_active.c
>> > @@ -318,7 +318,7 @@ active_instance(struct i915_active *ref, u64 idx)
>> >        */
>> >       node = kmem_cache_alloc(slab_cache, GFP_ATOMIC);
>> >       if (!node)
>> > -             goto out;
>> > +             goto err;
>> just a nit: this jump is not neccesery, you could return early.
>
> We specifically want to embrace the onion error handling idiom with goto
> rather than doing the spinlock release inline here.
>
> Preferred error handling should look more like:
>
> 	if (!try_lock(lock))
> 		goto err;
>
> 	mem = alloc();
> 	if (!mem)
> 		goto err_lock;
>
> 	if (!bla_bla(mem, bar))
> 		goto err_mem;
>
> 	...
>
> err_mem:
> 	free(mem);
> err_lock:
> 	unlock(lock);
> err:
> 	return ret;
>
> Rather than:
>
> 	if (!try_lock(lock))
> 		return ret;
>
> 	mem = alloc();
> 	if (!mem)
> 	{
> 		unlock(lock);
> 		return ret;
> 	}
>
> 	if (!bla_bla(mem, bar))
> 	{
> 		free(mem);
> 		unlock(lock);
> 		return ret;
> 	}
>
> 	...
>
> Regards, Joonas
Good to know, thanks for the explanation.

-- 
Best regards,
Sebastian