[PATCH] accel/habanalabs: fix use-after-free of decoder array on removal

[PATCH] accel/habanalabs: fix use-after-free of decoder array on removal

[Fan Wu]

The abnormal decoder interrupt queues the decoder's abnrm_intr_work
work item on the system workqueue, and the work callback uses the
decoder entry it is embedded in.

hl_device_fini() releases the decoder interrupts through
cleanup_resources() -> halt_engines() before it calls hl_dec_fini(),
which frees the decoder array without waiting for these work items.
A work item queued by one of the last interrupts can still be pending
and run after the array has been freed.

Fix this by waiting for each decoder's work item in hl_dec_fini()
before the array is freed. The error path of hl_dec_init() keeps
freeing the array directly: no interrupt has been requested yet, and
the entries past the failing one have never been initialized.

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

Fixes: d7bb1ac89b2f ("habanalabs: add gaudi2 asic-specific code")
Cc: stable@vger.kernel.org
Assisted-by: Codex:gpt-5.6
Co-developed-by: Song Li <songl@zju.edu.cn>
Signed-off-by: Song Li <songl@zju.edu.cn>
Signed-off-by: Fan Wu <fanwu01@zju.edu.cn>
---
 drivers/accel/habanalabs/common/decoder.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/accel/habanalabs/common/decoder.c b/drivers/accel/habanalabs/common/decoder.c
index e4802f30c08a..ed0c25934206 100644
--- a/drivers/accel/habanalabs/common/decoder.c
+++ b/drivers/accel/habanalabs/common/decoder.c
@@ -85,6 +85,16 @@ static void dec_abnrm_intr_work(struct work_struct *work)
 
 void hl_dec_fini(struct hl_device *hdev)
 {
+	struct asic_fixed_properties *prop = &hdev->asic_prop;
+	int i;
+
+	if (!hdev->dec)
+		return;
+
+	/* interrupts are gone, but a queued work item may still be pending */
+	for (i = 0; i < prop->max_dec; i++)
+		cancel_work_sync(&hdev->dec[i].abnrm_intr_work);
+
 	kfree(hdev->dec);
 }
 
@@ -119,7 +129,8 @@ int hl_dec_init(struct hl_device *hdev)
 	return 0;
 
 err_dec_fini:
-	hl_dec_fini(hdev);
+	/* No interrupt was requested yet, so no work could be queued. */
+	kfree(hdev->dec);
 
 	return rc;
 }

Re: [PATCH] accel/habanalabs: fix use-after-free of decoder array on removal

[sashiko-bot]

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

Pre-existing issues:
- [High] Potential Use-After-Free of `hdev` due to improper lifecycle management of `work_pci` during device unbind.
--

commit 72566230e41718de9b89ea8aac0671a21e78fed5
Author: Fan Wu <fanwu01@zju.edu.cn>

accel/habanalabs: fix use-after-free of decoder array on removal

This commit fixes a use-after-free issue by waiting for pending decoder
abnormal interrupt work items to complete before freeing the decoder array.
It cancels the work items using cancel_work_sync() during device teardown.

> diff --git a/drivers/accel/habanalabs/common/decoder.c b/drivers/accel/habanalabs/common/decoder.c
> index e4802f30c08a4..ed0c259342061 100644
> --- a/drivers/accel/habanalabs/common/decoder.c
> +++ b/drivers/accel/habanalabs/common/decoder.c
> @@ -85,6 +85,16 @@ static void dec_abnrm_intr_work(struct work_struct *work)
>  
>  void hl_dec_fini(struct hl_device *hdev)
>  {
> +	struct asic_fixed_properties *prop = &hdev->asic_prop;
> +	int i;
> +
> +	if (!hdev->dec)
> +		return;
> +
> +	/* interrupts are gone, but a queued work item may still be pending */
> +	for (i = 0; i < prop->max_dec; i++)
> +		cancel_work_sync(&hdev->dec[i].abnrm_intr_work);
> +
>  	kfree(hdev->dec);
>  }

[Severity: High]
This is a pre-existing issue, but while this patch correctly flushes the
abnrm_intr_work items to prevent a use-after-free, does a similar problem
exist with hdev->work_pci during device unbind?

Looking at drivers/accel/habanalabs/common/habanalabs_drv.c, if a
hot-unplug or sysfs unbind triggers hl_pci_remove(), the device can be
freed by devres without calling cancel_work_sync(&hdev->work_pci).

If pci_mon_func() is running concurrently:

drivers/accel/habanalabs/common/habanalabs_drv.c:pci_mon_func() {
    ...
    /* hdev is retrieved using idr_for_each_entry under hl_devs_idr_lock */
    ...
    /* The lock is released */
    queue_work(system_long_wq, &hdev->work_pci);
    ...
}

Could hl_pci_remove() complete and free the hdev structure before
queue_work() is called, or leave a pending work_pci item that executes on
the freed hdev structure?

[ ... ]

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