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

[sashiko-bot@kernel.org]

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