[PATCH] accel/habanalabs: disable pending-reset work in hl_device_fini()

[PATCH] accel/habanalabs: disable pending-reset work in hl_device_fini()

[Chengyu Liu]

hl_device_fini() sets device_fini_pending and flush_delayed_work()s the
pending-reset work, but flush only waits for a running callback - it
does not prevent the callback from re-arming.
device_hard_reset_pending() re-arms itself via queue_delayed_work() when
hl_device_reset() fails with -EBUSY and the (unlocked)
device_fini_pending check still reads 0, so a callback that passed that
check before fini set it re-arms during the flush, and the fresh timer
survives: destroy_workqueue(hdev->reset_wq) cannot see a delayed work
pending on the timer wheel, so the work fires later, queues onto a
destroyed workqueue and runs with a freed hdev.  Use-after-free.

On a busy device (live compute contexts make hl_device_reset() keep
failing with -EBUSY) this work sits in a per-second retry loop and the
callback body is a real reset attempt, so hitting the window on device
removal/FLR is observable in practice, and retries are free.

Use disable_delayed_work_sync(), which waits for a running callback to
finish and leaves the work item disabled, so the callback's
queue_delayed_work() re-arm has no effect.

Found by static analysis.  The race was reproduced on a KASAN build
with a small module exercising this exact flush/re-arm/free pattern
(no Gaudi hardware available); PoC and splat are below.

Cc: stable@vger.kernel.org
Signed-off-by: Chengyu Liu <lui2012qwq@gmail.com>
---

Reproducer notes (PoC source below; a module parameter selects the
shape): mode=0 (flush_delayed_work + lockless guard, this bug's shape) produces

    [    2.22] BUG: KASAN: slab-use-after-free in __run_timer_base.part.0+0x807/0x900

after the teardown frees the container; mode=1
(cancel_delayed_work_sync, i.e. this fix) stays clean.

---------------------------- reproducer: pm_flush_poc.c ----------------------------
// SPDX-License-Identifier: GPL-2.0
/*
 * pm_teardown_poc - mechanism PoC for the drv-teardown-sync C/D variants
 * (F-026 ipoib / F-028 epf-ntb / F-029 habanalabs).
 *
 * Modes (module parameter):
 *   0  F-029 vuln shape:  flush_delayed_work + lockless guard
 *   1  control:           cancel_delayed_work_sync (>=6.9 swallows re-arm)
 *   2  F-028 vuln shape:  cancel_delayed_work (non-sync) + unconditional re-arm
 *   3  F-026 vuln shape:  cancel_delayed_work (non-sync) + guard check/queue
 *                         race window (test passes, teardown slips in before
 *                         the queue_delayed_work)
 *
 * Every vuln mode must produce KASAN slab-use-after-free when the orphaned
 * work fires after kfree; the control mode must stay clean.
 */
#include <linux/module.h>
#include <linux/workqueue.h>
#include <linux/slab.h>
#include <linux/delay.h>

static int mode;
module_param(mode, int, 0444);
MODULE_PARM_DESC(mode, "0=flush(F-029) 1=cancel_sync(ctrl) 2=cancel_nosync(F-028) 3=cancel_nosync+guard-race(F-026)");

struct pm_td_ctx {
	bool			fini_pending;
	struct delayed_work	work;
};

static struct pm_td_ctx *ctx;

static void pm_td_cb(struct work_struct *w)
{
	struct pm_td_ctx *c = container_of(to_delayed_work(w),
					   struct pm_td_ctx, work);

	pr_info("PM-POC: cb running, fini_pending=%d\n", c->fini_pending);

	switch (mode) {
	case 0:		/* F-029: lockless guard then long body then re-arm */
		if (c->fini_pending)
			return;
		msleep(300);
		schedule_delayed_work(&c->work, HZ / 10);
		pr_info("PM-POC: re-armed (flush shape)\n");
		break;
	case 2:		/* F-028: unconditional re-arm, cancel must catch running cb */
		msleep(300);	/* long body so cancel lands mid-run */
		schedule_delayed_work(&c->work, HZ / 10);
		pr_info("PM-POC: re-armed (no guard)\n");
		break;
	case 3:		/* F-026: guard check passes, THEN teardown slips in */
		if (c->fini_pending)
			return;
		/* widen the check->queue race window like ipoib's 724->725 */
		msleep(300);
		schedule_delayed_work(&c->work, HZ / 10);
		pr_info("PM-POC: re-armed past guard (test-queue race)\n");
		break;
	default:
		break;
	}
}

static int __init pm_poc_init(void)
{
	ctx = kzalloc(sizeof(*ctx), GFP_KERNEL);
	if (!ctx)
		return -ENOMEM;
	ctx->fini_pending = false;
	INIT_DELAYED_WORK(&ctx->work, pm_td_cb);

	pr_info("PM-POC: arming, mode=%d\n", mode);
	schedule_delayed_work(&ctx->work, 0);

	msleep(50);	/* callback is mid-run now */

	/* teardown sequence for the selected variant */
	ctx->fini_pending = true;
	switch (mode) {
	case 1:
		cancel_delayed_work_sync(&ctx->work);
		break;
	case 0:
		flush_delayed_work(&ctx->work);
		break;
	default:
		cancel_delayed_work(&ctx->work);	/* non-sync: C variants */
		break;
	}

	pr_info("PM-POC: teardown done, freeing ctx\n");
	kfree(ctx);

	msleep(600);
	pr_info("PM-POC: SURVIVED (no KASAN splat)\n");
	return 0;
}

static void __exit pm_poc_exit(void) { }

module_init(pm_poc_init);
module_exit(pm_poc_exit);
MODULE_LICENSE("GPL");
MODULE_DESCRIPTION("drv-teardown-sync C/D variant mechanism PoC (F-026/028/029)");

 drivers/accel/habanalabs/common/device.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/accel/habanalabs/common/device.c b/drivers/accel/habanalabs/common/device.c
index 09b27bac3a..4635dc8572 100644
--- a/drivers/accel/habanalabs/common/device.c
+++ b/drivers/accel/habanalabs/common/device.c
@@ -2487,7 +2487,7 @@ void hl_device_fini(struct hl_device *hdev)
 	dev_info(hdev->dev, "Removing device %s\n", dev_name(&(hdev)->pdev->dev));
 
 	hdev->device_fini_pending = 1;
-	flush_delayed_work(&hdev->device_reset_work.reset_work);
+	disable_delayed_work_sync(&hdev->device_reset_work.reset_work);
 
 	if (hdev->pldm)
 		reset_sec = HL_PLDM_HARD_RESET_MAX_TIMEOUT;
-- 
2.25.1

Re: [PATCH] accel/habanalabs: disable pending-reset work in hl_device_fini()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] accel/habanalabs: disabling reset_work in hl_device_fini() strands in_reset
--

commit eb768a82770d27a90572b30cd01bc34db4fd01ba
Author: Chengyu Liu <lui2012qwq@gmail.com>

accel/habanalabs: disable pending-reset work in hl_device_fini()

This commit replaces flush_delayed_work() with disable_delayed_work_sync() in
hl_device_fini() to prevent device_hard_reset_pending() from re-arming
hdev->device_reset_work.reset_work during device teardown.

> diff --git a/drivers/accel/habanalabs/common/device.c b/drivers/accel/habanalabs/common/device.c
> index 09b27bac3a31d..4635dc8572f03 100644
> --- a/drivers/accel/habanalabs/common/device.c
> +++ b/drivers/accel/habanalabs/common/device.c
> @@ -2487,7 +2487,7 @@ void hl_device_fini(struct hl_device *hdev)
>  	dev_info(hdev->dev, "Removing device %s\n", dev_name(&(hdev)->pdev->dev));
>  
>  	hdev->device_fini_pending = 1;
> -	flush_delayed_work(&hdev->device_reset_work.reset_work);
> +	disable_delayed_work_sync(&hdev->device_reset_work.reset_work);

[Severity: Critical]
Does cancelling and disabling hdev->device_reset_work.reset_work here leave
hdev->reset_info.in_reset stuck at 1 and cause hl_device_fini() to time out
and skip teardown?

When a hard reset is initiated outside the reset thread, hl_device_reset()
sets hdev->reset_info.in_reset = 1 and hands off clearing it to
hdev->device_reset_work.reset_work:

drivers/accel/habanalabs/common/device.c:hl_device_reset() {
	...
		hdev->reset_info.in_compute_reset = !hard_reset;

		hdev->reset_info.in_reset = 1;

		spin_unlock(&hdev->reset_info.lock);
	...
	if ((hard_reset) && (!from_hard_reset_thread)) {
		hdev->reset_info.hard_reset_pending = true;
		...
		queue_delayed_work(hdev->reset_wq, &hdev->device_reset_work.reset_work, 0);

		return 0;
	}
}

When device_hard_reset_pending() runs hl_device_reset() and
device_kill_open_processes() returns -EBUSY while device_fini_pending is 0,
hl_device_reset() returns -EBUSY directly without jumping to out_err, keeping
hdev->reset_info.in_reset set to 1 across the retry delay:

drivers/accel/habanalabs/common/device.c:hl_device_reset() {
	...
		rc = device_kill_open_processes(hdev, 0, false);

		if (rc == -EBUSY) {
			if (hdev->device_fini_pending) {
				dev_crit(hdev->dev,
					"%s Failed to kill all open processes, stopping hard reset\n",
					dev_name(&(hdev)->pdev->dev));
				goto out_err;
			}

			/* signal reset thread to reschedule */
			return rc;
		}
	...
out_err:
	...
	hdev->reset_info.in_reset = 0;

	spin_unlock(&hdev->reset_info.lock);

	return rc;
}

Previously, setting hdev->device_fini_pending = 1 and calling
flush_delayed_work() forced any pending reset_work to execute immediately
with device_fini_pending == 1, which entered the goto out_err path and
cleared hdev->reset_info.in_reset = 0.

With disable_delayed_work_sync(), if reset_work is waiting on its retry timer
(or queued on reset_wq before starting), or if a running callback already read
hdev->device_fini_pending == 0 in hl_device_reset() before returning -EBUSY,
or if a concurrent hl_device_reset() sets in_reset = 1 and queues reset_work
before hl_device_fini() takes hdev->reset_info.lock, the work item never runs
again to clear hdev->reset_info.in_reset.

Won't hl_device_fini() then spin in while (device_in_reset) for the full
HL_HARD_RESET_MAX_TIMEOUT (or HL_PLDM_HARD_RESET_MAX_TIMEOUT) and return
early without cleaning up the device before hl_pci_remove() calls
destroy_hdev()?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261103163354.1599176-1-lui2012qwq@gmail.com?part=1

Re: [PATCH] accel/habanalabs: disable pending-reset work in hl_device_fini()

[Chengyu Liu]

Thanks for the review - this scenario is real, and the patch as posted
has exactly that problem.

A reset retry waiting on its timer holds reset_info.in_reset, and only
the work itself clears it (via out_err, because device_fini_pending is
set).  Disabling the work without running it strands in_reset, and
hl_device_fini() would then spin until HL_HARD_RESET_MAX_TIMEOUT and
return without tearing the device down.

v2 keeps the flush_delayed_work(), so a pending retry still runs and
clears in_reset, and adds disable_delayed_work_sync() only after
in_reset has been claimed for fini.  At that point no new reset can be
queued (the in_reset gate) and the reset thread does not re-arm
(because device_fini_pending is set), so nothing can be left pending
or running when the teardown proceeds.

One note on the second scenario in the review - a running callback
that already passed the device_fini_pending check and then returns
-EBUSY, after which the wrapper refuses to re-queue: that race exists
identically with the original flush-based code (the flush waits for
the callback, but nothing clears in_reset on that path either).  It is
pre-existing and unchanged by this fix.

Chengyu

On Wed, Nov 4, 2026 at 5:07 PM <sashiko-bot@kernel.org> wrote:
>
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Critical] accel/habanalabs: disabling reset_work in hl_device_fini() strands in_reset
> --
>
> commit eb768a82770d27a90572b30cd01bc34db4fd01ba
> Author: Chengyu Liu <lui2012qwq@gmail.com>
>
> accel/habanalabs: disable pending-reset work in hl_device_fini()
>
> This commit replaces flush_delayed_work() with disable_delayed_work_sync() in
> hl_device_fini() to prevent device_hard_reset_pending() from re-arming
> hdev->device_reset_work.reset_work during device teardown.
>
> > diff --git a/drivers/accel/habanalabs/common/device.c b/drivers/accel/habanalabs/common/device.c
> > index 09b27bac3a31d..4635dc8572f03 100644
> > --- a/drivers/accel/habanalabs/common/device.c
> > +++ b/drivers/accel/habanalabs/common/device.c
> > @@ -2487,7 +2487,7 @@ void hl_device_fini(struct hl_device *hdev)
> >       dev_info(hdev->dev, "Removing device %s\n", dev_name(&(hdev)->pdev->dev));
> >
> >       hdev->device_fini_pending = 1;
> > -     flush_delayed_work(&hdev->device_reset_work.reset_work);
> > +     disable_delayed_work_sync(&hdev->device_reset_work.reset_work);
>
> [Severity: Critical]
> Does cancelling and disabling hdev->device_reset_work.reset_work here leave
> hdev->reset_info.in_reset stuck at 1 and cause hl_device_fini() to time out
> and skip teardown?
>
> When a hard reset is initiated outside the reset thread, hl_device_reset()
> sets hdev->reset_info.in_reset = 1 and hands off clearing it to
> hdev->device_reset_work.reset_work:
>
> drivers/accel/habanalabs/common/device.c:hl_device_reset() {
>         ...
>                 hdev->reset_info.in_compute_reset = !hard_reset;
>
>                 hdev->reset_info.in_reset = 1;
>
>                 spin_unlock(&hdev->reset_info.lock);
>         ...
>         if ((hard_reset) && (!from_hard_reset_thread)) {
>                 hdev->reset_info.hard_reset_pending = true;
>                 ...
>                 queue_delayed_work(hdev->reset_wq, &hdev->device_reset_work.reset_work, 0);
>
>                 return 0;
>         }
> }
>
> When device_hard_reset_pending() runs hl_device_reset() and
> device_kill_open_processes() returns -EBUSY while device_fini_pending is 0,
> hl_device_reset() returns -EBUSY directly without jumping to out_err, keeping
> hdev->reset_info.in_reset set to 1 across the retry delay:
>
> drivers/accel/habanalabs/common/device.c:hl_device_reset() {
>         ...
>                 rc = device_kill_open_processes(hdev, 0, false);
>
>                 if (rc == -EBUSY) {
>                         if (hdev->device_fini_pending) {
>                                 dev_crit(hdev->dev,
>                                         "%s Failed to kill all open processes, stopping hard reset\n",
>                                         dev_name(&(hdev)->pdev->dev));
>                                 goto out_err;
>                         }
>
>                         /* signal reset thread to reschedule */
>                         return rc;
>                 }
>         ...
> out_err:
>         ...
>         hdev->reset_info.in_reset = 0;
>
>         spin_unlock(&hdev->reset_info.lock);
>
>         return rc;
> }
>
> Previously, setting hdev->device_fini_pending = 1 and calling
> flush_delayed_work() forced any pending reset_work to execute immediately
> with device_fini_pending == 1, which entered the goto out_err path and
> cleared hdev->reset_info.in_reset = 0.
>
> With disable_delayed_work_sync(), if reset_work is waiting on its retry timer
> (or queued on reset_wq before starting), or if a running callback already read
> hdev->device_fini_pending == 0 in hl_device_reset() before returning -EBUSY,
> or if a concurrent hl_device_reset() sets in_reset = 1 and queues reset_work
> before hl_device_fini() takes hdev->reset_info.lock, the work item never runs
> again to clear hdev->reset_info.in_reset.
>
> Won't hl_device_fini() then spin in while (device_in_reset) for the full
> HL_HARD_RESET_MAX_TIMEOUT (or HL_PLDM_HARD_RESET_MAX_TIMEOUT) and return
> early without cleaning up the device before hl_pci_remove() calls
> destroy_hdev()?
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20261103163354.1599176-1-lui2012qwq@gmail.com?part=1

[PATCH v2] accel/habanalabs: disable pending-reset work in hl_device_fini()

[Chengyu Liu]

hl_device_fini() uses flush_delayed_work() to drain
device_reset_work.reset_work, but a flush cannot stop the work from
being armed again afterwards, so the reset work can still fire while -
or after - the device is torn down.

Simply switching the flush to disable_delayed_work_sync() is not
enough: a reset retry sitting on its timer holds reset_info.in_reset,
and only the work itself clears it (via out_err once
device_fini_pending is set).  Killing the work without running it
strands in_reset, and hl_device_fini() then waits for the full
HL_HARD_RESET_MAX_TIMEOUT and returns without tearing the device
down.

So keep the flush first: with device_fini_pending set, a pending
retry runs immediately and clears in_reset via out_err.  Then, after
in_reset has been claimed for fini - so no new reset can be queued
and the reset thread does not re-arm - disable the work to guarantee
nothing is pending or running during the teardown.

Found by static analysis.

Signed-off-by: Chengyu Liu <lui2012qwq@gmail.com>
---
v2: keep the flush so a pending reset retry still runs and clears
    in_reset (sashiko review); the disable now happens after in_reset
    is claimed for fini, where no new reset can be queued and the
    reset thread does not re-arm.

 drivers/accel/habanalabs/common/device.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/accel/habanalabs/common/device.c b/drivers/accel/habanalabs/common/device.c
index 09b27bac3a31..c27c13e693d5 100644
--- a/drivers/accel/habanalabs/common/device.c
+++ b/drivers/accel/habanalabs/common/device.c
@@ -2527,6 +2527,13 @@ void hl_device_fini(struct hl_device *hdev)
 		}
 	}
 
+	/*
+	 * in_reset is claimed now, so no new reset can be queued and the
+	 * reset thread does not re-arm with device_fini_pending set; make
+	 * sure no reset work is left pending or running during teardown.
+	 */
+	disable_delayed_work_sync(&hdev->device_reset_work.reset_work);
+
 	cancel_delayed_work_sync(&hdev->device_release_watchdog_work.reset_work);
 
 	/* Disable PCI access from device F/W so it won't send us additional
-- 
2.25.1

Re: [PATCH v2] accel/habanalabs: disable pending-reset work in hl_device_fini()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Low] accel/habanalabs: missing Fixes trailer for hl_device_fini()
  reset work fix
--

commit e560ec133709db78ecf2c5f01a23a035c9ef7ca3
Author: Chengyu Liu <lui2012qwq@gmail.com>

> accel/habanalabs: disable pending-reset work in hl_device_fini()
>
> hl_device_fini() uses flush_delayed_work() to drain
> device_reset_work.reset_work, but a flush cannot stop the work from
> being armed again afterwards, so the reset work can still fire while -
> or after - the device is torn down.
>
> Simply switching the flush to disable_delayed_work_sync() is not
> enough: a reset retry sitting on its timer holds reset_info.in_reset,
> and only the work itself clears it (via out_err once
> device_fini_pending is set).  Killing the work without running it
> strands in_reset, and hl_device_fini() then waits for the full
> HL_HARD_RESET_MAX_TIMEOUT and returns without tearing the device
> down.
>
> So keep the flush first: with device_fini_pending set, a pending
> retry runs immediately and clears in_reset via out_err.  Then, after
> in_reset has been claimed for fini - so no new reset can be queued
> and the reset thread does not re-arm - disable the work to guarantee
> nothing is pending or running during the teardown.
>
> Found by static analysis.
>
> Signed-off-by: Chengyu Liu <lui2012qwq@gmail.com>

[Severity: Low]
This fixes the incomplete draining of device_reset_work.reset_work in
hl_device_fini().  Should this include:

  Fixes: adb51298fdc4 ("habanalabs: improve hard reset procedure")

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261104095159.6487-1-lui2012qwq@gmail.com?part=1

[PATCH v3] accel/habanalabs: disable pending-reset work in hl_device_fini()

[Chengyu Liu]

hl_device_fini() uses flush_delayed_work() to drain
device_reset_work.reset_work, but a flush cannot stop the work from
being armed again afterwards, so the reset work can still fire while
or after the device is torn down.

Simply switching the flush to disable_delayed_work_sync() is not
enough: a reset retry sitting on its timer holds reset_info.in_reset,
and only the work itself clears it (via out_err once
device_fini_pending is set).  Killing the work without running it
strands in_reset, and hl_device_fini() then waits for the full
HL_HARD_RESET_MAX_TIMEOUT and returns without tearing the device
down.

So keep the flush first: with device_fini_pending set, a pending
retry runs immediately and clears in_reset via out_err.  Then, after
in_reset has been claimed for fini - so no new reset can be queued
and the reset thread does not re-arm - disable the work to guarantee
nothing is pending or running during the teardown.

Found by static analysis.

Fixes: adb51298fdc4 ("habanalabs: improve hard reset procedure")
Signed-off-by: Chengyu Liu <lui2012qwq@gmail.com>
---
v3: add the Fixes trailer suggested by sashiko; no code change.
    
v2: keep the flush so a pending reset retry still runs and clears
    in_reset (sashiko review); the disable now happens after in_reset
    is claimed for fini, where no new reset can be queued and the
    reset thread does not re-arm.
    https://lore.kernel.org/r/20261104095159.6487-1-lui2012qwq@gmail.com

v1: https://lore.kernel.org/r/20261103163354.1599176-1-lui2012qwq@gmail.com

 drivers/accel/habanalabs/common/device.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/accel/habanalabs/common/device.c b/drivers/accel/habanalabs/common/device.c
index 09b27bac3a31..c27c13e693d5 100644
--- a/drivers/accel/habanalabs/common/device.c
+++ b/drivers/accel/habanalabs/common/device.c
@@ -2527,6 +2527,13 @@ void hl_device_fini(struct hl_device *hdev)
 		}
 	}
 
+	/*
+	 * in_reset is claimed now, so no new reset can be queued and the
+	 * reset thread does not re-arm with device_fini_pending set; make
+	 * sure no reset work is left pending or running during teardown.
+	 */
+	disable_delayed_work_sync(&hdev->device_reset_work.reset_work);
+
 	cancel_delayed_work_sync(&hdev->device_release_watchdog_work.reset_work);
 
 	/* Disable PCI access from device F/W so it won't send us additional
-- 
2.25.1