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