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

[Esben Haabendal <esben@geanix.com>]

<sashiko-bot@kernel.org> writes:

> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] Leaking an SRCU read lock on the error path in `sn65dsi83_reset_work()` due to an early return without calling `drm_bridge_exit()`.
> --
>
> --- Patch [1]: [PATCH v3 1/3] drm/bridge: ti-sn65dsi83: Propagate error in sn65dsi83_reset_pipe() ---
> commit d19637cc30dea31a13c6747fe2aa3733116ec5e5
> Author: Esben Haabendal <esben@geanix.com>
>
> drm/bridge: ti-sn65dsi83: Propagate error in sn65dsi83_reset_pipe()
>
> Propagate the error code from drm_bridge_helper_reset_crtc(), so that
> callers has a chance to handle the failure.
>
>> diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi83.c b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
>> index 42b451432bbb6..476f255bd34dd 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;
>>  }
>>
>>  static void sn65dsi83_reset_work(struct work_struct *ws)
>
> [Severity: High]
> Does propagating the error here cause an SRCU read lock leak?
>
> By propagating the error from sn65dsi83_reset_pipe(), it is now possible
> to hit the early return path in sn65dsi83_reset_work():
>
> 	if (!drm_bridge_enter(&ctx->bridge, &idx))
> 		return;
>
> 	/* Reset the pipe */
> 	ret = sn65dsi83_reset_pipe(ctx);
> 	if (ret) {
> 		dev_err(ctx->dev, "reset pipe failed %pe\n", ERR_PTR(ret));
> 		return;
> 	}
>
> Because this early return skips the call to drm_bridge_exit(idx) at the
> end of the function, wouldn't this leak the lock and cause a system hang
> during drm_bridge_unplug() when synchronize_srcu() is called?

Yes. This is resolved in patch 2. I will reorder the patches in next
version.

/Esben