Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Pre-existing issues: - [High] Use-After-Release of devm-managed GPIOs if the bridge is accessed after the platform device unbinds. - [High] Race condition where the HPD IRQ is active before its associated mutexes are initialized, and remains active after they are destroyed. -- --- Patch [23]: [PATCH v2 23/37] drm/bridge: ti-tpd12s015: Add atomic bridge state handlers --- commit 8627a6c33db1265257320b25744b5861c235ec83 Author: Maxime Ripard <[email protected]> drm/bridge: ti-tpd12s015: Add atomic bridge state handlers The ti-tpd12s015 bridge doesn't implement any enable or disable callbacks, but is still missing the atomic state handlers. In order to remove the legacy bridge callback support from the DRM bridge core, add them. > diff --git a/drivers/gpu/drm/bridge/ti-tpd12s015.c > b/drivers/gpu/drm/bridge/ti-tpd12s015.c > index 136e47ad1a10d..e7b172ab7f552 100644 > --- a/drivers/gpu/drm/bridge/ti-tpd12s015.c > +++ b/drivers/gpu/drm/bridge/ti-tpd12s015.c [ ... ] > @@ -96,6 +97,9 @@ static void tpd12s015_hpd_disable(struct drm_bridge *bridge) > } > > static const struct drm_bridge_funcs tpd12s015_bridge_funcs = { > + .atomic_create_state = drm_atomic_helper_bridge_create_state, > + .atomic_destroy_state = drm_atomic_helper_bridge_destroy_state, > + .atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state, > .attach = tpd12s015_attach, > .detach = tpd12s015_detach, > .detect = tpd12s015_bridge_detect, [Severity: High] This isn't a bug introduced by this patch, but could this detect callback race with driver unbinding and cause a use-after-free? The DRM core holds a reference to the bridge, allowing it to outlive the platform device's devres lifecycle. However, the GPIO descriptors are allocated using devm_gpiod_get_index() in tpd12s015_probe(): gpio = devm_gpiod_get_index(&pdev->dev, NULL, 2, GPIOD_IN); If the platform device unbinds, devres frees the GPIOs. If the still-attached bridge's detect callback (tpd12s015_detect) is then invoked by the DRM core, it accesses the freed GPIO: if (gpiod_get_value_cansleep(tpd->hpd_gpio)) Will this cause memory corruption or a kernel oops if the bridge is accessed after the device unbinds? [Severity: High] This is another pre-existing issue, but is there a potential race condition with the HPD IRQ and bridge mutex initialization? In tpd12s015_probe(), the HPD IRQ is requested via devm before drm_bridge_add() initializes bridge->hpd_mutex: ret = devm_request_threaded_irq(&pdev->dev, tpd->hpd_irq, NULL, tpd12s015_hpd_isr, ...); If an interrupt fires immediately, tpd12s015_hpd_isr() calls drm_bridge_hpd_notify(), which attempts to lock the uninitialized hpd_mutex. Similarly, in tpd12s015_remove(): drm_bridge_remove(&tpd->bridge); drm_bridge_remove() is called to destroy the mutexes while the devm-managed IRQ is still active. Can this lead to locking an uninitialized or destroyed mutex if the HPD pin toggles during probe or removal? > .hpd_enable = tpd12s015_hpd_enable, > .hpd_disable = tpd12s015_hpd_disable, > }; -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=23
