Re: [PATCH v4 3/3] HID: Intel-thc-hid: Intel-quickspi: Refine recover callback

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Race condition during device teardown allows the IRQ thread handler to unconditionally schedule `recover_work` after `cancel_work_sync()` has completed, leading to a Use-After-Free.
--

--- Patch [3]: [PATCH v4 3/3] HID: Intel-thc-hid: Intel-quickspi: Refine recover callback ---
commit 476846b5d6215f9a1a461a86f427618ddd19b6d1
Author: Even Xu <even.xu@intel.com>

HID: Intel-thc-hid: Intel-quickspi: Refine recover callback

This patch updates the recovery flow to execute in a workqueue rather than
directly in the IRQ handler. It introduces a recover_work item and a
recovery_disabled flag to manage the teardown process.

> diff --git a/drivers/hid/intel-thc-hid/intel-quickspi/pci-quickspi.c b/drivers/hid/intel-thc-hid/intel-quickspi/pci-quickspi.c
> index 4ae2e1718b30c..db7ede5cc7a29 100644
> --- a/drivers/hid/intel-thc-hid/intel-quickspi/pci-quickspi.c
> +++ b/drivers/hid/intel-thc-hid/intel-quickspi/pci-quickspi.c
[ ... ]
> @@ -337,11 +334,10 @@ static irqreturn_t quickspi_irq_thread_handler(int irq, void *dev_id)
>  	}
>  
>  end:
>  	if (err_recover)
> -		if (try_recover(qsdev))
> -			qsdev->state = QUICKSPI_DISABLED;
> +		schedule_work(&qsdev->recover_work);

[Severity: High]
Can this schedule the work after it has been cancelled during device teardown?

Since the threaded IRQ handler doesn't check the recovery_disabled flag
before calling schedule_work(), if quickspi_irq_thread_handler() runs
concurrently with quickspi_dev_deinit():

CPU 1
quickspi_dev_deinit()
    WRITE_ONCE(qsdev->recovery_disabled, true);
    cancel_work_sync(&qsdev->recover_work);
    // returns, and eventually devres frees qsdev

CPU 2
quickspi_irq_thread_handler()
    ...
    if (err_recover)
        schedule_work(&qsdev->recover_work);

Could CPU 2 queue the work after CPU 1 finishes cancel_work_sync()?

If the scheduled work executes after qsdev has been freed, will this cause a
use-after-free when try_recover() reads qsdev->recovery_disabled?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260717035609.3874015-1-even.xu@intel.com?part=3