Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
New issues: - [High] Dereference of `panel_bridge` after it may have been freed inside `devm_drm_panel_bridge_release()`. Pre-existing issues: - [High] `drm_panel_bridge_remove()` accesses `panel->dev` for devres removal, but the underlying `struct device` may have already been freed. -- --- Patch [2]: [PATCH v2 2/5] drm/bridge/panel: hold a reference to the wrapped panel --- commit 94f4ef45af3a89d595cd95811619b47c422771c3 Author: Albert Esteve <[email protected]> drm/bridge/panel: hold a reference to the wrapped panel This patch acquires a reference to the wrapped drm_panel in drm_panel_bridge_add_typed() using drm_panel_get() and releases it in the teardown paths to prevent dereferencing dangling pointers. > diff --git a/drivers/gpu/drm/bridge/panel.c b/drivers/gpu/drm/bridge/panel.c > index 4978ec98a0828..6b98ad19508df 100644 > --- a/drivers/gpu/drm/bridge/panel.c > +++ b/drivers/gpu/drm/bridge/panel.c [ ... ] > @@ -316,6 +316,7 @@ EXPORT_SYMBOL(drm_panel_bridge_add_typed); > void drm_panel_bridge_remove(struct drm_bridge *bridge) > { > struct panel_bridge *panel_bridge; > + struct drm_panel *panel; > > if (!bridge) > return; > @@ -326,10 +327,12 @@ void drm_panel_bridge_remove(struct drm_bridge *bridge) > } > > panel_bridge = drm_bridge_to_panel_bridge(bridge); > + panel = panel_bridge->panel; > > drm_bridge_remove(bridge); > /* TODO remove this after reworking panel_bridge lifetime */ > - devm_drm_put_bridge(panel_bridge->panel->dev, bridge); > + devm_drm_put_bridge(panel->dev, bridge); [Severity: High] This is a pre-existing issue, but can accessing panel->dev here lead to a use-after-free of the struct device? While this patch successfully keeps the drm_panel structure alive via drm_panel_get(), it does not appear to take a get_device() reference to the underlying struct device (panel->dev). If the panel device was completely unregistered, could the struct device memory be freed while the drm_panel object remains alive, causing devm_drm_put_bridge() to operate on a dangling pointer? > + drm_panel_put(panel); > } > EXPORT_SYMBOL(drm_panel_bridge_remove); > > @@ -357,11 +360,14 @@ EXPORT_SYMBOL(drm_panel_bridge_set_orientation); > static void devm_drm_panel_bridge_release(struct device *dev, void *res) > { > struct drm_bridge *bridge = *(struct drm_bridge **)res; > + struct panel_bridge *panel_bridge; > > if (!bridge) > return; > > + panel_bridge = drm_bridge_to_panel_bridge(bridge); > drm_bridge_remove(bridge); > + drm_panel_put(panel_bridge->panel); [Severity: High] Could this result in a use-after-free of the panel_bridge structure? If the consumer device unbinds, devm_drm_panel_bridge_release() calls drm_bridge_remove(bridge). If this drops the final reference to the bridge, it triggers __drm_bridge_free() which frees the panel_bridge structure. The very next line calls drm_panel_put(panel_bridge->panel). Does this dereference the potentially freed panel_bridge pointer? Perhaps panel_bridge->panel should be cached in a local variable before calling drm_bridge_remove()? > } > > /** -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=2
