Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
New issues: - [High] Releasing the panel reference in the unregister paths instead of a .destroy callback introduces a Use-After-Free. Pre-existing issues: - [High] Accessing `panel->dev` in `drm_panel_bridge_remove()` can cause a Use-After-Free of a `struct device` if the panel device has been unregistered. -- --- Patch [2]: [PATCH v3 2/4] drm/bridge/panel: hold a reference to the wrapped panel --- commit ac2f873b583c71488e3debe5277a8e3347eee2cd Author: Albert Esteve <[email protected]> drm/bridge/panel: hold a reference to the wrapped panel This commit acquires a reference to the drm_panel in drm_panel_bridge_add_typed() to prevent dangling pointers. It then releases the reference in each teardown path. > diff --git a/drivers/gpu/drm/bridge/panel.c b/drivers/gpu/drm/bridge/panel.c > index 4978ec98a0828..3bfa18167dd26 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 could accessing panel->dev here lead to a use-after-free if the panel device has already been unregistered? While drm_panel_get() keeps the drm_panel alive, it doesn't appear to acquire a reference to the underlying struct device. If the device is unbound and freed, panel->dev becomes a dangling pointer. Could devm_drm_put_bridge() then dereference it when traversing the devres list? > + drm_panel_put(panel); [Severity: High] Does this drop the panel reference prematurely and cause a regression? Since drm_bridge objects are refcounted, calling drm_bridge_remove() only unregisters the bridge and drops the list reference. The bridge could remain alive if an active DRM pipeline holds a reference. If the panel is freed here, could subsequent DRM operations on the bridge dereference panel_bridge->panel and trigger a use-after-free? Would it be safer to move the drm_panel_put() to a new .destroy callback in panel_bridge_bridge_funcs, which is invoked by __drm_bridge_free() when the bridge's refcount reaches zero? > } > EXPORT_SYMBOL(drm_panel_bridge_remove); > > @@ -357,11 +360,16 @@ 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; > + struct drm_panel *panel; > > if (!bridge) > return; > > + panel_bridge = drm_bridge_to_panel_bridge(bridge); > + panel = panel_bridge->panel; > drm_bridge_remove(bridge); > + drm_panel_put(panel); [Severity: High] Similarly, if the bridge is kept alive by a reference elsewhere, does dropping the panel reference here risk a use-after-free regression in bridge callbacks later on? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=2
