Re: [PATCH v2] drm/gma500: Create the primary plane in the driver

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Freeing `gma_crtc` on `drm_crtc_init_with_planes()` failure causes a Use-After-Free because the CRTC remains linked in `dev->mode_config.crtc_list`.
--

commit 2d329a448a74770c14b7d09aef862579fa99672e
Author: Shixiong Ou <oushixiong@kylinos.cn>

drm/gma500: Create the primary plane in the driver

drm_crtc_init() creates the primary plane from a fixed format list
that includes ARGB8888. The display engine programs the primary plane
with DISPPLANE_32BPP_NO_ALPHA, so it does not support per-pixel alpha
and must not advertise alpha formats. This commit replaces drm_crtc_init()
with a driver-owned primary plane that advertises only XRGB8888.

> diff --git a/drivers/gpu/drm/gma500/psb_intel_display.c b/drivers/gpu/drm/gma500/psb_intel_display.c
> index 0df75a4a77396..f79484b9d593e 100644
> --- a/drivers/gpu/drm/gma500/psb_intel_display.c
> +++ b/drivers/gpu/drm/gma500/psb_intel_display.c
[ ... ]
> @@ -494,7 +511,17 @@ void psb_intel_crtc_init(struct drm_device *dev, int pipe,
>  		return;
>  	}
>  
> -	drm_crtc_init(dev, &gma_crtc->base, &gma_crtc_funcs);
> +	primary = drmm_universal_plane_alloc(dev, struct drm_plane, dev, 0,
> +					     &gma_primary_plane_funcs,
> +					     gma_primary_formats,
> +					     ARRAY_SIZE(gma_primary_formats),
> +					     NULL, DRM_PLANE_TYPE_PRIMARY, NULL);
> +	if (IS_ERR(primary))
> +		goto err_free_crtc;
> +
> +	if (drm_crtc_init_with_planes(dev, &gma_crtc->base, primary, NULL,
> +				      &gma_crtc_funcs, NULL))
> +		goto err_free_crtc;

[Severity: High]
Could this error handling path leave a freed pointer linked in the DRM core?

Looking at __drm_crtc_init_with_planes(), the CRTC is added to the list early
in the function:

	list_add_tail(&crtc->head, &config->crtc_list);
	crtc->index = config->num_crtc++;

If a subsequent initialization step fails (such as drm_crtc_crc_init()),
__drm_crtc_init_with_planes() returns an error without removing the CRTC from
the list.

When jumping to err_free_crtc here, gma_crtc is freed via kfree() below. Might
this create a use-after-free condition when dev->mode_config.crtc_list is later
traversed?

[ ... ]
> @@ -524,6 +551,11 @@ void psb_intel_crtc_init(struct drm_device *dev, int pipe,
>  
>  	/* Set to true so that the pipe is forced off on initial config. */
>  	gma_crtc->active = true;
> +	return;
> +
> +err_free_crtc:
> +	kfree(gma_crtc->crtc_state);
> +	kfree(gma_crtc);
>  }
>  
>  struct drm_crtc *psb_intel_get_crtc_from_pipe(struct drm_device *dev, int pipe)

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260903071149.423579-1-oushixiong1025@163.com?part=1