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 <[email protected]> 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/[email protected]?part=2
