Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues: - [High] Using `devm_*` APIs in the component `bind` callback coupled with a `drmm_` allocated context causes resource leaks on bind retries and use-after-free vulnerabilities. -- commit 78a45b3c5f7568988aeb46e28d8dcd405d6b493f Author: Cristian Ciocaltea <[email protected]> drm/rockchip: dw_hdmi_qp: Fix invalid drvdata access in PM ops This commit defers publishing the driver data until the component bind callback completes successfully, and clears it during unbind. This ensures that the PM callbacks can rely on a single null check to avoid accessing invalid or freed state. Closes: https://lore.kernel.org/all/[email protected]/ Closes: https://lore.kernel.org/all/[email protected]/ > diff --git a/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c > b/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c > index 9c4cf68d79d50..646a4cb968309 100644 > --- a/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c > +++ b/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c > @@ -683,8 +683,6 @@ static int dw_hdmi_qp_rockchip_bind(struct device *dev, > struct device *master, > if (ret) > return dev_err_probe(hdmi->dev, ret, "Failed to init encoder"); > > - platform_set_drvdata(pdev, hdmi); > - [Severity: High] This is a pre-existing issue, but does using devm_ allocations in a component bind callback risk resource leaks and use-after-free bugs? The component bind function registers resources like clocks, GPIO, PHY, and IRQs using devm_ APIs on the component's platform device (dev). Since these resources are tied to the platform device's lifetime, they won't be freed when the component is unbound or when bind fails and is retried. For example, earlier in dw_hdmi_qp_rockchip_bind(): ret = devm_request_threaded_irq(dev, hdmi->hpd_irq, cfg->ctrl_ops->hardirq_callback, cfg->ctrl_ops->irq_callback, IRQF_NO_AUTOEN, "dw-hdmi-qp-hpd", hdmi); This registers the IRQ against the platform device's lifetime, but passes the hdmi pointer as context. The hdmi structure is allocated using drmm_kzalloc(), which ties it to the DRM master device's lifetime. If the DRM master is unbound or fails to probe, hdmi is freed, but the IRQ remains registered. If an interrupt fires after the master is unbound, can the active IRQ handler in dw_hdmi_qp_rk3588_irq() dereference the freed hdmi pointer? Furthermore, do error paths after IRQ registration return without canceling the delayed work or freeing the devm_ resources, accumulating leaks on repeated bind failures? > hdmi->hdmi = dw_hdmi_qp_bind(pdev, encoder, &plat_data); > if (IS_ERR(hdmi->hdmi)) > return dev_err_probe(hdmi->dev, PTR_ERR(hdmi->hdmi), [ ... ] > @@ -704,6 +704,8 @@ static void dw_hdmi_qp_rockchip_unbind(struct device *dev, > { > struct rockchip_hdmi_qp *hdmi = dev_get_drvdata(dev); > > + dev_set_drvdata(dev, NULL); > + > cancel_delayed_work_sync(&hdmi->hpd_work); > } [Severity: High] This is also a pre-existing issue, but does cleaning up the delayed work here still leave the IRQ registered against the platform device? Because devm_request_threaded_irq() was used, the IRQ isn't freed during unbind. Could a subsequent interrupt trigger a use-after-free by queuing the work embedded within the already freed hdmi struct? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=31
