Linux Input/HID development
 help / color / mirror / Atom feed
* [PATCH] HID: intel-thc-hid: intel-quicki2c: fix autosuspend cleanup during teardown
@ 2026-08-08 14:57 Guangshuo Li
  2026-08-08 15:14 ` sashiko-bot
                   ` (3 more replies)
  0 siblings, 4 replies; 5+ messages in thread
From: Guangshuo Li @ 2026-08-08 14:57 UTC (permalink / raw)
  To: Even Xu, Xinpeng Sun, Jiri Kosina, Benjamin Tissoires,
	Sakari Ailus, Guangshuo Li, Mark Pearson, Srinivas Pandruvada,
	linux-input, linux-kernel
  Cc: stable

quicki2c_probe() calls pm_runtime_use_autosuspend(), but
quicki2c_remove() does not call the matching
pm_runtime_dont_use_autosuspend() during teardown.

If the autosuspend delay is set to a negative value while autosuspend
is enabled, the runtime PM core increments usage_count to prevent
runtime suspend. Without calling pm_runtime_dont_use_autosuspend()
during teardown, this reference is not dropped.

The documentation for pm_runtime_use_autosuspend() also notes that it
is important to undo it with pm_runtime_dont_use_autosuspend() at
driver exit time, unless runtime PM was initially enabled with
devm_pm_runtime_enable().

Add the missing pm_runtime_dont_use_autosuspend() call to the driver
remove path.

This issue was found by manual code inspection.

Fixes: 5f420e8215c6 ("HID: intel-thc-hid: intel-quicki2c: Add PM implementation")
Cc: stable@vger.kernel.org
Signed-off-by: Guangshuo Li <lgs201920130244@gmail.com>
---
 drivers/hid/intel-thc-hid/intel-quicki2c/pci-quicki2c.c | 1 +
 1 file changed, 1 insertion(+)

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 46d3e9a01999..4126c2408d98 100644
--- a/drivers/hid/intel-thc-hid/intel-quicki2c/pci-quicki2c.c
+++ b/drivers/hid/intel-thc-hid/intel-quicki2c/pci-quicki2c.c
@@ -774,6 +774,7 @@ static void quicki2c_remove(struct pci_dev *pdev)
 	quicki2c_hid_remove(qcdev);
 	quicki2c_dma_deinit(qcdev);
 
+	pm_runtime_dont_use_autosuspend(qcdev->dev);
 	pm_runtime_get_noresume(qcdev->dev);
 
 	quicki2c_dev_deinit(qcdev);
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] HID: intel-thc-hid: intel-quicki2c: fix autosuspend cleanup during teardown
  2026-08-08 14:57 [PATCH] HID: intel-thc-hid: intel-quicki2c: fix autosuspend cleanup during teardown Guangshuo Li
@ 2026-08-08 15:14 ` sashiko-bot
  2026-08-10  0:53 ` Xu, Even
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 5+ messages in thread
From: sashiko-bot @ 2026-08-08 15:14 UTC (permalink / raw)
  To: Guangshuo Li; +Cc: dmitry.torokhov, linux-input

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

Pre-existing issues:
- [Critical] Missing bounds check on untrusted DMA data length in `handle_input_report()` leads to out-of-bounds read and potential crash.
- [High] The driver drops the Runtime PM usage count twice in `quicki2c_probe()`, causing an unbalanced usage count and potential underflow.
- [High] The hard IRQ handler accesses memory-mapped hardware registers without verifying the device is in an active power state.
- [High] The device removal routine destroys the HID device and unconfigures DMA before disabling the hardware interrupt, allowing concurrent access to freed structures (Use-After-Free).
--

--- Patch [1]: [PATCH] HID: intel-thc-hid: intel-quicki2c: fix autosuspend cleanup during teardown ---
commit 8a35b0594fb992d3e7467b6a60b895077101a1f0
Author: Guangshuo Li <lgs201920130244@gmail.com>

HID: intel-thc-hid: intel-quicki2c: fix autosuspend cleanup during teardown
    
quicki2c_probe() calls pm_runtime_use_autosuspend(), but
quicki2c_remove() does not call the matching
pm_runtime_dont_use_autosuspend() during teardown.
    
If the autosuspend delay is set to a negative value while autosuspend
is enabled, the runtime PM core increments usage_count to prevent
runtime suspend. Without calling pm_runtime_dont_use_autosuspend()
during teardown, this reference is not dropped.

> 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..4126c2408d986 100644
> --- a/drivers/hid/intel-thc-hid/intel-quicki2c/pci-quicki2c.c
> +++ b/drivers/hid/intel-thc-hid/intel-quicki2c/pci-quicki2c.c
> @@ -774,6 +774,7 @@ static void quicki2c_remove(struct pci_dev *pdev)
>  	quicki2c_hid_remove(qcdev);
>  	quicki2c_dma_deinit(qcdev);
>  
> +	pm_runtime_dont_use_autosuspend(qcdev->dev);
>  	pm_runtime_get_noresume(qcdev->dev);
>  
>  	quicki2c_dev_deinit(qcdev);

[Severity: High]
This isn't a bug introduced by this patch, but does this teardown sequence
allow a Use-After-Free?

The device removal routine destroys the HID device via quicki2c_hid_remove()
before disabling hardware interrupts in quicki2c_dev_deinit(). If an
interrupt fires in this window, the IRQ thread executes:

quicki2c_irq_thread_handler()
   handle_input_report()
      quicki2c_hid_send_report()

which could access the freed qcdev->hid_dev structure.


drivers/hid/intel-thc-hid/intel-quicki2c/pci-quicki2c.c:quicki2c_probe() {
    ...
	pm_runtime_use_autosuspend(qcdev->dev);
	pm_runtime_set_autosuspend_delay(qcdev->dev, DEFAULT_AUTO_SUSPEND_DELAY_MS);
	pm_runtime_put_noidle(qcdev->dev);
	pm_runtime_put_autosuspend(qcdev->dev);
    ...
}

[Severity: High]
This is a pre-existing issue, but does this code drop the PM usage count
twice?

Calling pm_runtime_put_noidle() followed by pm_runtime_put_autosuspend()
drops the usage count twice when it should only be dropped once to balance
the initial count of 1.

This can cause the usage count to underflow and break runtime PM logic.


drivers/hid/intel-thc-hid/intel-quicki2c/pci-quicki2c.c:quicki2c_irq_quick_handler() {
    ...
	if (qcdev->state == QUICKI2C_DISABLED)
		return IRQ_HANDLED;

	/* Disable THC interrupt before current interrupt be handled */
	thc_interrupt_enable(qcdev->thc_hw, false);
    ...
}

[Severity: High]
This isn't a bug introduced by this patch, but does this hard IRQ handler
access memory-mapped registers without verifying the device is in an active
power state?

The PCI core can transition the device to D3hot while an in-flight MSI is
arriving.

When quicki2c_irq_quick_handler() executes in this suspended state, blindly
writing to the MMIO register via thc_interrupt_enable() can cause a PCIe
Master Abort, potentially freezing or crashing the system.


drivers/hid/intel-thc-hid/intel-quicki2c/pci-quicki2c.c:handle_input_report() {
    ...
		/* Discard samples before driver probe complete */
		if (qcdev->state != QUICKI2C_ENABLED)
			continue;

		quicki2c_hid_send_report(qcdev, pkt->data,
					 HIDI2C_DATA_LEN(le16_to_cpu(pkt->len)));
	}

	return 0;
}

[Severity: Critical]
This is a pre-existing issue, but does this code lack a bounds check on the
untrusted DMA data length?

The pkt->len field is read directly from DMA memory originating from the
peripheral.

If a malicious or faulty device supplies a len of 1, the HIDI2C_DATA_LEN()
macro calculates 1 - 2, which underflows.

Passing this value to quicki2c_hid_send_report() causes an out-of-bounds read
that can leak memory or cause a kernel panic.

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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* RE: [PATCH] HID: intel-thc-hid: intel-quicki2c: fix autosuspend cleanup during teardown
  2026-08-08 14:57 [PATCH] HID: intel-thc-hid: intel-quicki2c: fix autosuspend cleanup during teardown Guangshuo Li
  2026-08-08 15:14 ` sashiko-bot
@ 2026-08-10  0:53 ` Xu, Even
  2026-08-14 13:13 ` Jiri Kosina
  2026-08-18  8:39 ` Johan Hovold
  3 siblings, 0 replies; 5+ messages in thread
From: Xu, Even @ 2026-08-10  0:53 UTC (permalink / raw)
  To: Guangshuo Li, Sun, Xinpeng, Jiri Kosina, Benjamin Tissoires,
	Sakari Ailus, Mark Pearson, Srinivas Pandruvada,
	linux-input@vger.kernel.org, linux-kernel@vger.kernel.org
  Cc: stable@vger.kernel.org



> -----Original Message-----
> From: Guangshuo Li <lgs201920130244@gmail.com>
> Sent: Saturday, August 8, 2026 10:58 PM
> To: Xu, Even <even.xu@intel.com>; Sun, Xinpeng <xinpeng.sun@intel.com>; Jiri
> Kosina <jikos@kernel.org>; Benjamin Tissoires <bentiss@kernel.org>; Sakari Ailus
> <sakari.ailus@linux.intel.com>; Guangshuo Li <lgs201920130244@gmail.com>;
> Mark Pearson <mpearson-lenovo@squebb.ca>; Srinivas Pandruvada
> <srinivas.pandruvada@linux.intel.com>; linux-input@vger.kernel.org; linux-
> kernel@vger.kernel.org
> Cc: stable@vger.kernel.org
> Subject: [PATCH] HID: intel-thc-hid: intel-quicki2c: fix autosuspend cleanup during
> teardown
> 
> quicki2c_probe() calls pm_runtime_use_autosuspend(), but
> quicki2c_remove() does not call the matching
> pm_runtime_dont_use_autosuspend() during teardown.
> 
> If the autosuspend delay is set to a negative value while autosuspend is enabled,
> the runtime PM core increments usage_count to prevent runtime suspend.
> Without calling pm_runtime_dont_use_autosuspend() during teardown, this
> reference is not dropped.
> 
> The documentation for pm_runtime_use_autosuspend() also notes that it is
> important to undo it with pm_runtime_dont_use_autosuspend() at driver exit
> time, unless runtime PM was initially enabled with devm_pm_runtime_enable().
> 
> Add the missing pm_runtime_dont_use_autosuspend() call to the driver remove
> path.
> 
> This issue was found by manual code inspection.
> 
> Fixes: 5f420e8215c6 ("HID: intel-thc-hid: intel-quicki2c: Add PM
> implementation")
> Cc: stable@vger.kernel.org
> Signed-off-by: Guangshuo Li <lgs201920130244@gmail.com>
> ---
>  drivers/hid/intel-thc-hid/intel-quicki2c/pci-quicki2c.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> 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 46d3e9a01999..4126c2408d98 100644
> --- a/drivers/hid/intel-thc-hid/intel-quicki2c/pci-quicki2c.c
> +++ b/drivers/hid/intel-thc-hid/intel-quicki2c/pci-quicki2c.c
> @@ -774,6 +774,7 @@ static void quicki2c_remove(struct pci_dev *pdev)
>  	quicki2c_hid_remove(qcdev);
>  	quicki2c_dma_deinit(qcdev);
> 
> +	pm_runtime_dont_use_autosuspend(qcdev->dev);

Thanks for the patch!
Reviewed-by: Even Xu <even.xu@intel.com>

>  	pm_runtime_get_noresume(qcdev->dev);
> 
>  	quicki2c_dev_deinit(qcdev);
> --
> 2.43.0


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] HID: intel-thc-hid: intel-quicki2c: fix autosuspend cleanup during teardown
  2026-08-08 14:57 [PATCH] HID: intel-thc-hid: intel-quicki2c: fix autosuspend cleanup during teardown Guangshuo Li
  2026-08-08 15:14 ` sashiko-bot
  2026-08-10  0:53 ` Xu, Even
@ 2026-08-14 13:13 ` Jiri Kosina
  2026-08-18  8:39 ` Johan Hovold
  3 siblings, 0 replies; 5+ messages in thread
From: Jiri Kosina @ 2026-08-14 13:13 UTC (permalink / raw)
  To: Guangshuo Li
  Cc: Even Xu, Xinpeng Sun, Benjamin Tissoires, Sakari Ailus,
	Mark Pearson, Srinivas Pandruvada, linux-input, linux-kernel,
	stable

On Sat, 8 Aug 2026, Guangshuo Li wrote:

> quicki2c_probe() calls pm_runtime_use_autosuspend(), but
> quicki2c_remove() does not call the matching
> pm_runtime_dont_use_autosuspend() during teardown.
> 
> If the autosuspend delay is set to a negative value while autosuspend
> is enabled, the runtime PM core increments usage_count to prevent
> runtime suspend. Without calling pm_runtime_dont_use_autosuspend()
> during teardown, this reference is not dropped.
> 
> The documentation for pm_runtime_use_autosuspend() also notes that it
> is important to undo it with pm_runtime_dont_use_autosuspend() at
> driver exit time, unless runtime PM was initially enabled with
> devm_pm_runtime_enable().
> 
> Add the missing pm_runtime_dont_use_autosuspend() call to the driver
> remove path.
> 
> This issue was found by manual code inspection.
> 
> Fixes: 5f420e8215c6 ("HID: intel-thc-hid: intel-quicki2c: Add PM implementation")
> Cc: stable@vger.kernel.org
> Signed-off-by: Guangshuo Li <lgs201920130244@gmail.com>

Applied, thanks.

-- 
Jiri Kosina
SUSE Labs


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] HID: intel-thc-hid: intel-quicki2c: fix autosuspend cleanup during teardown
  2026-08-08 14:57 [PATCH] HID: intel-thc-hid: intel-quicki2c: fix autosuspend cleanup during teardown Guangshuo Li
                   ` (2 preceding siblings ...)
  2026-08-14 13:13 ` Jiri Kosina
@ 2026-08-18  8:39 ` Johan Hovold
  3 siblings, 0 replies; 5+ messages in thread
From: Johan Hovold @ 2026-08-18  8:39 UTC (permalink / raw)
  To: Guangshuo Li
  Cc: Even Xu, Xinpeng Sun, Jiri Kosina, Benjamin Tissoires,
	Sakari Ailus, Mark Pearson, Srinivas Pandruvada, linux-input,
	linux-kernel, stable

On Sat, Aug 08, 2026 at 10:57:48PM +0800, Guangshuo Li wrote:
> quicki2c_probe() calls pm_runtime_use_autosuspend(), but
> quicki2c_remove() does not call the matching
> pm_runtime_dont_use_autosuspend() during teardown.
> 
> If the autosuspend delay is set to a negative value while autosuspend
> is enabled, the runtime PM core increments usage_count to prevent
> runtime suspend. Without calling pm_runtime_dont_use_autosuspend()
> during teardown, this reference is not dropped.

I see that the HID ones have already been merged, but for the record:

As I've explained elsewhere, this is just misleading. There is no usage
count leak here as the count is balanced whenever the user re-enables
autosuspend through sysfs (by writing a non-negative timeout).

Drivers should clean up after themselves and disable autosuspend, but
this is more of a clean up than a fix and should not be backported.

You've sent upwards of 60 of these in the matter of a just a few days,
some of which have even been picked up. Please send follow-ups (replies
or v2s) as soon as possible to prevent further of these from getting
merged.

> The documentation for pm_runtime_use_autosuspend() also notes that it
> is important to undo it with pm_runtime_dont_use_autosuspend() at
> driver exit time, unless runtime PM was initially enabled with
> devm_pm_runtime_enable().
> 
> Add the missing pm_runtime_dont_use_autosuspend() call to the driver
> remove path.
> 
> This issue was found by manual code inspection.
> 
> Fixes: 5f420e8215c6 ("HID: intel-thc-hid: intel-quicki2c: Add PM implementation")
> Cc: stable@vger.kernel.org
> Signed-off-by: Guangshuo Li <lgs201920130244@gmail.com>

Johan

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-08-18  8:39 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-08 14:57 [PATCH] HID: intel-thc-hid: intel-quicki2c: fix autosuspend cleanup during teardown Guangshuo Li
2026-08-08 15:14 ` sashiko-bot
2026-08-10  0:53 ` Xu, Even
2026-08-14 13:13 ` Jiri Kosina
2026-08-18  8:39 ` Johan Hovold

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox