Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] drm/rockchip: dw_hdmi: UAF in dw_hdmi_rockchip_resume_early() on stale drvdata --
commit c6209b9cde57945de5a9c2033ae8339d9c3026b8 Author: Jonas Karlman <[email protected]> drm/rockchip: dw_hdmi: Use resume_early pm ops for system suspend This commit switches the dw-hdmi Rockchip glue driver to use the resume_early PM callback so that dw_hdmi_resume() runs before rockchip_drm_sys_resume() during system resume. It also adds a NULL check on the driver data pointer in dw_hdmi_rockchip_resume_early(). Link: https://patch.msgid.link/[email protected] > diff --git a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c > b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c > index 727cdf768161a..ff34aeef3a6ff 100644 > --- a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c > +++ b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c > @@ -635,17 +635,18 @@ static void dw_hdmi_rockchip_remove(struct > platform_device *pdev) > component_del(&pdev->dev, &dw_hdmi_rockchip_ops); > } > > -static int __maybe_unused dw_hdmi_rockchip_resume(struct device *dev) > +static int __maybe_unused dw_hdmi_rockchip_resume_early(struct device *dev) > { > struct rockchip_hdmi *hdmi = dev_get_drvdata(dev); > > - dw_hdmi_resume(hdmi->hdmi); > + if (hdmi) > + dw_hdmi_resume(hdmi->hdmi); [Severity: High] Is checking if (hdmi) sufficient to guarantee that hdmi and hdmi->hdmi are valid here? In drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c, hdmi is allocated with devm_kzalloc() inside dw_hdmi_rockchip_bind(), which runs inside the component devres group opened by component_bind(). dw_hdmi_rockchip_bind() sets the driver data before calling dw_hdmi_bind(), and does not clear it on failure: platform_set_drvdata(pdev, hdmi); hdmi->hdmi = dw_hdmi_bind(pdev, encoder, plat_data); if (IS_ERR(hdmi->hdmi)) { ret = PTR_ERR(hdmi->hdmi); goto err_bind; } return 0; err_bind: drm_encoder_cleanup(encoder); return ret; When dw_hdmi_bind() fails (for example with -EPROBE_DEFER when the DDC I2C adapter is not ready), component_bind() calls devres_release_group(), which frees hdmi and disables the devm-managed clocks and regulators while leaving dev->driver_data as a non-NULL dangling pointer. Similarly, dw_hdmi_rockchip_unbind() does not clear dev->driver_data before component_unbind() calls devres_release_group(): static void dw_hdmi_rockchip_unbind(struct device *dev, struct device *master, void *data) { struct rockchip_hdmi *hdmi = dev_get_drvdata(dev); dw_hdmi_unbind(hdmi->hdmi); drm_encoder_cleanup(&hdmi->encoder.encoder); } If a system suspend/resume occurs after a failed bind or after component unbind, wouldn't dev_get_drvdata(dev) in dw_hdmi_rockchip_resume_early() return a non-NULL dangling pointer and pass a freed or ERR_PTR hdmi->hdmi to dw_hdmi_resume() while the HDMI clocks and regulators are disabled? > > return 0; > } > > static const struct dev_pm_ops dw_hdmi_rockchip_pm = { > - SET_SYSTEM_SLEEP_PM_OPS(NULL, dw_hdmi_rockchip_resume) > + SET_LATE_SYSTEM_SLEEP_PM_OPS(NULL, dw_hdmi_rockchip_resume_early) > }; > > struct platform_driver dw_hdmi_rockchip_pltfm_driver = { നേരിട്ട് -- Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1
