[PATCH] drm/bridge: anx78xx: fix potential use-after-free on device removal

[PATCH] drm/bridge: anx78xx: fix potential use-after-free on device removal

[Fan Wu]

The HPD and INTP interrupts are requested with
devm_request_threaded_irq(), so they stay armed until the devm cleanup
runs after ->remove() has returned.  anx78xx_i2c_remove() unregisters
the dummy i2c clients and frees the cached EDID before that, and both
threaded handlers access those clients through their regmaps, so a
handler running in this window can cause a use-after-free, or a
double-free of the EDID when a cable is unplugged.

Free both interrupts with devm_free_irq() before the dummy clients are
unregistered.  free_irq() waits for a running threaded handler to
finish, so the handlers can no longer race with the teardown.  The
probe error path taken when the INTP request fails unregisters the
dummy clients with the HPD interrupt still armed; free it there too.

This issue was found by an in-house static analysis tool.

Fixes: 0647e7dd3f7a ("drm/bridge: Add Analogix anx78xx support")
Cc: stable@vger.kernel.org
Assisted-by: Codex:gpt-5.6
Signed-off-by: Fan Wu <fanwu01@zju.edu.cn>
---
 drivers/gpu/drm/bridge/analogix/analogix-anx78xx.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/gpu/drm/bridge/analogix/analogix-anx78xx.c b/drivers/gpu/drm/bridge/analogix/analogix-anx78xx.c
index ba0fc149a9e7..a5f4ecb96498 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix-anx78xx.c
+++ b/drivers/gpu/drm/bridge/analogix/analogix-anx78xx.c
@@ -1304,6 +1304,7 @@ static int anx78xx_i2c_probe(struct i2c_client *client)
 					"anx78xx-intp", anx78xx);
 	if (err) {
 		DRM_ERROR("Failed to request INTP threaded IRQ: %d\n", err);
+		devm_free_irq(&client->dev, pdata->hpd_irq, anx78xx);
 		goto err_poweroff;
 	}
 
@@ -1327,6 +1328,14 @@ static void anx78xx_i2c_remove(struct i2c_client *client)
 {
 	struct anx78xx *anx78xx = i2c_get_clientdata(client);
 
+	/*
+	 * The threaded IRQ handlers access the dummy I2C clients through
+	 * regmap and may free the cached EDID, so stop them before those
+	 * resources are freed below.
+	 */
+	devm_free_irq(&client->dev, anx78xx->pdata.hpd_irq, anx78xx);
+	devm_free_irq(&client->dev, anx78xx->pdata.intp_irq, anx78xx);
+
 	drm_bridge_remove(&anx78xx->bridge);
 
 	unregister_i2c_dummy_clients(anx78xx);
-- 
2.34.1

Re: [PATCH] drm/bridge: anx78xx: fix potential use-after-free on device removal

[sashiko-bot]

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

Pre-existing issues:
- [Critical] Connector lifetime mismatch causes use-after-free on driver unbind.
- [High] Missing anx78xx_poweroff() in driver remove path leaves regulators enabled.
- [High] The INTP interrupt handler may dereference a NULL `connector.dev` pointer if an interrupt occurs before the bridge is attached.
- [High] DRM bridge operations can access freed regmap, regulator, and GPIO resources if called after device removal.
- [Medium] Missing mutex locking in probe and bridge enable/disable paths causes race conditions with threaded interrupt handlers.
--

--- Patch [1]: [PATCH] drm/bridge: anx78xx: fix potential use-after-free on device removal ---
commit 5c75354b8bb8f58551e701235d2a45c7111239b5
Author: Fan Wu <fanwu01@zju.edu.cn>

drm/bridge: anx78xx: fix potential use-after-free on device removal

The commit fixes a potential use-after-free on device removal by freeing
devm-managed interrupts before unregistering dummy i2c clients. It also
adds a missing devm_free_irq() call in the probe error path.

Cc: stable@vger.kernel.org

> diff --git a/drivers/gpu/drm/bridge/analogix/analogix-anx78xx.c b/drivers/gpu/drm/bridge/analogix/analogix-anx78xx.c
> index 32186efa6f501..735b0404ceba8 100644
> --- a/drivers/gpu/drm/bridge/analogix/analogix-anx78xx.c
> +++ b/drivers/gpu/drm/bridge/analogix/analogix-anx78xx.c
> @@ -1309,6 +1309,7 @@ static int anx78xx_i2c_probe(struct i2c_client *client)
>  					"anx78xx-intp", anx78xx);
>  	if (err) {
>  		DRM_ERROR("Failed to request INTP threaded IRQ: %d\n", err);
> +		devm_free_irq(&client->dev, pdata->hpd_irq, anx78xx);

[Severity: High]
This isn't a bug introduced by this patch, but could the INTP interrupt
handler dereference a NULL connector.dev pointer if an interrupt occurs
before the bridge is attached?

Since the interrupt is requested here and enabled immediately, if it fires
before the connector is fully initialized, could the handler crash when
calling drm_helper_hpd_irq_event(anx78xx->connector.dev)?

[Severity: Medium]
This is a pre-existing issue, but does anx78xx_poweroff() and other probe
and bridge enable/disable paths lack mutex locking? Could this cause race
conditions with the threaded interrupt handlers, leading to corrupted
hardware state or imbalanced regulators?

>  		goto err_poweroff;
>  	}
>  
> @@ -1332,6 +1333,14 @@ static void anx78xx_i2c_remove(struct i2c_client *client)
>  {
>  	struct anx78xx *anx78xx = i2c_get_clientdata(client);
>  
> +	/*
> +	 * The threaded IRQ handlers access the dummy I2C clients through
> +	 * regmap and may free the cached EDID, so stop them before those
> +	 * resources are freed below.
> +	 */
> +	devm_free_irq(&client->dev, anx78xx->pdata.hpd_irq, anx78xx);
> +	devm_free_irq(&client->dev, anx78xx->pdata.intp_irq, anx78xx);
> +
>  	drm_bridge_remove(&anx78xx->bridge);

[Severity: Critical]
This is a pre-existing issue, but does removing the bridge here cause a
connector lifetime mismatch and lead to a use-after-free on driver
unbind? If the unbound connector is accessed by the DRM subsystem after
this point, will it trigger a use-after-free?

[Severity: High]
This is a pre-existing issue, but can DRM bridge operations access freed
regmap, regulator, and GPIO resources if called after device removal?
When anx78xx_poweroff() writes to SP_POWERDOWN_CTRL_REG, could it be
accessing freed memory if the bridge outlives the device removal?

>  
>  	unregister_i2c_dummy_clients(anx78xx);

[Severity: High]
This is a pre-existing issue, but is there a missing call to
anx78xx_poweroff() in this driver remove path? Does unbinding the driver
without powering off the chip leave the dvdd10, pd, and reset GPIOs
permanently active instead of turning them off?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260820044746.3087-1-fanwu01@zju.edu.cn?part=1