Re: [PATCH 44/60] drm/exynos: Convert to atomic_create_state

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] NULL pointer dereference in `exynos_drm_plane_create_state` due to accessing `plane->state` before it is assigned.
--

--- Patch [44]: [PATCH 44/60] drm/exynos: Convert to atomic_create_state ---
commit 425d9a6e7835f76d839e38884c8943389a4e1c1c
Author: Maxime Ripard <mripard@kernel.org>

drm/exynos: Convert to atomic_create_state

This commit converts the plane reset implementation to use atomic_create_state
by replacing the custom reset hook. A Coccinelle semantic patch was used to
perform the conversion.

> diff --git a/drivers/gpu/drm/exynos/exynos_drm_plane.c b/drivers/gpu/drm/exynos/exynos_drm_plane.c
> --- a/drivers/gpu/drm/exynos/exynos_drm_plane.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_plane.c
[ ... ]
> -static void exynos_drm_plane_reset(struct drm_plane *plane)
> +static struct drm_plane_state *exynos_drm_plane_create_state(struct drm_plane *plane)
>  {
>  	struct exynos_drm_plane *exynos_plane = to_exynos_plane(plane);
>  	struct exynos_drm_plane_state *exynos_state;
>  
> -	if (plane->state) {
> -		exynos_state = to_exynos_plane_state(plane->state);
> -		__drm_atomic_helper_plane_destroy_state(plane->state);
> -		kfree(exynos_state);
> -		plane->state = NULL;
> -	}
> -
>  	exynos_state = kzalloc_obj(*exynos_state);
> -	if (exynos_state) {
> -		__drm_atomic_helper_plane_reset(plane, &exynos_state->base);
> -		plane->state->zpos = exynos_plane->config->zpos;
> -	}
> +	if (!exynos_state)
> +		return ERR_PTR(-ENOMEM);
> +
> +	__drm_atomic_helper_plane_state_init(&exynos_state->base, plane);
> +	plane->state->zpos = exynos_plane->config->zpos;

[Severity: High]
Can this cause a NULL pointer dereference during plane initialization?

Since __drm_atomic_helper_plane_state_init() does not assign plane->state
(unlike the old __drm_atomic_helper_plane_reset() helper), plane->state
is likely NULL during initial creation. Even if it is not NULL, should this
code be modifying the newly allocated state (exynos_state->base.zpos)
instead of plane->state->zpos?

> +
> +	return &exynos_state->base;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260709-drm-no-more-plane-reset-v1-0-302d986fe5f0@kernel.org?part=44