Re: [PATCH v2 02/28] drm/atomic_helper: Skip over NULL private_obj pointers

[Simona Vetter]

On Thu, Apr 23, 2026 at 12:18:15PM +0200, Maxime Ripard wrote:
> Just like for connectors, drm_atomic_state contains an array of

Maybe s/connectors/all other objects/ because for a moment I've pondered
whether we do this because connectors are special as dynamically
created/refcounted kms objects. Unlike planes/crtcs and private objects.

With that also Reviewed-by: Simona Vetter <simona.vetter@xxxxxxxx>

Cheers, Sima

> drm_private_state, with the number of states found in num_private_objs.
> 
> If we are to clean up a state by hand for some reason before calling
> drm_atomic_state_put(), chances are that the pointer to the affected
> drm_private_obj and drm_private_states would have been cleared to avoid
> any use-after-free.
> 
> However, since it's just an array, as we progress and free the items, we
> can't update num_private_objs as we go since we would reduce the array
> size, preventing us to remove the final elements.
> 
> And if the caller was to forget to update num_private_objs after it
> iterated over the whole array, we're left with a (valid) array with a
> non-zero number of NULL elements.
> 
> If we were to call drm_atomic_state_put() at this point, chances are
> that drm_atomic_state_default_clear() would be called and it would
> iterate over all those empty NULL items.
> 
> However, unlike what is found for connectors, crtcs and planes, we don't
> test that our pointers are non-NULL before dereferencing them, leading
> to a NULL pointer dereference.
> 
> Such callers should obviously be fixed, but there's no reason to not do
> such a simple check, if only to be consistent.
> 
> Reviewed-by: Thomas Zimmermann <tzimmermann@xxxxxxx>
> Signed-off-by: Maxime Ripard <mripard@xxxxxxxxxx>
> ---
>  drivers/gpu/drm/drm_atomic.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index 3bd52602fe30..84bc91886b4c 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -334,10 +334,13 @@ void drm_atomic_state_default_clear(struct drm_atomic_state *state)
>  	}
>  
>  	for (i = 0; i < state->num_private_objs; i++) {
>  		struct drm_private_obj *obj = state->private_objs[i].ptr;
>  
> +		if (!obj)
> +			continue;
> +
>  		obj->funcs->atomic_destroy_state(obj,
>  						 state->private_objs[i].state_to_destroy);
>  		state->private_objs[i].ptr = NULL;
>  		state->private_objs[i].state_to_destroy = NULL;
>  		state->private_objs[i].old_state = NULL;
> 
> -- 
> 2.53.0
> 

-- 
Simona Vetter
Software Engineer
http://blog.ffwll.ch