Re: [PATCH v4 2/3] drm/bridge: ti-sn65dsi83: Propagate error in sn65dsi83_reset_pipe()

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] The background work `reset_work` is not cancelled when the device is unbound, leading to a Use-After-Free when the workqueue executes the pending work item after the driver data structure has been freed.
- [High] Re-enabling the interrupt after a failed bridge reset creates an interrupt storm and infinite workqueue loop.
- [High] Preexisting double-unlock of connection_mutex in drm_bridge_helper_reset_crtc() triggered during pipeline reset.
--

--- Patch [2]: [PATCH v4 2/3] drm/bridge: ti-sn65dsi83: Propagate error in sn65dsi83_reset_pipe() ---
commit a871b12bfaa133da03afd275986f3c4abe9be9c0
Author: Esben Haabendal <esben@geanix.com>
drm/bridge: ti-sn65dsi83: Propagate error in sn65dsi83_reset_pipe()

This commit modifies sn65dsi83_reset_pipe() to propagate the error code from
drm_bridge_helper_reset_crtc() instead of unconditionally returning 0. This
gives callers the opportunity to handle reset failures.

> diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi83.c b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
> index 0ccfff401b2a6..475224c5a5d83 100644
> --- a/drivers/gpu/drm/bridge/ti-sn65dsi83.c
> +++ b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
> @@ -403,7 +403,7 @@ static int sn65dsi83_reset_pipe(struct sn65dsi83 *sn65dsi83)
>  	drm_modeset_drop_locks(&ctx);
>  	drm_modeset_acquire_fini(&ctx);
>  
> -	return 0;
> +	return err;

[Severity: High]
This is a pre-existing issue, but returning the error here exposes a
problem in drm_bridge_helper_reset_crtc() which is called earlier in
this function.

Does drm_bridge_helper_reset_crtc() suffer from a double-unlock of
connection_mutex?

It manually acquires connection_mutex, then calls:
drm_atomic_get_connector_for_encoder(encoder, ctx)

This helper appears to unconditionally unlock connection_mutex before returning:

drivers/gpu/drm/drm_atomic.c:drm_atomic_get_connector_for_encoder() {
    ...
    ret = drm_modeset_lock(&dev->mode_config.connection_mutex, ctx);
    ...
    drm_modeset_unlock(&dev->mode_config.connection_mutex);
    return out_connector;
}

Then drm_bridge_helper_reset_crtc() explicitly drops it again:

drivers/gpu/drm/drm_bridge_helper.c:drm_bridge_helper_reset_crtc() {
    ...
    connector = drm_atomic_get_connector_for_encoder(encoder, ctx);
    ...
out:
    drm_modeset_unlock(&dev->mode_config.connection_mutex);
    return ret;
}

Could this lead to lock state corruption or kernel panics when the reset path
is executed?

>  }
>  
>  static void sn65dsi83_reset_work(struct work_struct *ws)

[Severity: High]
This isn't a bug introduced by this patch, but now that
sn65dsi83_reset_pipe() propagates errors, will this workqueue re-enable the
interrupt even if the reset failed?

If sn65dsi83_reset_pipe() returns an error, the hardware might not be reset
and the error condition could remain asserted. If we unconditionally call
enable_irq(ctx->irq) at the end of sn65dsi83_reset_work():

drivers/gpu/drm/bridge/ti-sn65dsi83.c:sn65dsi83_reset_work() {
    ...
    ret = sn65dsi83_reset_pipe(ctx);
    if (ret)
        dev_err(ctx->dev, "reset pipe failed %pe\n", ERR_PTR(ret));

    if (ctx->irq)
        enable_irq(ctx->irq);
}

Will the interrupt immediately fire again, causing an interrupt storm and an
infinite workqueue loop?

[Severity: High]
This is another pre-existing issue, but there appears to be a problem with
reset_work during driver removal.

Does sn65dsi83_remove() fail to cancel reset_work before the device is
unbound?

drivers/gpu/drm/bridge/ti-sn65dsi83.c:sn65dsi83_remove() {
    struct sn65dsi83 *ctx = i2c_get_clientdata(client);

    drm_bridge_unplug(&ctx->bridge);
}

If an error interrupt fires and schedules reset_work, and the driver is
unbound before the work executes, will devres free the sn65dsi83 structure
while the workqueue still holds a dangling pointer to it, leading to a
use-after-free?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260810-ti-sn65dsi83-fixes-v4-0-ae9c95c8d162@geanix.com?part=2