Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
New issues: - [High] Reusing the `drm_bridge` object across component bind/unbind cycles causes subsequent binds to permanently fail with `-EBUSY`. Pre-existing issues: - [High] Missing cancellation of `hpd_work` in the unbind path leads to a Use-After-Free. -- --- Patch [1]: [PATCH v6 01/16] drm/bridge: synopsys: dw-dp: Fix incorrect resource lifetimes in bind callback --- commit 7f5878cc180f32e03ed02693838ae580e5707a02 Author: Sebastian Reichel <[email protected]> drm/bridge: synopsys: dw-dp: Fix incorrect resource lifetimes in bind callback This commit moves resource allocations to a new probe helper function to fix resource lifetimes on driver removal. It updates the rockchip driver to use the new probe and bind split. > diff --git a/drivers/gpu/drm/bridge/synopsys/dw-dp.c > b/drivers/gpu/drm/bridge/synopsys/dw-dp.c > index 3445c82e6f50e..368e1f85e6253 100644 > --- a/drivers/gpu/drm/bridge/synopsys/dw-dp.c > +++ b/drivers/gpu/drm/bridge/synopsys/dw-dp.c [ ... ] > +int dw_dp_bind(struct dw_dp *dp, struct drm_encoder *encoder) > { [ ... ] > + ret = drm_bridge_attach(encoder, bridge, NULL, > DRM_BRIDGE_ATTACH_NO_CONNECTOR); > + if (ret) { > + dev_err_probe(dev, ret, "Failed to attach bridge\n"); > + goto unregister_aux; > + } [ ... ] > +void dw_dp_unbind(struct dw_dp *dp) > +{ > + disable_irq(dp->irq); [Severity: High] This is a pre-existing issue, but missing a work cancellation here can lead to a use-after-free. While disable_irq() waits for currently executing IRQ handlers to finish, it does not cancel work items that have already been scheduled. If the IRQ handler schedules dp->hpd_work right before disable_irq() runs, the work remains queued. If the component unbind is followed by a driver removal, devm will free the struct dw_dp memory. Won't the asynchronously executing dw_dp_hpd_work() then access freed memory? Could a cancel_work_sync(&dp->hpd_work) be added to safely flush pending work? > phy_exit(dp->phy); > + drm_dp_aux_unregister(&dp->aux); > + drm_bridge_remove(&dp->bridge); [Severity: High] Because the bridge allocation (embedded in struct dw_dp) was moved to dw_dp_probe(), the same bridge object is now reused across component bind/unbind cycles. When drm_bridge_attach() is called during bind, it successfully links the bridge and sets bridge->dev. However, drm_bridge_remove() in this unbind path does not clear this pointer. Doesn't this mean a subsequent bind attempt will permanently fail with -EBUSY when drm_bridge_attach() sees that bridge->dev is already populated from the previous bind? > } -- Sashiko AI review ยท https://sashiko.dev/#/patchset/20260724-synopsys-dw-dp-improvements-v6-0-041d99a19...@collabora.com?part=1
