Re: [PATCH v2 23/37] drm/bridge: ti-tpd12s015: Add atomic bridge state handlers

[sashiko-bot@kernel.org]

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 <mripard@kernel.org>

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/20260707-drm-all-atomic-bridges-v2-0-21d03cbca446@kernel.org?part=23