Re: [PATCH v10 01/21] drm/bridge: synopsys: dw-dp: Register DP AUX on bridge attach

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

New issues:
- [High] Moving `drm_dp_aux_unregister()` to the bridge `detach` callback leaves the AUX character device exposed to userspace after the component is unbound, leading to a Use-After-Free and hardware fault.

Pre-existing issues:
- [High] Incorrect use of `devm_` APIs on the component device in `dw_dp_bind` causes permanent resource leaks and a fatal NULL pointer dereference on unbind.
--

--- Patch [1]: [PATCH v10 01/21] drm/bridge: synopsys: dw-dp: Register DP AUX on bridge attach ---
commit 1118ca21508c977536ef793f4a69acdf62b0bbbb
Author: Sebastian Reichel <sebastian.reichel-ZGY8ohtN/[email protected]>

drm/bridge: synopsys: dw-dp: Register DP AUX on bridge attach

This commit delays the DP AUX device registration until the bridge attach
callback, ensuring the DRM device is ready and the bridge is initialized.
It also moves the unregistration from the component unbind callback to the
bridge detach callback.

> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-dp.c b/drivers/gpu/drm/bridge/synopsys/dw-dp.c
> index 3445c82e6f50e..112a499113095 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-dp.c
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-dp.c
[ ... ]
> @@ -1813,7 +1813,36 @@ static struct drm_bridge_state *dw_dp_bridge_atomic_duplicate_state(struct drm_b
[ ... ]
> +static void dw_dp_bridge_detach(struct drm_bridge *bridge)
> +{
> +	struct dw_dp *dp = bridge_to_dp(bridge);
> +
> +	drm_dp_aux_unregister(&dp->aux);
> +}

[Severity: High]
Does moving drm_dp_aux_unregister() to the bridge detach callback leave the AUX
character device exposed to userspace after the component is unbound?

If a userspace process opens the DRM device, holding a reference, and the
dw_dp driver is unbound (e.g., via sysfs), the component unbind callback will
now do nothing, leaving the AUX device registered.

The component devres handlers will then execute, freeing dp->regmap and
dp->phy, and releasing the MMIO mappings.

Since dw_dp_bridge_detach() relies on drmm cleanup, its execution is delayed
until the userspace process closes the DRM file descriptor.

If userspace initiates a read or write on the still-registered
/dev/drm_dp_auxN character device during this time, dw_dp_aux_transfer()
could attempt to write to the unmapped and freed dp->regmap, resulting in a
hardware fault or kernel panic.

[ ... ]
> @@ -2065,37 +2084,33 @@ struct dw_dp *dw_dp_bind(struct device *dev, struct drm_encoder *encoder,
[ ... ]
>  	ret = devm_request_threaded_irq(dev, dp->irq, NULL, dw_dp_irq,
>  					IRQF_ONESHOT, dev_name(dev), dp);

[Severity: High]
This is a pre-existing issue, but does using devm_ APIs like
devm_request_threaded_irq() inside dw_dp_bind() tie the resource to the
component device's probe/remove lifecycle rather than the bind/unbind
lifecycle?

When the master DRM driver unbinds, the component .unbind hook is called, but
devres on the component device does not run because the component driver
itself was not removed.

This leaves the IRQ handler active. When the master DRM device's drmm cleanup
runs, it triggers drm_bridge_detach(), which explicitly sets dp->bridge.dev
to NULL.

If a hotplug event then triggers the active IRQ, it schedules dw_dp_hpd_work():

drivers/gpu/drm/bridge/synopsys/dw-dp.c:dw_dp_hpd_work() {
    ...
    drm_helper_hpd_irq_event(dp->bridge.dev);
    ...
}

Will this dereference the NULL dp->bridge.dev pointer and crash the kernel?

Additionally, if the master DRM driver attempts to rebind, will dw_dp_bind()
fail with -EBUSY when calling devm_request_threaded_irq() because the previous
allocation leaked?

>  	if (ret) {
>  		dev_err_probe(dev, ret, "failed to request irq\n");
> -		goto unregister_aux;
> +		return ERR_PTR(ret);
>  	}
>  
>  	return dp;
> -
> -unregister_aux:
> -	drm_dp_aux_unregister(&dp->aux);
> -	return ERR_PTR(ret);
>  }
>  EXPORT_SYMBOL_GPL(dw_dp_bind);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260806-synopsys-dw-dp-improvements-v10-0-d84a26c303d6-ZGY8ohtN/[email protected]?part=1