Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues: - [High] The HDMI driver acquires the DDC I2C adapter using `of_find_i2c_adapter_by_node()`, which takes a device reference but not a module reference. This allows a privileged user to unload the I2C adapter module while it is still in use, leading to a deadlock during module removal. -- commit c9e2f0cee6fea92e4b3c20dab0df7308240a2d76 Author: Guangshuo Li <[email protected]> drm/mediatek: mtk_hdmi: Fix DDC adapter double put in v2 This commit removes the extra i2c_put_adapter() call in the HDMI v2 remove callback to prevent a double put of the DDC adapter device reference. The reference is managed by a devm action set up during probe. > mtk_hdmi_common_probe() gets the DDC adapter with > of_find_i2c_adapter_by_node() and registers a devm action to release the > adapter device reference with put_device(). > > The HDMI v2 remove callback also calls i2c_put_adapter() on the same DDC > adapter. This is not paired with of_find_i2c_adapter_by_node(): it drops > the adapter device reference before the devm action drops it again, and > it also puts a module reference that was never taken. [Severity: High] This is a pre-existing issue, but does using of_find_i2c_adapter_by_node() leave the driver vulnerable to a deadlock? Since of_find_i2c_adapter_by_node() takes a device reference but not a module reference, it seems a privileged user could unload the I2C adapter module (e.g., rmmod i2c_mt65xx) or unbind it via sysfs while the mtk_hdmi driver is still using it. If that happens, will the I2C core's i2c_del_adapter() wait indefinitely for the device reference to reach zero, causing a deadlock and hanging the unload process? Code snippet for reference from drivers/gpu/drm/mediatek/mtk_hdmi_common.c: mtk_hdmi_dt_parse_pdata() { ... hdmi->ddc_adpt = of_find_i2c_adapter_by_node(i2c_np); ... } Would it be safer to migrate this to of_get_i2c_adapter_by_node() and pair it with i2c_put_adapter() in the devm cleanup action, so that the module reference is properly held? > Remove the extra i2c_put_adapter() call and drop the now-empty HDMI v2 > remove callback. The common devm action releases the adapter device > reference. > > Fixes: 8d0f79886273 ("drm/mediatek: Introduce HDMI/DDC v2 for MT8195/MT8188") > Cc: [email protected] > Signed-off-by: Guangshuo Li <[email protected]> > diff --git a/drivers/gpu/drm/mediatek/mtk_hdmi_v2.c > b/drivers/gpu/drm/mediatek/mtk_hdmi_v2.c > index 7bbf463056c95..ffe456238a2bf 100644 > --- a/drivers/gpu/drm/mediatek/mtk_hdmi_v2.c > +++ b/drivers/gpu/drm/mediatek/mtk_hdmi_v2.c [ ... ] > @@ -1499,13 +1499,6 @@ static int mtk_hdmi_v2_probe(struct platform_device > *pdev) > return 0; > } > > -static void mtk_hdmi_v2_remove(struct platform_device *pdev) > -{ > - struct mtk_hdmi *hdmi = platform_get_drvdata(pdev); > - > - i2c_put_adapter(hdmi->ddc_adpt); > -} > - -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
