Re: [PATCH] GPU: drm: meson: inline call to drm_simple_encoder_init()

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] The DRM bridge is added to the global bridge list but is not removed on subsequent error paths, leading to a Use-After-Free.
- [High] The devm-managed encoder is registered to the DRM device, causing a Use-After-Free on unbind if the DRM device outlives the platform device.
--

commit 38b3bb11e881972fb31cc13c9266c01adfa5805b
Author: Liam Zuiderhoek <zuiderhoekl@gmail.com>

GPU: drm: meson: inline call to drm_simple_encoder_init()

This commit inlines the function drm_simple_encoder_init for the meson
driver. It replaces the simple encoder init call with drm_encoder_init
and a custom cleanup struct.

> diff --git a/drivers/gpu/drm/meson/meson_encoder_hdmi.c b/drivers/gpu/drm/meson/meson_encoder_hdmi.c
> index 55c0601df3c62..0dd3a76ad959e 100644
> --- a/drivers/gpu/drm/meson/meson_encoder_hdmi.c
> +++ b/drivers/gpu/drm/meson/meson_encoder_hdmi.c

[ ... ]

> @@ -407,8 +411,9 @@ int meson_encoder_hdmi_probe(struct meson_drm *priv)
>  	meson_encoder_hdmi->priv = priv;
>  
>  	/* Encoder */
> -	ret = drm_simple_encoder_init(priv->drm, &meson_encoder_hdmi->encoder,
> -				      DRM_MODE_ENCODER_TMDS);
> +	ret = drm_encoder_init(priv->drm, &meson_encoder_hdmi->encoder,
> +			       &drm_simple_encoder_funcs_cleanup,
> +			       DRM_MODE_ENCODER_TMDS, NULL);

[Severity: High]
This isn't a bug introduced by this patch, but does registering a devres
managed encoder with the DRM device cause a use-after-free during unbind?

The encoder structure is allocated earlier using devres:

	meson_encoder_hdmi = devm_drm_bridge_alloc(priv->dev, ...);

It is then initialized and registered into the DRM device's mode config list
here. If the driver is unbound and the platform device is destroyed, devres
will free the encoder memory. However, if userspace holds an open DRM file
descriptor, the DRM device outlives the platform device.

When the file descriptor is later closed and the DRM device is finally
destroyed, drm_mode_config_cleanup() will attempt to call the .destroy
callback on the freed encoder memory.

>  	if (ret) {
>  		dev_err_probe(priv->dev, ret, "Failed to init HDMI encoder\n");
>  		goto err_put_node;

[Severity: High]
This is a pre-existing issue, but does this error path lead to a
use-after-free?

Earlier in meson_encoder_hdmi_probe(), the bridge is added to the global
bridge list:

	drm_bridge_add(&meson_encoder_hdmi->bridge);

If drm_encoder_init() fails, we jump to err_put_node, which returns without
calling drm_bridge_remove(). The devres cleanup will then free
meson_encoder_hdmi, leaving a dangling pointer in the DRM core's global
bridge list.

>  	}

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