Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
New issues: - [High] Accessing `panel_bridge->panel` after calling `drm_bridge_remove()` causes a use-after-free if the panel device was removed first. Pre-existing issues: - [High] Passing `panel->dev` to `devm_drm_put_bridge()` can cause a use-after-free or a WARN_ON if the panel's struct device has already been freed. -- commit 0ea9791b2d1689f026240930cee1476a24170351 Author: Albert Esteve <[email protected]> drm/bridge/panel: hold a reference to the wrapped panel drm_panel_bridge_add_typed() stores a pointer to the drm_panel it wraps, but never acquires a reference to it. If the panel device goes away while a panel_bridge still exists, the dangling pointer can be dereferenced through panel_bridge->panel. Acquire a reference in drm_panel_bridge_add_typed() with drm_panel_get() and release it in each teardown path. > 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 isn't a bug introduced by this patch, but could this pass a freed or unbound device pointer to devm_drm_put_bridge()? If the panel device is unbound and unregistered, its struct device might be freed if no other references are held. Even though the struct drm_panel is kept alive by drm_panel_get(), it does not inherently hold a reference to panel->dev. If the host device later removes the bridge via drm_panel_bridge_remove(), passing panel->dev to devm_drm_put_bridge() would dereference a dangling pointer, or trigger a WARN_ON if the device was kept alive but unbound. > + 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] Does this cause a use-after-free of panel_bridge? If the panel device is unbound before the host device, its devres actions drop the bridge's initial reference. Later, when the host device unbinds, devm_drm_panel_bridge_release() calls drm_bridge_remove(). Because that was the final reference, drm_bridge_remove() frees the panel_bridge structure. The immediate next line dereferences panel_bridge->panel. Should we cache the panel pointer in a local variable before calling drm_bridge_remove(), similar to what was done in drm_panel_bridge_remove()? > } -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=2
