Re: [PATCH v2 13/15] drm/meson: remove dependency on DRM simple helpers

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Potential Use-After-Free due to mixing devres (`devm_`) allocation with DRM object lifecycle.
--

--- Patch [13]: [PATCH v2 13/15] drm/meson: remove dependency on DRM simple helpers ---
commit f601252d45634d954a7087cd68009adce3fbf5f7
Author: Diogo Silva <diogompaissilva@gmail.com>

drm/meson: remove dependency on DRM simple helpers

This patch replaces the deprecated simple KMS helper drm_simple_encoder_init()
with drm_encoder_init() across the CVBS, DSI, and HDMI encoders in the meson
DRM driver.

> diff --git a/drivers/gpu/drm/meson/meson_encoder_cvbs.c b/drivers/gpu/drm/meson/meson_encoder_cvbs.c
> index 22cacb1660c4f..cdb84d2283f83 100644
> --- a/drivers/gpu/drm/meson/meson_encoder_cvbs.c
> +++ b/drivers/gpu/drm/meson/meson_encoder_cvbs.c
[ ... ]
> @@ -218,6 +218,10 @@ static const struct drm_bridge_funcs meson_encoder_cvbs_bridge_funcs = {
>  	.atomic_create_state = drm_atomic_helper_bridge_create_state,
>  };
>  
> +static const struct drm_encoder_funcs meson_encoder_cvbs_funcs = {
> +	.destroy = drm_encoder_cleanup,
> +};
> +
>  int meson_encoder_cvbs_probe(struct meson_drm *priv)
>  {
[ ... ]
> @@ -257,8 +261,9 @@ int meson_encoder_cvbs_probe(struct meson_drm *priv)
>  	meson_encoder_cvbs->priv = priv;
>  
>  	/* Encoder */
> -	ret = drm_simple_encoder_init(priv->drm, &meson_encoder_cvbs->encoder,
> -				      DRM_MODE_ENCODER_TVDAC);
> +	ret = drm_encoder_init(priv->drm, &meson_encoder_cvbs->encoder,
> +			       &meson_encoder_cvbs_funcs,
> +			       DRM_MODE_ENCODER_TVDAC, NULL);

[Severity: High]
This is a pre-existing issue, but does using devres for the encoder allocation
create a use-after-free risk during device unbind?

Looking earlier in meson_encoder_cvbs_probe(), the structure containing the DRM
encoder is allocated with devres:

	meson_encoder_cvbs = devm_drm_bridge_alloc(priv->dev,
						   struct meson_encoder_cvbs,
						   bridge,
						   &meson_encoder_cvbs_bridge_funcs);

If the physical device is unbound (e.g., via sysfs) while a user application
holds a DRM file descriptor open, the devres framework will free this memory,
including the embedded struct drm_encoder.

Later, when the file descriptor is closed and the DRM core tears down the
device, it will call drm_encoder_cleanup() on the already-freed encoder.

The exact same devres allocation pattern is also present in 
meson_encoder_dsi_probe() for meson_encoder_dsi, and in
meson_encoder_hdmi_probe() for meson_encoder_hdmi.

Could these be converted to use DRM-managed allocators (like drmm_kzalloc) to
ensure the memory safely outlives the physical device unbind?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260720-drm_simple_encoder_init-v2-0-5020b630668a@gmail.com?part=13