Re: [PATCH 20/37] drm/bridge: samsung-dsim: attach: return -EPROBE_DEFER is next bridge not yet available

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Double fetch of `dsi->bridge.next_bridge` in `samsung_dsim_attach()` causes a race condition leading to unintended `-EINVAL` errors or Use-After-Free.
--

commit b9977e0ebaa4dd776acdd10e8085cc7df8ae862c
Author: Luca Ceresoli <luca.ceresoli@bootlin.com>

drm/bridge: samsung-dsim: attach: return -EPROBE_DEFER is next bridge not yet available

When samsung_dsim_attach() is called without a known next_bridge, it previously
returned -EINVAL. This commit changes it to return -EPROBE_DEFER instead, in
preparation for bridge hotplug support.

> drm/bridge: samsung-dsim: attach: return -EPROBE_DEFER is next bridge not yet available

This isn't a bug, but there is a typo in the subject line ("is" instead of
"if").

> diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c b/drivers/gpu/drm/bridge/samsung-dsim.c
> index 555e343b386d0..83c74d57d0d44 100644
> --- a/drivers/gpu/drm/bridge/samsung-dsim.c
> +++ b/drivers/gpu/drm/bridge/samsung-dsim.c
> @@ -1828,6 +1828,9 @@ static int samsung_dsim_attach(struct drm_bridge *bridge,
>  {
>  	struct samsung_dsim *dsi = bridge_to_dsi(bridge);
>  
> +	if (!dsi->bridge.next_bridge)
> +		return -EPROBE_DEFER;
> +
>  	return drm_bridge_attach(encoder, dsi->bridge.next_bridge, bridge,
>  				 flags);

Can this double fetch of dsi->bridge.next_bridge cause a race condition
leading to unintended -EINVAL errors or a use-after-free?

The commit message states this change is in preparation for bridge hotplug,
meaning hot-unplug events could occur concurrently.

If a detach executes concurrently during an unplug event, it could clear
dsi->bridge.next_bridge between the if check and the call to
drm_bridge_attach(). This would cause the first fetch to see a valid pointer,
but the second fetch to read NULL, resulting in drm_bridge_attach() returning
-EINVAL instead of -EPROBE_DEFER.

Furthermore, even if the value is fetched into a local variable to prevent the
double fetch, could the bridge be freed by the concurrent detach before
drm_bridge_attach() takes its own reference?

Does this require proper synchronization, such as holding a lock while
checking and obtaining a reference to the bridge?

>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260519-drm-bridge-hotplug-v1-0-45e2bdb3dfb4@bootlin.com?part=20