Re: [PATCH 2/3] drm/panthor: Add initial support for CIX Sky1

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found no new issues
in this patch, but noticed 1 related pre-existing issue it does not
introduce:
- [Critical] drm/panthor: memory leak and use-after-free in
  panthor_devfreq_init() [1]
--

commit a4d7e2c7598493163cc0be92c3c3410ed211540f
Author: Cunyuan Liu <cunyuan.liu@cixtech.com>

drm/panthor: Add initial support for CIX Sky1

This commit adds SoC data and driver handling for CIX Sky1 to perform an
external GPU reset on initial resume, program SYSC_ALLOC registers before
L2 power-on, and skip devfreq initialization.

> diff --git a/drivers/gpu/drm/panthor/panthor_devfreq.c b/drivers/gpu/drm/panthor/panthor_devfreq.c
> index 2249b41ca4afe..1914549f7b1fb 100644
> --- a/drivers/gpu/drm/panthor/panthor_devfreq.c
> +++ b/drivers/gpu/drm/panthor/panthor_devfreq.c
> @@ -154,6 +154,12 @@ int panthor_devfreq_init(struct panthor_device *ptdev)
>  
>  	ptdev->devfreq = pdevfreq;
>  
> +	/* Keep the firmware clock settings until Sky1 devfreq is supported. */
> +	if (ptdev->soc_data && ptdev->soc_data->skip_devfreq) {
> +		ptdev->fast_rate = clk_get_rate(ptdev->clks.core);
> +		return 0;
> +	}
> +
>  	/*
>  	 * The power domain associated with the GPU may have already added an
>  	 * OPP table, complete with OPPs, as part of the platform bus

[Severity: Critical]
This problem wasn't introduced by this patch, but does panthor_devfreq_init()
leak the cooling device [1] and leave a dangling pointer to
pdevfreq->devfreq on unbind?

drivers/gpu/drm/panthor/panthor_devfreq.c:panthor_devfreq_init() {
	...
	pdevfreq->devfreq = devm_devfreq_add_device(dev, &panthor_devfreq_profile,
						    DEVFREQ_GOV_SIMPLE_ONDEMAND,
						    &pdevfreq->gov_data);
	...
	cooling = devfreq_cooling_em_register(pdevfreq->devfreq, NULL);
	if (IS_ERR(cooling))
		DRM_DEV_INFO(dev, "Failed to register cooling device\n");

	return 0;
}

cooling is registered with non-devm devfreq_cooling_em_register() and
never saved or unregistered, while pdevfreq->devfreq is managed by devres
and freed when the device is unbound.  Any subsequent thermal governor or
sysfs access to the cooling device will dereference the freed devfreq
instance.

[1] https://sashiko.dev/#/bug/linux-aae5bae7-91d6-4e9e-9c28-1cbf604c1457

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261106104735.1651267-1-cunyuan.liu@cixtech.com?part=2