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 <[email protected]>

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/[email protected]?part=13

Reply via email to