Re: [PATCH 35/37] drm/bridge: drm_bridge_attach(): don't fail on -EPROBE_DEFER

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Critical] Masking `-EPROBE_DEFER` leaves failed, uninitialized bridges in the active bridge chain, leading to UAF and crashes.
- [High] Unconditionally swallowing `-EPROBE_DEFER` breaks the generic probe deferral mechanism for all bridge resources.
- [Low] The probe deferral diagnostic block in `err_reset_bridge` is now unreachable dead code.
--

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

drm/bridge: drm_bridge_attach(): don't fail on -EPROBE_DEFER

This commit modifies drm_bridge_attach() to bypass the error cleanup path
when a bridge's attach callback returns -EPROBE_DEFER. This is intended to
allow the display pipeline to probe even if a downstream bridge is not yet
connected.

> diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
> index 10af107823902..594cb1468266e 100644
> --- a/drivers/gpu/drm/drm_bridge.c
> +++ b/drivers/gpu/drm/drm_bridge.c
> @@ -593,7 +593,7 @@ int drm_bridge_attach(struct drm_encoder *encoder, struct drm_bridge *bridge,
>  
>  	if (bridge->funcs->attach) {
>  		ret = bridge->funcs->attach(bridge, encoder, flags);
> -		if (ret < 0)
> +		if (ret < 0 && ret != -EPROBE_DEFER)
>  			goto err_reset_bridge;
>  	}

If the bridge's attach callback returns -EPROBE_DEFER, skipping the
err_reset_bridge path will cause the function to return 0 (success) instead
of propagating the error. Does this break the generic probe deferral
mechanism? -EPROBE_DEFER could be returned because any required resource
(such as clocks or regulators) is missing, and returning 0 means the upstream
driver will finish probing without the driver core ever retrying.

Additionally, because the bridge was added to encoder->bridge_chain prior to
calling the attach callback:

	mutex_lock(&encoder->bridge_chain_mutex);
	if (previous)
		list_add(&bridge->chain_node, &previous->chain_node);
	else
		list_add(&bridge->chain_node, &encoder->bridge_chain);
	mutex_unlock(&encoder->bridge_chain_mutex);

If we don't go to err_reset_bridge to remove it from the list, the bridge is
left permanently linked in the active chain even though its attach operation
failed and was rolled back. Will this cause use-after-free issues or crashes
when the DRM core later traverses the chain to invoke pipeline callbacks?

Finally, lower down in this function at the err_reset_bridge label, there is
a check for -EPROBE_DEFER:

err_reset_bridge:
	...
	if (ret != -EPROBE_DEFER)
		DRM_ERROR(...);
	else
		dev_err_probe(encoder->dev->dev, -EPROBE_DEFER, ...);

Since the err_reset_bridge label is now only reachable when ret is strictly
not equal to -EPROBE_DEFER, is the else branch containing dev_err_probe() now
unreachable dead code?

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