Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Low] drm/bridge: dw-hdmi: bogus fix introduces dead code and double-put risk --
commit a2a89ee3551f54a10aa7baa4ebb98caa3f85eb6f Author: Wentao Liang <[email protected]> drm/bridge: dw-hdmi: Fix next_bridge reference leak in dw_hdmi_parse_dt() This commit attempts to fix a claimed reference leak of the next bridge in the pipeline by manually dropping the reference in the probe error path and the remove function. > diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c > b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c > index 937d61c42b58d..a1434beef92ff 100644 > --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c > +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c [ ... ] > @@ -3588,6 +3588,7 @@ struct dw_hdmi *dw_hdmi_probe(struct platform_device > *pdev, > return hdmi; > > err_res: > + drm_bridge_put(hdmi->bridge.next_bridge); [Severity: Low] Is this fix addressing a phantom issue? The commit message mentions fixing a leak in dw_hdmi_parse_dt(), but that function appears to have been removed in an earlier refactoring. The bridge lookup now happens dynamically in dw_hdmi_bridge_attach() using a local variable that manages refcounting. Also, since hdmi->bridge.next_bridge is never actually populated anywhere in dw-hdmi.c (and is initialized to NULL by devm_drm_bridge_alloc), does this simply introduce dead code by calling drm_bridge_put(NULL)? Furthermore, if the bridge were populated, wouldn't this manual put cause a double-put risk? According to the DRM bridge core lifecycle, next_bridge is automatically put when the bridge's refcount reaches zero and __drm_bridge_free() is invoked during probe failure cleanup. > i2c_put_adapter(hdmi->ddc); > > return ERR_PTR(ret); > @@ -3597,6 +3598,7 @@ EXPORT_SYMBOL_GPL(dw_hdmi_probe); > void dw_hdmi_remove(struct dw_hdmi *hdmi) > { > drm_bridge_remove(&hdmi->bridge); > + drm_bridge_put(hdmi->bridge.next_bridge); [Severity: Low] Similar to the err_res path in dw_hdmi_probe(), isn't this manual put both dead code (acting on a NULL pointer) and a violation of the DRM bridge API? If next_bridge were actually populated, the DRM core would intrinsically execute drm_bridge_put(bridge->next_bridge) during standard teardown when the refcount drops to zero. Calling it manually here risks a double-put during teardown. > > if (hdmi->audio && !IS_ERR(hdmi->audio)) > platform_device_unregister(hdmi->audio); -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
