-----Original Message-----
From: sashiko-bot@kernel.org <sashiko-bot@kernel.org>
Sent: Friday, July 3, 2026 4:11 PM
To: Xu, Even <even.xu@intel.com>
Cc: dmitry.torokhov@gmail.com; linux-input@vger.kernel.org
Subject: Re: [PATCH v2 2/3] HID: Intel-thc-hid: Intel-quicki2c: Refine recover
callback

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

New issues:
- [High] Use-After-Free due to premature `cancel_work_sync` during device
removal and shutdown.

Pre-existing issues:
- [High] Missing `pci_free_irq_vectors()` in probe error paths and device removal
leaks IRQ resources.
- [High] The hard IRQ handler accesses hardware registers unconditionally,
violating PM subsystem guidelines.
--

commit e88da9d40562f5d0be755568ae60010a7ba925df
Author: Even Xu [off-list ref]

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

This commit refines the recover flow by offloading it to a workqueue instead of
processing it within the IRQ handler. It simplifies the operation with a reset API
and ensures interrupts are disabled during the recovery process.

quoted

diff --git a/drivers/hid/intel-thc-hid/intel-quicki2c/pci-quicki2c.c

b/drivers/hid/intel-thc-hid/intel-quicki2c/pci-quicki2c.c
index 46d3e9a019995..11e0e129b44c3 100644

--- a/drivers/hid/intel-thc-hid/intel-quicki2c/pci-quicki2c.c
+++ b/drivers/hid/intel-thc-hid/intel-quicki2c/pci-quicki2c.c

@@ -245,28 +245,28 @@ static irqreturn_t

quicki2c_irq_quick_handler(int irq, void *dev_id)  }

[Severity: High]
This is a pre-existing issue, but does quicki2c_irq_quick_handler() access
hardware registers unconditionally?

If this device shares an INTx interrupt line, a spurious interrupt from another
device could cause quicki2c_irq_quick_handler() to execute. It then calls
thc_interrupt_enable() which writes to memory-mapped registers. If the device is
in a low-power state, this hardware access can trigger a bus fault.

Should pm_runtime_get_if_active() be used to check if the device is active before
touching the hardware registers?

[ ... ]

quoted

@@ -771,6 +771,8 @@ static void quicki2c_remove(struct pci_dev *pdev)
 	if (!qcdev)
 		return;

+	cancel_work_sync(&qcdev->recover_work);

THC runtime pm doesn't put THC into D3, THC registers can be accessed all the time.

[Severity: High]
Can this sequence lead to a use-after-free?

cancel_work_sync() is called here, but hardware interrupts are not yet quiesced,
and the state is still active. If an interrupt fires immediately after
cancel_work_sync(), quicki2c_irq_thread_handler() can execute and call:

    schedule_work(&qcdev->recover_work);

This re-enqueues the work item. After quicki2c_remove() returns, devres frees
the qcdev structure. When the scheduled work later executes, it will dereference
the freed qcdev pointer.

Should the work be cancelled after interrupts are disabled and the device state is
set to QUICKI2C_DISABLED?

quoted

 	quicki2c_hid_remove(qcdev);
 	quicki2c_dma_deinit(qcdev);

 	pm_runtime_get_noresume(qcdev->dev);

 	quicki2c_dev_deinit(qcdev);

 	pci_clear_master(pdev);
 }

Good finding!
In fact, I have another patch for remove() and shutdown(), if so, let me combine them into this patch.

[Severity: High]
This is a pre-existing issue, but does quicki2c_remove() leak IRQ resources?

The remove path unbinds the device but doesn't appear to call
pci_free_irq_vectors(). Similarly, if quicki2c_probe() fails after
pci_alloc_irq_vectors(), the error path (disable_pci_device) does not free the
vectors.

Repeated unbind/bind operations or probe deferrals will continually leak these
vectors, which can eventually exhaust system IRQ capacity.

This same as above.

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