Re: [PATCH 08/11] drm/mcde: dsi: simplify device_node management using scoped for_each variant

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

Pre-existing issues:
- [High] Missing `drm_bridge_remove()` for `d->bridge` during unbind causes a memory leak and use-after-free risk.
- [High] Deferred probing for panels is broken by mapping `-EPROBE_DEFER` to `-EINVAL`.
--

--- Patch [8]: [PATCH 08/11] drm/mcde: dsi: simplify device_node management using scoped for_each variant ---
commit 6cf52153aa2a545f24c563456ea7265e0b307010
Author: Luca Ceresoli <[email protected]>

drm/mcde: dsi: simplify device_node management using scoped for_each variant

Before further modifying this function, simpify the struct device_node
management using the scoped variat of for_each_available_child_of_node().

> diff --git a/drivers/gpu/drm/mcde/mcde_dsi.c b/drivers/gpu/drm/mcde/mcde_dsi.c
> index 6943725818407..ce078aa3910f2 100644
> --- a/drivers/gpu/drm/mcde/mcde_dsi.c
> +++ b/drivers/gpu/drm/mcde/mcde_dsi.c

[ ... ]

> @@ -1104,7 +1103,7 @@ static int mcde_dsi_bind(struct device *dev, struct device *master,
>  	}
>  
>  	/* Look for a panel as a child to this node */
> -	for_each_available_child_of_node(dev->of_node, child) {
> +	for_each_available_child_of_node_scoped(dev->of_node, child) {
>  		panel = of_drm_find_panel(child);
>  		if (IS_ERR(panel)) {
>  			dev_err(dev, "failed to find panel try bridge (%ld)\n",
> @@ -1114,15 +1113,12 @@ static int mcde_dsi_bind(struct device *dev, struct device *master,
>  			bridge = of_drm_find_and_get_bridge(child);
>  			if (!bridge) {
>  				dev_err(dev, "failed to find bridge\n");
> -				of_node_put(child);
>  				return -EINVAL;

[Severity: High]
This is a pre-existing issue, but does returning -EINVAL here break
deferred probing?

If the panel driver is not yet probed, of_drm_find_panel() returns
ERR_PTR(-EPROBE_DEFER). This error path intercepts that by checking
IS_ERR(panel), setting panel = NULL, and falling back to
of_drm_find_and_get_bridge(). Since the panel's bridge is also not
registered yet, this returns NULL.

Finding no bridge, the driver then unconditionally returns -EINVAL instead
of propagating the -EPROBE_DEFER error. This causes the component bind to
fail permanently rather than deferring, which can lead to a loss of
display output if the MCDE driver probes before the panel driver.

>  			}
>  		}
>  
> -		if (panel || bridge) {
> -			of_node_put(child);
> +		if (panel || bridge)
>  			break;
> -		}
>  	}

[Severity: High]
This isn't a bug introduced by this patch, but is there a missing
drm_bridge_remove() in mcde_dsi_unbind() for the bridge registered in
mcde_dsi_bind()?

The driver registers d->bridge using drm_bridge_add() further down in
mcde_dsi_bind(), which acquires a reference and links it into the global
bridge_list.

However, in mcde_dsi_unbind(), the driver fails to call
drm_bridge_remove(), meaning the bridge is never unlinked and its reference
count never drops to zero.

Because d->bridge remains in the global bridge_list even after the device
is unbound and its devres-managed resources are freed, subsequent
iterations of the bridge_list by other drivers could dereference a stale
pointer. Can this cause a use-after-free and potential kernel panic?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=8