Re: [PATCH v2 1/3] drm/bridge: ti-sn65dsi83: Fix problem with premature PLL locking

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] The reset_work work item is never cancelled during device removal, leading to a Use-After-Free if executed after devres cleanup.
--

--- Patch [1]: [PATCH v2 1/3] drm/bridge: ti-sn65dsi83: Fix problem with premature PLL locking ---
commit 6d16fdef43813e12caf14d60e8f68b785f57a520
Author: Esben Haabendal <esben@geanix.com>
Subject: drm/bridge: ti-sn65dsi83: Fix problem with premature PLL locking

This commit moves the PLL locking sequence from atomic_pre_enable() to
atomic_enable(), ensuring the DSI HS clock is running before the PLL is
locked. This resolves PLL lock failures seen with certain configurations.

Link: https://lore.kernel.org/all/20260120-mtkdsi-v1-1-b0f4094f3ac3@gmail.com/

> diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi83.c b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
> index 42b451432bbb6..b4b220eee7909 100644
> --- a/drivers/gpu/drm/bridge/ti-sn65dsi83.c
> +++ b/drivers/gpu/drm/bridge/ti-sn65dsi83.c

[ ... ]

> @@ -734,6 +732,7 @@ static void sn65dsi83_atomic_enable(struct drm_bridge *bridge,
>  		sn65dsi83_monitor_start(ctx);
>  	}
>  
> +err_exit:
>  	drm_bridge_exit(idx);
>  }

[Severity: High]
This isn't a bug introduced by this patch, but I noticed a pre-existing
issue in this driver regarding device removal.

Could the reset_work item execute after the device has been unbound?

Looking at sn65dsi83_remove(), it unplugs the bridge but doesn't cancel
the pending work:

static void sn65dsi83_remove(struct i2c_client *client)
{
	struct sn65dsi83 *ctx = i2c_get_clientdata(client);

	drm_bridge_unplug(&ctx->bridge);
}

Because ctx is allocated via devres, the memory is freed immediately after
removal completes. If sn65dsi83_reset_work() is scheduled (e.g., from
hardware errors) and runs after this cleanup, it dereferences the freed
pointer:

static void sn65dsi83_reset_work(struct work_struct *ws)
{
	struct sn65dsi83 *ctx = container_of(ws, struct sn65dsi83, reset_work);
	...
	if (!drm_bridge_enter(&ctx->bridge, &idx))
		return;

Does this race condition lead to a use-after-free if the device is removed
while reset_work is pending?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260715-ti-sn65dsi83-fixes-v2-0-ebc4c3fe29b6@geanix.com?part=1