Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider: - [High] Dropping the panel reference before removing the bridge creates a Use-After-Free window during driver removal. - [Medium] Panel references acquired during probe/attach are not released on error paths, causing memory leaks. - [Medium] Panel reference leaks when wrapping a panel in a bridge without dropping the lookup reference. --
commit 621578b5ce602b599c8bef0b18966f05081671b7 Author: Albert Esteve <[email protected]> drm/bridge: release panel reference on all lookup exit paths of_drm_find_panel() and drm_of_find_panel_or_bridge() now return a counted reference that the caller must release with drm_panel_put(). For bridge drivers that immediately wrap the panel in a panel_bridge (which acquires its own reference), release the lookup reference right after the bridge creation call. [Severity: Medium] Since the lookup functions now return a counted reference, does this change introduce memory leaks on driver probe error paths? If a driver successfully acquires the panel reference during probe or attach, but a subsequent initialization step fails (like clock registration, component addition, or memory allocation), it appears the reference is leaked if drm_panel_put is not called before returning the error. For example, in exynos_dp_probe, if component_add fails, the panel reference is not released. Similar leaks appear to happen on error paths in analogix_dp_aux_done_probing, sun6i_dsi_attach, tegra_output_probe, and lvds_probe. [Severity: Medium] Additionally, are there missing conversions for drivers that wrap the panel in a bridge? The commit message notes that bridge drivers immediately wrapping the panel must release the lookup reference right after the bridge creation call, since the bridge acquires its own reference. However, it looks like several drivers were missed, such as lvds_codec_probe, fsl_ldb_probe, lt9211_parse_dt, ssd2825_probe, tc358767_probe, and tc358768_probe. In these functions, the panel is wrapped with devm_drm_panel_bridge_add or drm_panel_bridge_add_typed, but the initial lookup reference is never dropped. > diff --git a/drivers/gpu/drm/bridge/analogix/analogix-anx6345.c > b/drivers/gpu/drm/bridge/analogix/analogix-anx6345.c > index f3fe47b12edca..1fe11b075f860 100644 > --- a/drivers/gpu/drm/bridge/analogix/analogix-anx6345.c > +++ b/drivers/gpu/drm/bridge/analogix/analogix-anx6345.c > @@ -756,6 +756,9 @@ static void anx6345_i2c_remove(struct i2c_client *client) > { > struct anx6345 *anx6345 = i2c_get_clientdata(client); > > + if (anx6345->panel) > + drm_panel_put(anx6345->panel); > + > drm_bridge_remove(&anx6345->bridge); [Severity: High] Does this create a use-after-free window? By dropping the last reference to the panel here, the panel object might be freed. However, the bridge is removed after this step. During this window, the bridge remains globally visible and registered. If a concurrent drm commit or modeset (like an atomic disable) executes on the display pipeline before drm_bridge_remove is called, it could invoke the bridge callbacks which would dereference the already-freed panel pointer. Would it be safer to unregister the bridge before dropping the reference to the internal structures it relies on? [ ... ] > diff --git a/drivers/gpu/drm/stm/lvds.c b/drivers/gpu/drm/stm/lvds.c > index 50a878688e477..77735e26c56e3 100644 > --- a/drivers/gpu/drm/stm/lvds.c > +++ b/drivers/gpu/drm/stm/lvds.c > @@ -1189,6 +1189,9 @@ static void lvds_remove(struct platform_device *pdev) > { > struct stm_lvds *lvds = platform_get_drvdata(pdev); > > + if (lvds->panel) > + drm_panel_put(lvds->panel); > + > lvds_pixel_clk_unregister(lvds); > > drm_bridge_remove(&lvds->lvds_bridge); [Severity: High] Could this also trigger a use-after-free for the panel? Similar to the anx6345 driver, the panel reference is dropped before the bridge is removed. A concurrent atomic disable could execute in this window and dereference the freed panel inside the bridge callbacks. -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=4
