All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/bridge: ti-sn65dsi83: Cancel reset_work on remove to avoid use-after-free
@ 2026-08-05  9:08 Fan Wu
  2026-08-05  9:23 ` sashiko-bot
  2026-08-05 11:55 ` [PATCH v2] " Fan Wu
  0 siblings, 2 replies; 6+ messages in thread
From: Fan Wu @ 2026-08-05  9:08 UTC (permalink / raw)
  To: dri-devel
  Cc: Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
	Jonas Karlman, Jernej Skrabec, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, David Airlie, Simona Vetter, Marek Vasut,
	linux-kernel, stable, Fan Wu

The error recovery code queues ctx->reset_work from the threaded IRQ
handler and the polling monitor_work, but nothing ever cancels it.
sn65dsi83_remove() only unplugs the bridge, and the
sn65dsi83_release_resources devm action disables the IRQ at the chip and
stops monitor_work but does not cancel reset_work. The IRQ is
devm-managed, so a reset_work already queued survives detach and the devm
IRQ free and runs after ctx is freed, recovering it via container_of() and
leading to a use-after-free. drm_bridge_enter()/SRCU guards a running work
against bridge removal but cannot stop a queued work running after ctx is
gone.

Stop the producers before unplug, then drain: disable_irq so the threaded
handler can no longer queue reset_work, cancel monitor_work, then
cancel_work_sync(&ctx->reset_work). disable_irq keeps the IRQ registered
so a racing reset_work's trailing enable_irq() stays valid, while its
disable depth stops the handler from re-arming before ctx is freed.

This issue was found by an in-house static analysis tool.
Compile-tested only; runtime testing is appreciated.

Fixes: ad5c6ecef27e ("drm: bridge: ti-sn65dsi83: Add error recovery mechanism")
Cc: stable@vger.kernel.org
Assisted-by: Codex:gpt-5.6
Signed-off-by: Fan Wu <fanwu01@zju.edu.cn>
---
 drivers/gpu/drm/bridge/ti-sn65dsi83.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi83.c b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
index 42b451432bbb..d334b2898e25 100644
--- a/drivers/gpu/drm/bridge/ti-sn65dsi83.c
+++ b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
@@ -1069,6 +1069,12 @@ static void sn65dsi83_remove(struct i2c_client *client)
 {
 	struct sn65dsi83 *ctx = i2c_get_clientdata(client);
 
+	/* Stop the reset_work producers, then drain, before unplug. */
+	if (ctx->irq)
+		disable_irq(ctx->irq);
+	cancel_delayed_work_sync(&ctx->monitor_work);
+	cancel_work_sync(&ctx->reset_work);
+
 	drm_bridge_unplug(&ctx->bridge);
 }
 
-- 
2.39.5


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] drm/bridge: ti-sn65dsi83: Cancel reset_work on remove to avoid use-after-free
  2026-08-05  9:08 [PATCH] drm/bridge: ti-sn65dsi83: Cancel reset_work on remove to avoid use-after-free Fan Wu
@ 2026-08-05  9:23 ` sashiko-bot
  2026-08-05 11:55 ` [PATCH v2] " Fan Wu
  1 sibling, 0 replies; 6+ messages in thread
From: sashiko-bot @ 2026-08-05  9:23 UTC (permalink / raw)
  To: Fan Wu; +Cc: dri-devel

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

Pre-existing issues:
- [High] Concurrent DRM commit can re-queue monitor_work after it has been cancelled, causing a use-after-free in polling mode.
- [High] SRCU read lock leak on error path in `sn65dsi83_reset_work` and swallowed errors in `sn65dsi83_reset_pipe`.
- [High] Missing cancel_work_sync(&ctx->reset_work) in the probe error path leads to a use-after-free.
--

--- Patch [1]: [PATCH] drm/bridge: ti-sn65dsi83: Cancel reset_work on remove to avoid use-after-free ---
commit 8ee8b1cf3acf86f0f88ed002c2cab55bb6d809f3
Author: Fan Wu <fanwu01@zju.edu.cn>
    
drm/bridge: ti-sn65dsi83: Cancel reset_work on remove to avoid use-after-free
    
The error recovery code queues ctx->reset_work from the threaded IRQ
handler and the polling monitor_work, but nothing ever cancels it.

> diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi83.c b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
> --- a/drivers/gpu/drm/bridge/ti-sn65dsi83.c
> +++ b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
> @@ -1068,6 +1068,12 @@ static void sn65dsi83_remove(struct i2c_client *client)
>  {
>  	struct sn65dsi83 *ctx = i2c_get_clientdata(client);
>  
> +	/* Stop the reset_work producers, then drain, before unplug. */
> +	if (ctx->irq)
> +		disable_irq(ctx->irq);
> +	cancel_delayed_work_sync(&ctx->monitor_work);
> +	cancel_work_sync(&ctx->reset_work);
> +
>  	drm_bridge_unplug(&ctx->bridge);

[Severity: High]
This isn't a bug introduced by this patch, but could this ordering cause a 
use-after-free in polling mode?

Since drm_bridge_unplug() hasn't been called yet, bridge->unplugged is still 
false. If ctx->irq == 0 (polling mode), no IRQ is disabled. If a concurrent 
DRM atomic commit calls sn65dsi83_atomic_enable() in this window, it will 
successfully enter the bridge and call sn65dsi83_monitor_start(), which 
re-arms monitor_work.

When sn65dsi83_remove() finishes, devres frees ctx, and the newly queued work 
executes on freed memory. Should drm_bridge_unplug() be called before 
cancelling the works to act as a barrier?

While looking at the workqueue handling in this driver, I noticed a couple of 
other pre-existing issues. In sn65dsi83_reset_work():

drivers/gpu/drm/bridge/ti-sn65dsi83.c:sn65dsi83_reset_work() {
        ...
        ret = sn65dsi83_reset_pipe(ctx);
        if (ret) {
                dev_err(ctx->dev, "reset pipe failed %pe\n", ERR_PTR(ret));
                return;
        }
        ...
}

[Severity: High]
This is a pre-existing issue, but does this early return leak the SRCU read 
lock? sn65dsi83_reset_work() calls drm_bridge_enter(), but this error path 
returns without calling drm_bridge_exit(idx).

Additionally, in sn65dsi83_reset_pipe():

drivers/gpu/drm/bridge/ti-sn65dsi83.c:sn65dsi83_reset_pipe() {
        ...
        drm_modeset_drop_locks(&ctx);
        drm_modeset_acquire_fini(&ctx);
        
        return 0;
}

The error code from drm_bridge_helper_reset_crtc() is swallowed and 
unconditionally returns 0, which masks the lock leak above, but instead causes 
sn65dsi83_reset_work() to proceed and re-enable the IRQ without the hardware 
being recovered. Could this lead to an immediate IRQ storm and an infinite 
workqueue loop?

Finally, regarding the probe error path in sn65dsi83_probe():

drivers/gpu/drm/bridge/ti-sn65dsi83.c:sn65dsi83_probe() {
        ...
err_remove_bridge:
        drm_bridge_remove(&ctx->bridge);
        return ret;
}

[Severity: High]
This is another pre-existing issue, but is it possible for reset_work to 
execute after ctx is freed here?

The IRQ is requested via devm_request_threaded_irq() before 
sn65dsi83_host_attach() is called. If a spurious interrupt fires while the 
chip is held in reset, an I2C read in the interrupt handler will fail, causing 
it to queue ctx->reset_work. 

If sn65dsi83_host_attach() then returns -EPROBE_DEFER, the code jumps to 
err_remove_bridge: and returns. Devres will free the IRQ and ctx. Because 
drm_bridge_remove() does not set bridge->unplugged = true, and there is no 
cancel_work_sync(&ctx->reset_work) here, the queued work could execute later 
and dereference the freed ctx memory.

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v2] drm/bridge: ti-sn65dsi83: Cancel reset_work on remove to avoid use-after-free
  2026-08-05  9:08 [PATCH] drm/bridge: ti-sn65dsi83: Cancel reset_work on remove to avoid use-after-free Fan Wu
  2026-08-05  9:23 ` sashiko-bot
@ 2026-08-05 11:55 ` Fan Wu
  2026-08-05 12:15   ` sashiko-bot
  2026-08-05 14:18   ` [PATCH v3] " Fan Wu
  1 sibling, 2 replies; 6+ messages in thread
From: Fan Wu @ 2026-08-05 11:55 UTC (permalink / raw)
  To: dri-devel
  Cc: Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
	Jonas Karlman, Jernej Skrabec, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, David Airlie, Simona Vetter, Marek Vasut,
	linux-kernel, stable, Fan Wu

The error recovery code queues reset_work from the threaded IRQ handler
and polling monitor_work. Neither the remove path nor the probe failure
path after the IRQ is registered drains that work before devres releases
the bridge allocation.

Use drm_bridge_unplug() before stopping the work. It prevents a concurrent
atomic commit from entering the bridge and re-arming monitor_work through
monitor_start(), and waits for in-flight bridge critical sections to
finish. Then disable the IRQ and drain monitor_work and reset_work.

Use the same shutdown sequence when attaching the DSI host fails. This
prevents error recovery work queued by an early IRQ from accessing the
devm-managed bridge after the failed probe returns.

This issue was found by an in-house static analysis tool.
Compile-tested only; runtime testing is appreciated.

Fixes: ad5c6ecef27e ("drm: bridge: ti-sn65dsi83: Add error recovery mechanism")
Cc: stable@vger.kernel.org
Assisted-by: Codex:gpt-5.6
Signed-off-by: Fan Wu <fanwu01@zju.edu.cn>
---
Changes since v1:
  - Move drm_bridge_unplug() ahead of the work cancellation so a concurrent
    atomic commit cannot re-arm monitor_work via monitor_start() after the
    cancel.
  - Drain from the probe-error path (err_remove_bridge) too, closing a UAF
    where a spurious IRQ during probe queues reset_work and a later
    -EPROBE_DEFER frees ctx.
  - Factor the sequence into sn65dsi83_stop_error_recovery() shared by
    both paths.

 drivers/gpu/drm/bridge/ti-sn65dsi83.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi83.c b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
index 42b451432bbb..5f1a96856eac 100644
--- a/drivers/gpu/drm/bridge/ti-sn65dsi83.c
+++ b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
@@ -997,6 +997,18 @@ static irqreturn_t sn65dsi83_irq(int irq, void *data)
 	return IRQ_HANDLED;
 }
 
+static void sn65dsi83_stop_error_recovery(struct sn65dsi83 *ctx)
+{
+	/* Block new bridge users and wait for existing critical sections. */
+	drm_bridge_unplug(&ctx->bridge);
+
+	if (ctx->irq)
+		disable_irq(ctx->irq);
+
+	cancel_delayed_work_sync(&ctx->monitor_work);
+	cancel_work_sync(&ctx->reset_work);
+}
+
 static int sn65dsi83_probe(struct i2c_client *client)
 {
 	const struct i2c_device_id *id = i2c_client_get_device_id(client);
@@ -1061,7 +1073,7 @@ static int sn65dsi83_probe(struct i2c_client *client)
 	return 0;
 
 err_remove_bridge:
-	drm_bridge_remove(&ctx->bridge);
+	sn65dsi83_stop_error_recovery(ctx);
 	return ret;
 }
 
@@ -1069,6 +1081,6 @@ static void sn65dsi83_remove(struct i2c_client *client)
 {
 	struct sn65dsi83 *ctx = i2c_get_clientdata(client);
 
-	drm_bridge_unplug(&ctx->bridge);
+	sn65dsi83_stop_error_recovery(ctx);
 }
 
-- 
2.51.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] drm/bridge: ti-sn65dsi83: Cancel reset_work on remove to avoid use-after-free
  2026-08-05 11:55 ` [PATCH v2] " Fan Wu
@ 2026-08-05 12:15   ` sashiko-bot
  2026-08-05 14:18   ` [PATCH v3] " Fan Wu
  1 sibling, 0 replies; 6+ messages in thread
From: sashiko-bot @ 2026-08-05 12:15 UTC (permalink / raw)
  To: Fan Wu; +Cc: dri-devel

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

New issues:
- [High] Interrupt storm between drm_bridge_unplug() and disable_irq()

Pre-existing issues:
- [High] Spurious IRQ during probe causes NULL pointer dereference in sn65dsi83_reset_work()
- [High] Leaked SRCU read lock in sn65dsi83_reset_work() error path
- [High] Missing IRQ synchronization in sn65dsi83_release_resources() causes race condition
--

--- Patch [1]: [PATCH v2] drm/bridge: ti-sn65dsi83: Cancel reset_work on remove to avoid use-after-free ---
commit 197f183cb3d12c2d5d1810b8f11c167e77438773
Author: Fan Wu <fanwu01@zju.edu.cn>

drm/bridge: ti-sn65dsi83: Cancel reset_work on remove to avoid use-after-free

The error recovery code queues reset_work from the threaded IRQ handler
and polling monitor_work. Neither the remove path nor the probe failure
path after the IRQ is registered drains that work before devres releases
the bridge allocation.

> diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi83.c b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
> index f9fdbf48c6b32..bf263f407cb2c 100644
> --- a/drivers/gpu/drm/bridge/ti-sn65dsi83.c
> +++ b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
> @@ -996,6 +996,18 @@ static irqreturn_t sn65dsi83_irq(int irq, void *data)
>  	return IRQ_HANDLED;
>  }
>  
> +static void sn65dsi83_stop_error_recovery(struct sn65dsi83 *ctx)
> +{
> +	/* Block new bridge users and wait for existing critical sections. */
> +	drm_bridge_unplug(&ctx->bridge);
> +
> +	if (ctx->irq)
> +		disable_irq(ctx->irq);

[Severity: High]
Could this ordering cause an interrupt storm leading to a CPU lockup?

When drm_bridge_unplug() sets bridge->unplugged = true, if an IRQ fires before
disable_irq() is called, sn65dsi83_handle_errors() will see drm_bridge_enter()
fail and return early:

drivers/gpu/drm/bridge/ti-sn65dsi83.c:sn65dsi83_handle_errors() {
    ...
	if (!drm_bridge_enter(&ctx->bridge, &idx))
		return;
    ...
}

Since this threaded IRQ handler returns IRQ_HANDLED without clearing the
hardware interrupt or disabling the IRQ, will the IRQ core unmask the
still-asserted interrupt line, causing it to immediately fire again and
starve the teardown code?

[Severity: High]
This is a pre-existing issue, but will the addition of drm_bridge_unplug()
here cause a system deadlock during driver unbind or probe failure?

In the error path of sn65dsi83_reset_work(), if resetting the pipe fails,
it returns early:

drivers/gpu/drm/bridge/ti-sn65dsi83.c:sn65dsi83_reset_work() {
    ...
	ret = sn65dsi83_reset_pipe(ctx);
	if (ret) {
		dev_err(ctx->dev, "reset pipe failed %pe\n", ERR_PTR(ret));
		return;
	}
    ...
}

This early return skips drm_bridge_exit(idx), permanently leaking the SRCU
read lock. If that happens, won't this new drm_bridge_unplug() call block
forever in synchronize_srcu()?

> +
> +	cancel_delayed_work_sync(&ctx->monitor_work);
> +	cancel_work_sync(&ctx->reset_work);
> +}
> +
>  static int sn65dsi83_probe(struct i2c_client *client)
>  {
[ ... ]
> @@ -1060,7 +1072,7 @@ static int sn65dsi83_probe(struct i2c_client *client)
>  	return 0;
>  
>  err_remove_bridge:
> -	drm_bridge_remove(&ctx->bridge);
> +	sn65dsi83_stop_error_recovery(ctx);

[Severity: High]
This is a pre-existing issue, but can a spurious interrupt during probe
cause a NULL pointer dereference?

If a spurious interrupt fires after devm_request_threaded_irq() is called
in sn65dsi83_probe() but before the KMS driver calls drm_bridge_attach(),
the threaded handler will schedule reset_work.

The worker then executes sn65dsi83_reset_pipe() which calls:

drivers/gpu/drm/drm_bridge_helper.c:drm_bridge_helper_reset_crtc() {
    ...
	struct drm_device *dev = encoder->dev;
    ...
}

Since the bridge is not attached yet, bridge->encoder is NULL. Does this
unconditionally crash if an early interrupt occurs?

>  	return ret;
>  }
>  
> @@ -1068,7 +1080,7 @@ static void sn65dsi83_remove(struct i2c_client *client)
>  {
>  	struct sn65dsi83 *ctx = i2c_get_clientdata(client);
>  
> -	drm_bridge_unplug(&ctx->bridge);
> +	sn65dsi83_stop_error_recovery(ctx);
>  }

[Severity: High]
This is a pre-existing issue, but does sn65dsi83_release_resources() contain
a race condition with the IRQ handler?

During display teardown, sn65dsi83_release_resources() disables the hardware
interrupt and powers off the chip without synchronizing against the executing
IRQ handler:

drivers/gpu/drm/bridge/ti-sn65dsi83.c:sn65dsi83_release_resources() {
    ...
	if (ctx->irq) {
		/* Disable irq */
		regmap_write(ctx->regmap, REG_IRQ_EN, 0x0);
		regmap_write(ctx->regmap, REG_IRQ_GLOBAL, 0x0);
	}
    ...
}

If the threaded IRQ handler sn65dsi83_handle_errors() runs concurrently,
won't its I2C read fail due to the chip being powered down? It seems it
would misinterpret this as a hardware error and improperly schedule
reset_work while the display pipeline is being torn down.

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v3] drm/bridge: ti-sn65dsi83: Cancel reset_work on remove to avoid use-after-free
  2026-08-05 11:55 ` [PATCH v2] " Fan Wu
  2026-08-05 12:15   ` sashiko-bot
@ 2026-08-05 14:18   ` Fan Wu
  2026-08-05 15:05     ` sashiko-bot
  1 sibling, 1 reply; 6+ messages in thread
From: Fan Wu @ 2026-08-05 14:18 UTC (permalink / raw)
  To: dri-devel
  Cc: Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
	Jonas Karlman, Jernej Skrabec, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, David Airlie, Simona Vetter, Marek Vasut,
	linux-kernel, stable, Fan Wu

The error recovery code queues reset_work from the threaded IRQ handler
and polling monitor_work. Neither device removal nor a probe failure after
requesting the IRQ drains that work before devres releases the bridge
allocation.

Keep the IRQ disabled after it is requested and enable it only after the
bridge has attached successfully. This prevents an IRQ during probe from
queueing reset_work before bridge->encoder has been initialized.

On remove and the post-IRQ probe failure path, synchronously disable the
IRQ before declaring the bridge unplugged. This prevents an asserted IRQ
from repeatedly invoking a handler which can no longer acknowledge the
device. drm_bridge_unplug() then blocks new DRM entry points and waits for
existing bridge critical sections. Finally drain monitor_work and
reset_work before devres can free the bridge allocation.

This issue was found by an in-house static analysis tool.
Compile-tested only; runtime testing is appreciated.

Fixes: ad5c6ecef27e ("drm: bridge: ti-sn65dsi83: Add error recovery mechanism")
Cc: stable@vger.kernel.org
Assisted-by: Codex:gpt-5.6
Signed-off-by: Fan Wu <fanwu01@zju.edu.cn>
---
Changes since v2:
  - Request the IRQ with IRQF_NO_AUTOEN and enable it only after the bridge
    attaches, so it cannot fire during probe and queue reset_work before
    bridge->encoder is initialized.
  - Disable the IRQ before drm_bridge_unplug() in the teardown helper, so
    an asserted level IRQ is acknowledged by the handler while it can still
    enter the bridge instead of re-firing once the unplug barrier rejects
    it.

 drivers/gpu/drm/bridge/ti-sn65dsi83.c | 28 +++++++++++++++++++++++-----
 1 file changed, 23 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi83.c b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
index 42b451432bbb..d7648b5d5636 100644
--- a/drivers/gpu/drm/bridge/ti-sn65dsi83.c
+++ b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
@@ -296,9 +296,14 @@ static int sn65dsi83_attach(struct drm_bridge *bridge,
 			    enum drm_bridge_attach_flags flags)
 {
 	struct sn65dsi83 *ctx = bridge_to_sn65dsi83(bridge);
+	int ret;
 
-	return drm_bridge_attach(encoder, ctx->panel_bridge,
-				 &ctx->bridge, flags);
+	ret = drm_bridge_attach(encoder, ctx->panel_bridge, &ctx->bridge,
+				flags);
+	if (!ret && ctx->irq)
+		enable_irq(ctx->irq);
+
+	return ret;
 }
 
 static void sn65dsi83_detach(struct drm_bridge *bridge)
@@ -997,6 +1004,19 @@ static irqreturn_t sn65dsi83_irq(int irq, void *data)
 	return IRQ_HANDLED;
 }
 
+static void sn65dsi83_stop_error_recovery(struct sn65dsi83 *ctx)
+{
+	/* Stop the IRQ before the unplug barrier rejects its handler. */
+	if (ctx->irq)
+		disable_irq(ctx->irq);
+
+	/* Block new bridge users and wait for existing critical sections. */
+	drm_bridge_unplug(&ctx->bridge);
+
+	cancel_delayed_work_sync(&ctx->monitor_work);
+	cancel_work_sync(&ctx->reset_work);
+}
+
 static int sn65dsi83_probe(struct i2c_client *client)
 {
 	const struct i2c_device_id *id = i2c_client_get_device_id(client);
@@ -1039,7 +1056,8 @@ static int sn65dsi83_probe(struct i2c_client *client)
 	if (client->irq) {
 		ctx->irq = client->irq;
 		ret = devm_request_threaded_irq(ctx->dev, ctx->irq, NULL, sn65dsi83_irq,
-						IRQF_ONESHOT, dev_name(ctx->dev), ctx);
+						IRQF_ONESHOT | IRQF_NO_AUTOEN,
+						dev_name(ctx->dev), ctx);
 		if (ret)
 			return dev_err_probe(dev, ret, "failed to request irq\n");
 	}
@@ -1061,7 +1078,7 @@ static int sn65dsi83_probe(struct i2c_client *client)
 	return 0;
 
 err_remove_bridge:
-	drm_bridge_remove(&ctx->bridge);
+	sn65dsi83_stop_error_recovery(ctx);
 	return ret;
 }
 
@@ -1069,6 +1086,6 @@ static void sn65dsi83_remove(struct i2c_client *client)
 {
 	struct sn65dsi83 *ctx = i2c_get_clientdata(client);
 
-	drm_bridge_unplug(&ctx->bridge);
+	sn65dsi83_stop_error_recovery(ctx);
 }
 
-- 
2.51.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH v3] drm/bridge: ti-sn65dsi83: Cancel reset_work on remove to avoid use-after-free
  2026-08-05 14:18   ` [PATCH v3] " Fan Wu
@ 2026-08-05 15:05     ` sashiko-bot
  0 siblings, 0 replies; 6+ messages in thread
From: sashiko-bot @ 2026-08-05 15:05 UTC (permalink / raw)
  To: Fan Wu; +Cc: dri-devel

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

New issues:
- [High] Missing `disable_irq()` in `sn65dsi83_detach()` causes unbalanced IRQ state and Use-After-Free of DRM pipeline objects.

Pre-existing issues:
- [Medium] `sn65dsi83_reset_pipe()` silently discards errors from `drm_bridge_helper_reset_crtc()`.
--

--- Patch [1]: [PATCH v3] drm/bridge: ti-sn65dsi83: Cancel reset_work on remove to avoid use-after-free ---
commit 71f81476ddab3b26071a54fac540bd7518b097ca
Author: Fan Wu <fanwu01@zju.edu.cn>

drm/bridge: ti-sn65dsi83: Cancel reset_work on remove to avoid use-after-free

This commit addresses a use-after-free vulnerability by properly canceling
reset_work and monitor_work during device removal and probe failure paths.
It also ensures the IRQ is disabled before unplugging the bridge to
prevent handlers from executing after teardown.

> diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi83.c b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
> index f9fdbf48c6b32..8f17fd319cf90 100644
> --- a/drivers/gpu/drm/bridge/ti-sn65dsi83.c
> +++ b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
> @@ -298,9 +298,14 @@ static int sn65dsi83_attach(struct drm_bridge *bridge,
>  			    enum drm_bridge_attach_flags flags)
>  {
>  	struct sn65dsi83 *ctx = bridge_to_sn65dsi83(bridge);
> +	int ret;
> +
> +	ret = drm_bridge_attach(encoder, ctx->panel_bridge, &ctx->bridge,
> +				flags);
> +	if (!ret && ctx->irq)
> +		enable_irq(ctx->irq);
>  
> -	return drm_bridge_attach(encoder, ctx->panel_bridge,
> -				 &ctx->bridge, flags);
> +	return ret;
>  }
>  
>  static void sn65dsi83_detach(struct drm_bridge *bridge)
>  {
>  	struct sn65dsi83 *ctx = bridge_to_sn65dsi83(bridge);
>  
>  	if (!ctx->dsi)
>  		return;
>  
>  	ctx->dsi = NULL;
>  }

[Severity: High]
Does this require a symmetric disable_irq() in sn65dsi83_detach()?

If the master DRM driver unbinds, drm_bridge_detach() is called, removing
the bridge from the encoder chain. If we don't disable the IRQ here, the
hardware interrupt remains active.

If the master DRM driver binds again, sn65dsi83_attach() will call
enable_irq() on an already-enabled IRQ, which could trigger an unbalanced
enable warning.

More critically, if a hardware interrupt triggers while the bridge is
detached, sn65dsi83_irq() schedules reset_work. The work executes
drm_bridge_helper_reset_crtc(), which could dereference a freed encoder:

drm_bridge_helper_reset_crtc()
{
	struct drm_encoder *encoder = bridge->encoder;
	struct drm_device *dev = encoder->dev;
...

[ ... ]

[Severity: Medium]
This is a pre-existing issue, but sn65dsi83_reset_pipe() appears to silently
discard errors from drm_bridge_helper_reset_crtc():

sn65dsi83_reset_pipe() {
	err = drm_bridge_helper_reset_crtc(&sn65dsi83->bridge, &ctx);
	if (err == -EDEADLK) {
		drm_modeset_backoff(&ctx);
		goto retry;
	}

	drm_modeset_drop_locks(&ctx);
	drm_modeset_acquire_fini(&ctx);

	return 0;
}

Since sn65dsi83_reset_work() assumes the pipe reset was successful and
calls enable_irq(), could this immediately re-trigger the hardware interrupt
if the reset actually failed, potentially causing an IRQ storm?

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2026-08-05 15:05 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-05  9:08 [PATCH] drm/bridge: ti-sn65dsi83: Cancel reset_work on remove to avoid use-after-free Fan Wu
2026-08-05  9:23 ` sashiko-bot
2026-08-05 11:55 ` [PATCH v2] " Fan Wu
2026-08-05 12:15   ` sashiko-bot
2026-08-05 14:18   ` [PATCH v3] " Fan Wu
2026-08-05 15:05     ` sashiko-bot

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.