Re: [PATCH 02/10] drm/bridge: cdns-mhdp8546: Remove redundant dev_err()

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Use-After-Free in cdns_mhdp_remove() due to active interrupt handler rescheduling work
- [High] Uninitialized workqueue and waitqueue accessed by interrupt handler on probe
- [High] Use-After-Free due to asynchronous firmware loading timeout in cdns_mhdp_remove()
--

--- Patch [2]: [PATCH 02/10] drm/bridge: cdns-mhdp8546: Remove redundant dev_err() ---
commit bcaae5554e2b18efb874ecbcbc1b83f650920be8
Author: Pan Chuang <panchuang@vivo.com>

    drm/bridge: cdns-mhdp8546: Remove redundant dev_err()

This commit removes a redundant dev_err() call after devm_request_threaded_irq()
fails. The genirq subsystem already logs detailed error messages automatically,
making the driver-specific print unnecessary.

> diff --git a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
> index 504a3186ebb3a..f47717bd10798 100644
> --- a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
> +++ b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
> @@ -2343,7 +2343,6 @@ static int cdns_mhdp_probe(struct platform_device *pdev)
>  					cdns_mhdp_irq_handler, IRQF_ONESHOT,
>  					"mhdp8546", mhdp);

[Severity: High]
This is a pre-existing issue, but waitqueues and workqueues accessed by the
interrupt handler are uninitialized at this point.

Because devm_request_threaded_irq() enables the interrupt immediately, an
active interrupt line from the bootloader could trigger cdns_mhdp_irq_handler()
before the driver initializes mhdp->sw_events_wq and mhdp->hpd_work.

Will the interrupt handler trigger a kernel panic due to iterating over
uninitialized waitqueue list heads if the interrupt fires early?

>  	if (ret) {
> -		dev_err(dev, "cannot install IRQ %d\n", irq);
>  		ret = -EIO;
>  		goto plat_fini;
>  	}

[Severity: High]
This is a pre-existing issue, but using devm_request_threaded_irq() without
explicitly disabling the irq creates a use-after-free window in
cdns_mhdp_remove().

Because the devm action frees the IRQ after cdns_mhdp_remove() returns, the
interrupt remains fully active during driver unbind:

drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c:cdns_mhdp_remove() {
	...
	cancel_work_sync(&mhdp->modeset_retry_work);
	flush_work(&mhdp->hpd_work);
	/* Ignoring mhdp->hdcp.check_work and mhdp->hdcp.prop_work here. */
}

Could the active interrupt handler reschedule mhdp->hpd_work after flush_work()
completes, leading to a use-after-free when the devres cleanup subsequently
frees the mhdp structure?

[Severity: High]
This is a pre-existing issue, but is it safe to use a strict 100ms timeout in
cdns_mhdp_remove() while firmware is still loading?

drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c:cdns_mhdp_remove() {
	...
	unsigned long timeout = msecs_to_jiffies(100);
	...
	ret = wait_event_timeout(mhdp->fw_load_wq,
				 mhdp->hw_state == MHDP_HW_READY,
				 timeout);
	...
}

If the asynchronous firmware request takes longer than 100ms (e.g. waiting for
a user-mode helper fallback which times out after 60s), remove() will proceed
and devres will free the mhdp structure.

When the firmware subsystem later invokes cdns_mhdp_fw_cb(), won't it
dereference the freed mhdp context pointer?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260723131649.134127-1-panchuang@vivo.com?part=2