Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues: - [High] Missing IRQ disablement and workqueue cancellation in driver removal and probe error paths. -- commit e3d22519d26e84f17c45f704d4c2a1fc871e1036 Author: Cristian Ciocaltea <[email protected]> drm/bridge: adv7511: Advertise HDMI 1.2 capabilities Bridges using DRM_BRIDGE_OP_HDMI are now expected to provide a supported HDMI version. This commit advertises HDMI 1.2 as the supported version and sets the bridge's max_tmds_char_rate. It also drops the redundant adv7511_bridge_hdmi_tmds_char_rate_valid() hook. > diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c > b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c > index 038b852405b54..c3ef44074477b 100644 > --- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c > +++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c [ ... ] > @@ -1359,6 +1341,8 @@ static int adv7511_probe(struct i2c_client *i2c) > > adv7511->bridge.vendor = "Analog"; > adv7511->bridge.product = adv7511->info->name; > + adv7511->bridge.supported_hdmi_ver = HDMI_VERSION_1_2; > + adv7511->bridge.max_tmds_char_rate = 1000ULL * > adv7511->info->max_mode_clock_khz; > > #ifdef CONFIG_DRM_I2C_ADV7511_AUDIO [Severity: High] This isn't a bug introduced by this patch, but I noticed a potential use-after-free in the error handling and driver removal paths of this driver. In adv7511_probe(), the driver registers a shared interrupt via devm_request_threaded_irq() and initializes adv7511->hpd_work. However, in the err_unregister_audio probe error path, the devm-managed IRQ can still fire because the resources are manually freed before the devm cleanup runs: adv7511_probe() { ... err_unregister_audio: drm_bridge_remove(&adv7511->bridge); i2c_unregister_device(adv7511->i2c_cec); clk_disable_unprepare(adv7511->cec_clk); err_i2c_unregister_packet: i2c_unregister_device(adv7511->i2c_packet); ... } Similarly, adv7511_remove() manually unregisters I2C devices while the devm-managed IRQ remains active until devres cleanup: adv7511_remove() { ... i2c_unregister_device(adv7511->i2c_cec); clk_disable_unprepare(adv7511->cec_clk); i2c_unregister_device(adv7511->i2c_packet); i2c_unregister_device(adv7511->i2c_edid); ... } Could a hotplug or CEC interrupt firing during or just after these cleanup paths attempt to access the freed I2C clients? Furthermore, since cancel_work_sync() is never called for hpd_work, could any scheduled work execute after the adv7511 structure has been freed by devres, leading to memory corruption or a kernel panic? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=50
