> [Severity: High] This is a pre-existing issue, but by adding this > error path, the patch exposes it further. Does returning an error > here without cancelling hdmi->hpd_work risk a use-after-free?
Thanks for taking a look. The window you describe does exist, but it is not introduced or widened by this patch: hdmi->hpd_work is initialized and the HPD interrupt requested early in dw_hdmi_qp_rockchip_bind(), and the function already has several error returns after the interrupt is live -- encoder init, dw_hdmi_qp_bind() and drm_bridge_connector_init() -- none of which cancel the delayed work. This patch adds one more return immediately after the last of those, following the function's existing error-handling pattern. Since the cancellation gap is common to all of bind()'s error paths, I think the right fix is a separate patch addressing all of them at once (e.g. switching to devm_delayed_work_autocancel() or a devres action) rather than special-casing the new return. I'm happy to send that as a follow-up. Igor On Tue, Jul 14, 2026 at 10:40 PM <[email protected]> wrote: > > Thank you for your contribution! Sashiko AI review found 1 potential issue(s) > to consider: > > Pre-existing issues: > - [High] Returning an error from `dw_hdmi_qp_rockchip_bind()` without > cancelling `hdmi->hpd_work` leads to a Use-After-Free when the delayed work > or HPD interrupt fires. > -- > > --- Patch [1]: [PATCH] drm/rockchip: dw_hdmi_qp: attach the HDMI Colorspace > connector property --- > commit 77d2f0e8bc60d50755aec22dee119ad08bb92b3c > Author: Igor Paunovic <[email protected]> > > drm/rockchip: dw_hdmi_qp: attach the HDMI Colorspace connector property > > This commit creates and attaches the standard HDMI colorspace property > after the bridge connector is initialised. This allows userspace to request > BT.2020 colorimetry and enables wide-gamut/HDR output on RK3588. > > > diff --git a/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c > > b/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c > > --- a/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c > > +++ b/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c > [ ... ] > > @@ -695,6 +695,20 @@ static int dw_hdmi_qp_rockchip_bind(struct device > > *dev, struct device *master, > > return dev_err_probe(hdmi->dev, PTR_ERR(connector), > > "Failed to init bridge connector\n"); > > > > + /* > > + * Attach the HDMI Colorspace property. The HDMI connector framework > > + * already consumes conn_state->colorspace for the AVI infoframe > > + * (drm_hdmi_avi_infoframe_colorimetry()), and HDR_OUTPUT_METADATA and > > + * max bpc are attached by drmm_connector_hdmi_init(), but without the > > + * Colorspace property userspace cannot request BT.2020 signalling, > > + * which keeps wide-gamut/HDR output unavailable. Passing 0 selects > > + * the full HDMI colorspace set defined by the core. > > + */ > > + ret = drm_mode_create_hdmi_colorspace_property(connector, 0); > > + if (ret) > > + return ret; > > [Severity: High] > This is a pre-existing issue, but by adding this error path, the patch > exposes it further. Does returning an error here without cancelling > hdmi->hpd_work risk a use-after-free? > > If a hardware HPD interrupt fires right before the devres cleanup > unregisters the IRQ, hdmi->hpd_work can get queued. The DRM master > teardown then frees hdmi (which was allocated via drmm_kzalloc), > leaving the delayed work to execute later with a dangling pointer: > > drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c:dw_hdmi_qp_rk3588_hpd_work() { > struct rockchip_hdmi_qp *hdmi = container_of(work, > struct rockchip_hdmi_qp, > hpd_work.work); > struct drm_device *drm = hdmi->encoder.encoder.dev; > ... > } > > Could we ensure that the delayed work is properly cancelled before > returning errors from dw_hdmi_qp_rockchip_bind()? > > > + drm_connector_attach_colorspace_property(connector); > > > > return 0; > > } > > -- > Sashiko AI review · > https://sashiko.dev/#/patchset/[email protected]?part=1
