Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues: - [High] The HPD workqueue (adv7511->hpd_work) is not cancelled when the driver is unbound or fails to probe after the IRQ is registered, leading to a Use-After-Free. -- commit 27fb30e849a482fc0f7d55469140b878f0a937a9 Author: Maxime Ripard <[email protected]> drm/bridge: adv7511: Switch to atomic_create_state This commit deprecates the drm_bridge_funcs.atomic_reset callback and its helpers. It switches the adv7511 bridge driver to use atomic_create_state and drm_atomic_helper_bridge_create_state instead. > diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c > b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c > index 02f8f7e78a163..db99e252aeebc 100644 > --- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c > +++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c > @@ -1004,7 +1004,7 @@ static const struct drm_bridge_funcs > adv7511_bridge_funcs = { > .atomic_disable = adv7511_bridge_atomic_disable, > .atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state, > .atomic_destroy_state = drm_atomic_helper_bridge_destroy_state, > - .atomic_reset = drm_atomic_helper_bridge_reset, > + .atomic_create_state = drm_atomic_helper_bridge_create_state, > > .hdmi_tmds_char_rate_valid = adv7511_bridge_hdmi_tmds_char_rate_valid, [Severity: High] This isn't a bug introduced by this patch, but I noticed a pre-existing issue in this driver's error handling and removal paths regarding the HPD workqueue. In adv7511_probe(), an IRQ handler is registered via devm_request_threaded_irq() which can schedule adv7511->hpd_work. If the probe fails after this point, the error path does not cancel the work before returning: drivers/gpu/drm/bridge/adv7511/adv7511_drv.c:adv7511_probe() { ... err_unregister_audio: drm_bridge_remove(&adv7511->bridge); i2c_unregister_device(adv7511->i2c_cec); ... } Similarly, adv7511_remove() does not cancel the work either: drivers/gpu/drm/bridge/adv7511/adv7511_drv.c:adv7511_remove() { struct adv7511 *adv7511 = i2c_get_clientdata(i2c); of_node_put(adv7511->host_node); adv7511_uninit_regulators(adv7511); drm_bridge_remove(&adv7511->bridge); ... } Because devres eventually frees the IRQ but free_irq() does not cancel already scheduled work, could this lead to a use-after-free? If the work remains pending after the final reference to the adv7511 structure is dropped, any executing adv7511_hpd_work() would dereference the freed memory. Should cancel_work_sync(&adv7511->hpd_work) be called in these paths to prevent the work from executing after the structure is freed? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=8
