Re: [PATCH 2/6] drm/tidss: Open-code drm_simple_encoder_init()

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Use-After-Free on driver unbind or probe failure due to mixed devm/drmm lifecycles for the combined encoder/bridge structure.
--

commit b7421f2a0634d4a6488478cb165139b91f9afc7d
Author: Naman Arora <namanarora029@gmail.com>

drm/tidss: Open-code drm_simple_encoder_init()

The helper drm_simple_encoder_init() is a thin wrapper around
drm_encoder_init() with a simple destroy-only encoder funcs struct.
Remove the dependency on drm_simple_kms_helper by open-coding the
encoder initialization directly in the driver.

> diff --git a/drivers/gpu/drm/tidss/tidss_encoder.c b/drivers/gpu/drm/tidss/tidss_encoder.c
> index 34db8d2a3792e..1841dc14c739b 100644
> --- a/drivers/gpu/drm/tidss/tidss_encoder.c
> +++ b/drivers/gpu/drm/tidss/tidss_encoder.c

[ ... ]

> @@ -95,8 +98,9 @@ int tidss_encoder_create(struct tidss_device *tidss,
>  	if (IS_ERR(t_enc))
>  		return PTR_ERR(t_enc);
>  
> -	ret = drm_simple_encoder_init(&tidss->ddev, &t_enc->encoder,
> -				      encoder_type);
> +	ret = drm_encoder_init(&tidss->ddev, &t_enc->encoder,
> +			       &tidss_drm_encoder_funcs,
> +			       encoder_type, NULL);

[Severity: High]
This is a pre-existing issue, but could this mix of devm and drm lifecycles
lead to a use-after-free during driver unbind or probe failure?

The t_enc struct is allocated using devm_drm_bridge_alloc(), giving it an
initial refcount of 1. Here, drm_encoder_init() registers the embedded
t_enc->encoder. Later in this function, drm_bridge_attach() will increase
the refcount to 2.

During driver unbind, devres LIFO cleanup executes the bridge release action,
calling drm_bridge_put() and dropping the refcount to 1. Next, the drm_device
release action triggers drm_mode_config_cleanup(), which calls
drm_encoder_cleanup().

Inside drm_encoder_cleanup(), calling drm_bridge_detach() drops the final
refcount to 0, causing t_enc to be freed. Immediately after that,
drm_encoder_cleanup() accesses the now-freed encoder structure.

A similar issue appears to happen if drm_bridge_attach() fails during probe,
as devres would free t_enc, leaving a dangling pointer for
drm_mode_config_cleanup() to trip over.

>  	if (ret)
>  		return ret;
>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260531074627.8936-1-namanarora029@gmail.com?part=2

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic