Re: [PATCH v5 1/2] drm/bridge: ti-sn65dsi83: Fix error handling in sn65dsi83_reset_work()

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Missing `cancel_work_sync` for `reset_work` leads to a use-after-free on driver unbind.
--

commit 47920da4c643294656a651a954618ec33114a7d9
Author: Esben Haabendal <esben@geanix.com>

drm/bridge: ti-sn65dsi83: Fix error handling in sn65dsi83_reset_work()

The commit reintroduces a non-zero return value from sn65dsi83_reset_pipe()
on error to prevent erroneous IRQ re-enabling during failures. It also
updates sn65dsi83_reset_work() to properly exit the DRM bridge critical
section by always calling drm_bridge_exit().

> diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi83.c b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
> index 42b451432bbb6..9f530cd05d2f3 100644
> --- a/drivers/gpu/drm/bridge/ti-sn65dsi83.c
> +++ b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
[ ... ]
> @@ -419,11 +419,13 @@ static void sn65dsi83_reset_work(struct work_struct *ws)
>  	ret = sn65dsi83_reset_pipe(ctx);
>  	if (ret) {
>  		dev_err(ctx->dev, "reset pipe failed %pe\n", ERR_PTR(ret));
> -		return;
> +		goto bridge_exit;
>  	}
> +
>  	if (ctx->irq)
>  		enable_irq(ctx->irq);
>  
> +bridge_exit:
>  	drm_bridge_exit(idx);
>  }

[Severity: High]
This isn't a bug introduced by this patch, but I noticed a potential
use-after-free related to the reset_work structure that is being modified here.

During driver unbind (e.g., via sysfs), sn65dsi83_remove() unplugs the bridge
but does not explicitly call cancel_work_sync(&ctx->reset_work). After
sn65dsi83_remove() returns, devres automatically frees the IRQ and the ctx
structure.

If an interrupt had previously scheduled the work, the work item might remain
in the queue and eventually execute sn65dsi83_reset_work(), which will attempt
to access the freed ctx pointer (e.g., when calling
drm_bridge_enter(&ctx->bridge, &idx)).

Should sn65dsi83_remove() be updated to include a call to
cancel_work_sync(&ctx->reset_work) to ensure the work is fully cancelled
before devres tears down the allocated memory?

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