Re: [PATCH v3 28/40] drm/atmel-hlcdc: Convert to atomic_create_state

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] A NULL pointer dereference occurs in atmel_hlcdc_plane_atomic_duplicate_state if plane->state is NULL.
--

commit 004bf4e4c17577d951c839204e87ff451eddcce3
Author: Maxime Ripard <mripard@kernel.org>

drm/atmel-hlcdc: Convert to atomic_create_state

This commit converts the plane reset implementation to use the standard
atomic_create_state hook. The change was generated using a Coccinelle
semantic patch to automate the transformation.

> diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
> index 70a1cef24388f..e09ce6342085d 100644
> --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
> +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
> @@ -1183,31 +1183,31 @@ static void atmel_hlcdc_plane_atomic_destroy_state(struct drm_plane *p,
[ ... ]
> -static void atmel_hlcdc_plane_reset(struct drm_plane *p)
> +static struct drm_plane_state *atmel_hlcdc_plane_create_state(struct drm_plane *p)
>  {
>  	struct atmel_hlcdc_plane_state *state;
> -
> -	if (p->state) {
> -		atmel_hlcdc_plane_atomic_destroy_state(p, p->state);
> -		p->state = NULL;
> -	}
> +	int ret;
>  
>  	state = kzalloc_obj(*state);
> -	if (state) {
> -		if (atmel_hlcdc_plane_alloc_dscrs(p, state)) {
> -			kfree(state);
> -			drm_err(p->dev,
> -				"Failed to allocate initial plane state\n");
> -			return;
> -		}
> -		__drm_atomic_helper_plane_reset(p, &state->base);
> +	if (!state)
> +		return ERR_PTR(-ENOMEM);

[Severity: High]
This is a pre-existing issue, but could a failure here cause a NULL pointer
dereference later?

If this initial allocation fails during device probe or reset, the DRM core
leaves p->state as NULL.

When a subsequent atomic modeset is initiated, the core calls the driver's
atomic_duplicate_state hook. In atmel_hlcdc_plane_atomic_duplicate_state(),
this NULL state is retrieved:

    struct atmel_hlcdc_plane_state *state =
            drm_plane_state_to_atmel_hlcdc_plane_state(p->state);

And then directly passed to kmemdup():

    copy = kmemdup(state, sizeof(*state), GFP_KERNEL);

Since kmemdup() does not check for a NULL source pointer, this leads to a
kernel panic inside its internal memcpy() call.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260831-drm-no-more-plane-reset-v3-0-1877c7aa57b3@kernel.org?part=28