[PATCH] drm/exynos: hdmi: take i2c adapter module reference

[PATCH] drm/exynos: hdmi: take i2c adapter module reference

[Johan Hovold]

The i2c subsystem currently blocks during adapter deregistration
whenever there are consumers holding a reference.

Switch to using of_get_i2c_adapter_by_node() which also takes a
reference to the adapter module so that an attempt to unload the module
while in use fails gracefully instead of blocking uninterruptibly.

Signed-off-by: Johan Hovold <johan@kernel.org>
---
 drivers/gpu/drm/exynos/exynos_hdmi.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c b/drivers/gpu/drm/exynos/exynos_hdmi.c
index 09b2cabb236f..b971b7098a4d 100644
--- a/drivers/gpu/drm/exynos/exynos_hdmi.c
+++ b/drivers/gpu/drm/exynos/exynos_hdmi.c
@@ -1913,7 +1913,7 @@ static int hdmi_get_ddc_adapter(struct hdmi_context *hdata)
 		return -ENODEV;
 	}
 
-	adpt = of_find_i2c_adapter_by_node(np);
+	adpt = of_get_i2c_adapter_by_node(np);
 	of_node_put(np);
 
 	if (!adpt) {
@@ -2069,7 +2069,7 @@ static int hdmi_probe(struct platform_device *pdev)
 	if (hdata->regs_hdmiphy)
 		iounmap(hdata->regs_hdmiphy);
 err_ddc:
-	put_device(&hdata->ddc_adpt->dev);
+	i2c_put_adapter(hdata->ddc_adpt);
 
 	return ret;
 }
@@ -2094,7 +2094,7 @@ static void hdmi_remove(struct platform_device *pdev)
 	if (hdata->regs_hdmiphy)
 		iounmap(hdata->regs_hdmiphy);
 
-	put_device(&hdata->ddc_adpt->dev);
+	i2c_put_adapter(hdata->ddc_adpt);
 
 	drm_bridge_put(hdata->bridge);
 
-- 
2.54.0

Re: [PATCH] drm/exynos: hdmi: take i2c adapter module reference

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

Pre-existing issues:
- [High] Use-after-free on error paths in probe due to un-cancelled delayed work.
- [High] Resource leak of drm_bridge in probe error paths.
- [High] Resource leak of DRM connector and CEC notifier if drm_bridge_attach fails.
--

--- Patch [1]: [PATCH] drm/exynos: hdmi: take i2c adapter module reference ---
commit 63541a71709f0db259d49c1fafb2342af0cef43e
Author: Johan Hovold <johan@kernel.org>
drm/exynos: hdmi: take i2c adapter module reference

This patch changes the i2c adapter retrieval to use
of_get_i2c_adapter_by_node() instead of of_find_i2c_adapter_by_node()
so it takes a module reference to prevent uninterruptible blocking
during adapter deregistration. The removal path is updated to use
i2c_put_adapter() instead of put_device().

> diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c b/drivers/gpu/drm/exynos/exynos_hdmi.c
> index 09b2cabb236f3..b971b7098a4d9 100644
> --- a/drivers/gpu/drm/exynos/exynos_hdmi.c
> +++ b/drivers/gpu/drm/exynos/exynos_hdmi.c
[ ... ]
> @@ -2069,7 +2069,7 @@ static int hdmi_probe(struct platform_device *pdev)
>  	if (hdata->regs_hdmiphy)
>  		iounmap(hdata->regs_hdmiphy);
>  err_ddc:
> -	put_device(&hdata->ddc_adpt->dev);
> +	i2c_put_adapter(hdata->ddc_adpt);

[Severity: High]
This isn't a bug introduced by this patch, but does the error path here leave
a pending delayed work item that could cause a use-after-free?

In hdmi_probe(), hdata->hotplug_work is initialized and an IRQ handler is
registered via devm_request_threaded_irq(). If the IRQ triggers during the
remainder of the probe function, the handler will schedule the delayed work.

If probe subsequently fails, devres automatically disables and frees the IRQ,
and then frees hdata. However, because the error paths do not call
cancel_delayed_work_sync(), the scheduled work could remain on the timer or
workqueue and eventually execute, accessing the already freed hdata memory.

[Severity: High]
This isn't a bug introduced by this patch, but is there a resource leak of
the drm_bridge in these error paths?

Earlier in hdmi_probe(), hdmi_resources_init() acquires a bridge reference via
of_drm_find_and_get_bridge() and stores it in hdata->bridge. If hdmi_probe()
fails and takes this error path, it seems drm_bridge_put(hdata->bridge) is
never called, leaking the reference and permanently pinning the bridge module.

>  
>  	return ret;
>  }

[Severity: High]
This is a pre-existing issue, but can hdmi_create_connector() leak resources
if drm_bridge_attach() fails?

In hdmi_create_connector(), if drm_bridge_attach() fails, ret is set to an
error code, but the function does not return immediately:

drivers/gpu/drm/exynos/exynos_hdmi.c:hdmi_create_connector() {
    ...
	if (hdata->bridge)
		ret = drm_bridge_attach(encoder, hdata->bridge, NULL, 0);

	cec_fill_conn_info_from_drm(&conn_info, connector);

	hdata->notifier = cec_notifier_conn_register(hdata->dev, NULL,
						     &conn_info);
    ...
	return ret;
}

If the CEC registration succeeds, the function returns the error code from
drm_bridge_attach(). The caller observes the error and aborts, but since
hdmi_connector_destroy() is never called, do the DRM connector and CEC
notifier remain registered and leaked?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260716131802.1564776-1-johan@kernel.org?part=1