Linux Input/HID development
 help / color / mirror / Atom feed
* Re: [PATCH 1/2] input: atkbd: add softleds quirk for broken EC PS/2 emulation
From: Dmitry Torokhov @ 2026-06-28  5:20 UTC (permalink / raw)
  To: Rodnei Cilto
  Cc: Ike Panhc, Mark Pearson, Derek J. Clark, Hans de Goede,
	Ilpo Järvinen, linux-input, linux-kernel,
	platform-driver-x86
In-Reply-To: <20260628-ideapad-kbd-leds-v1-1-50b421d94927@gmail.com>

Hi Rodnei,

On Sun, Jun 28, 2026 at 10:57:03PM -0300, Rodnei Cilto wrote:
> Some Lenovo IdeaPad laptops (e.g. 83RR/83SR, Wildcat Lake) implement
> PS/2 keyboard emulation via the Embedded Controller (EC) but do not
> fully support the AT protocol. Specifically, sending the SETLEDS
> command (0xED) after initialization causes the EC to return corrupted
> scancodes (reported as '**' in i8042.debug), rendering the keyboard
> non-functional.

"**" do not represent corrupted scancodes, they are safety measure to
not disclose your password on accident.

Use i8042.debug=1 i8042.unmask_kbd_data=1 to unveil real data.

Thanks.

-- 
Dmitry

^ permalink raw reply

* Re: [regression] synaptics-rmi4: Failed to read the Control Register Descriptor: EIO
From: Dmitry Torokhov @ 2026-06-28  5:43 UTC (permalink / raw)
  To: David Heidelberg; +Cc: linux-input, Linux Next Mailing List
In-Reply-To: <916192e4-7d2c-4463-8576-3cfe9e36faa2@ixit.cz>

On Sat, Jun 27, 2026 at 12:06:29PM +0200, David Heidelberg wrote:
> On 26/06/2026 23:18, Dmitry Torokhov wrote:
> > Hi David,
> > 
> > On Fri, Jun 26, 2026 at 11:09:54PM +0200, David Heidelberg wrote:
> > > With next-20260626 without any additional patches, the synaptics fails on
> > > OnePlus 6T with:
> > > 
> > > ```
> > > [   16.620292] rmi4_i2c 12-0020: registering I2C-connected sensor
> > > [   16.857617] rmi4_f01 rmi4-00.fn01: found RMI device, manufacturer:
> > > Synaptics, product: S3706B, fw id: 2852315
> > > [   16.867324] rmi4_f12 rmi4-00.fn12: Failed to read the Control Register
> > > Descriptor: -5
> > > [   16.867338] rmi4_f12 rmi4-00.fn12: probe with driver rmi4_f12 failed with
> > > error -5
> > > ```
> > 
> > Yes, at least a98518e72439 "Input: rmi4 - fix register descriptor address
> > calculation" is wrong and needs to be reverted.
> 
> That did it for me, now the TS works again.

Thank you for letting me know.

-- 
Dmitry

^ permalink raw reply

* [PATCH] HID: hiddev: keep state alive through disconnect unlock
From: Yousef Alhouseen @ 2026-06-28  9:32 UTC (permalink / raw)
  To: Jiri Kosina, Benjamin Tissoires
  Cc: linux-input, linux-usb, linux-kernel, stable,
	syzbot+563191a4939ddbfe73d4, Yousef Alhouseen

hiddev_disconnect() drops existancelock before freeing the hiddev state,
but a waiting final file release can run as soon as the mutex becomes
available. On PREEMPT_RT, that waiter may free hiddev while the disconnect
thread is still executing the mutex slow-unlock path, causing a
use-after-free in the mutex implementation.

Give the connection and each open file an explicit reference. Drop each
reference only after its existancelock critical section has completed, so
the state cannot be freed from the other unlock path.

Fixes: 079034073faf ("HID: hiddev cleanup -- handle all error conditions properly")
Reported-by: syzbot+563191a4939ddbfe73d4@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=563191a4939ddbfe73d4
Cc: stable@vger.kernel.org
Signed-off-by: Yousef Alhouseen <alhouseenyousef@gmail.com>
---
 drivers/hid/usbhid/hiddev.c | 37 +++++++++++++++++++------------------
 include/linux/hiddev.h      |  2 ++
 2 files changed, 21 insertions(+), 18 deletions(-)

diff --git a/drivers/hid/usbhid/hiddev.c b/drivers/hid/usbhid/hiddev.c
index 6378801b22c6..21396481995b 100644
--- a/drivers/hid/usbhid/hiddev.c
+++ b/drivers/hid/usbhid/hiddev.c
@@ -46,6 +46,12 @@ struct hiddev_list {
 	struct mutex thread_lock;
 };
 
+static void hiddev_put(struct hiddev *hiddev)
+{
+	if (refcount_dec_and_test(&hiddev->refcount))
+		kfree(hiddev);
+}
+
 /*
  * Find a report, given the report's type and ID.  The ID can be specified
  * indirectly by REPORT_ID_FIRST (which returns the first report of the given
@@ -216,26 +222,21 @@ static int hiddev_fasync(int fd, struct file *file, int on)
 static int hiddev_release(struct inode * inode, struct file * file)
 {
 	struct hiddev_list *list = file->private_data;
+	struct hiddev *hiddev = list->hiddev;
 	unsigned long flags;
 
-	spin_lock_irqsave(&list->hiddev->list_lock, flags);
+	spin_lock_irqsave(&hiddev->list_lock, flags);
 	list_del(&list->node);
-	spin_unlock_irqrestore(&list->hiddev->list_lock, flags);
+	spin_unlock_irqrestore(&hiddev->list_lock, flags);
 
-	mutex_lock(&list->hiddev->existancelock);
-	if (!--list->hiddev->open) {
-		if (list->hiddev->exist) {
-			hid_hw_close(list->hiddev->hid);
-			hid_hw_power(list->hiddev->hid, PM_HINT_NORMAL);
-		} else {
-			mutex_unlock(&list->hiddev->existancelock);
-			kfree(list->hiddev);
-			vfree(list);
-			return 0;
-		}
+	mutex_lock(&hiddev->existancelock);
+	if (!--hiddev->open && hiddev->exist) {
+		hid_hw_close(hiddev->hid);
+		hid_hw_power(hiddev->hid, PM_HINT_NORMAL);
 	}
 
-	mutex_unlock(&list->hiddev->existancelock);
+	mutex_unlock(&hiddev->existancelock);
+	hiddev_put(hiddev);
 	vfree(list);
 
 	return 0;
@@ -270,6 +271,7 @@ static int __hiddev_open(struct hiddev *hiddev, struct file *file)
 	spin_unlock_irq(&hiddev->list_lock);
 
 	file->private_data = list;
+	refcount_inc(&hiddev->refcount);
 
 	return 0;
 
@@ -897,6 +899,7 @@ int hiddev_connect(struct hid_device *hid, unsigned int force)
 	INIT_LIST_HEAD(&hiddev->list);
 	spin_lock_init(&hiddev->list_lock);
 	mutex_init(&hiddev->existancelock);
+	refcount_set(&hiddev->refcount, 1);
 	hid->hiddev = hiddev;
 	hiddev->hid = hid;
 	hiddev->exist = 1;
@@ -937,9 +940,7 @@ void hiddev_disconnect(struct hid_device *hid)
 	if (hiddev->open) {
 		hid_hw_close(hiddev->hid);
 		wake_up_interruptible(&hiddev->wait);
-		mutex_unlock(&hiddev->existancelock);
-	} else {
-		mutex_unlock(&hiddev->existancelock);
-		kfree(hiddev);
 	}
+	mutex_unlock(&hiddev->existancelock);
+	hiddev_put(hiddev);
 }
diff --git a/include/linux/hiddev.h b/include/linux/hiddev.h
index 2164c03d2c72..8e9f8a33e359 100644
--- a/include/linux/hiddev.h
+++ b/include/linux/hiddev.h
@@ -13,6 +13,7 @@
 #ifndef _HIDDEV_H
 #define _HIDDEV_H
 
+#include <linux/refcount.h>
 #include <uapi/linux/hiddev.h>
 
 
@@ -24,6 +25,7 @@ struct hiddev {
 	int minor;
 	int exist;
 	int open;
+	refcount_t refcount;
 	struct mutex existancelock;
 	wait_queue_head_t wait;
 	struct hid_device *hid;
-- 
2.54.0


^ permalink raw reply related

* [PATCH] HID: mcp2221: reject short input reports
From: Yousef Alhouseen @ 2026-06-28  9:36 UTC (permalink / raw)
  To: Rishi Gupta, Jiri Kosina, Benjamin Tissoires
  Cc: linux-i2c, linux-input, linux-kernel, stable,
	syzbot+1018672fe70298606e5f, Yousef Alhouseen

The MCP2221 raw-event callback reads fixed offsets from the 64-byte input
report, including data[50] and a variable payload beginning at data[4].
Raw-event callbacks run before HID core extends short reports to their
declared size, so a malformed USB device can make these accesses run past
the received buffer.

Reject reports whose size does not match the protocol and complete the
pending command with -EMSGSIZE so its caller does not wait for a timeout.

Fixes: 67a95c21463d ("HID: mcp2221: add usb to i2c-smbus host bridge")
Reported-by: syzbot+1018672fe70298606e5f@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=1018672fe70298606e5f
Cc: stable@vger.kernel.org
Signed-off-by: Yousef Alhouseen <alhouseenyousef@gmail.com>
---
 drivers/hid/hid-mcp2221.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/hid/hid-mcp2221.c b/drivers/hid/hid-mcp2221.c
index e4ddd8e9293b..311b51c17b4e 100644
--- a/drivers/hid/hid-mcp2221.c
+++ b/drivers/hid/hid-mcp2221.c
@@ -861,6 +861,12 @@ static int mcp2221_raw_event(struct hid_device *hdev,
 	u8 *buf;
 	struct mcp2221 *mcp = hid_get_drvdata(hdev);
 
+	if (size != sizeof(mcp->txbuf)) {
+		mcp->status = -EMSGSIZE;
+		complete(&mcp->wait_in_report);
+		return 1;
+	}
+
 	switch (data[0]) {
 
 	case MCP2221_I2C_WR_DATA:
-- 
2.54.0


^ permalink raw reply related

* Re: [PATCH] HID: mcp2221: reject short input reports
From: sashiko-bot @ 2026-06-28  9:51 UTC (permalink / raw)
  To: Yousef Alhouseen; +Cc: dmitry.torokhov, linux-input
In-Reply-To: <20260628093658.43445-1-alhouseenyousef@gmail.com>

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

Pre-existing issues:
- [Critical] Use-After-Free and kernel stack corruption due to un-cleared mcp->rxbuf on timeout.
- [Critical] Stack buffer overflow due to using uninitialized memory as I2C read length.
- [High] Delayed USB reports indiscriminately complete pending waits, causing state desynchronization.
- [High] Static retries variable underflow causes infinite loop of workqueue rescheduling.
--

commit 421d556dea19ceb8f40a640c782b6801d60f43cd
Author: Yousef Alhouseen <alhouseenyousef@gmail.com>

HID: mcp2221: reject short input reports

The MCP2221 raw-event callback reads fixed offsets from the 64-byte input
report, including data[50] and a variable payload beginning at data[4].
This rejects reports whose size does not match the protocol and completes
the pending command with -EMSGSIZE.

Closes: https://syzkaller.appspot.com/bug?extid=1018672fe70298606e5f

> diff --git a/drivers/hid/hid-mcp2221.c b/drivers/hid/hid-mcp2221.c
> index e4ddd8e9293b6..311b51c17b4e4 100644
> --- a/drivers/hid/hid-mcp2221.c
> +++ b/drivers/hid/hid-mcp2221.c
> @@ -861,6 +861,12 @@ static int mcp2221_raw_event(struct hid_device *hdev,
>  	u8 *buf;
>  	struct mcp2221 *mcp = hid_get_drvdata(hdev);
>  
> +	if (size != sizeof(mcp->txbuf)) {
> +		mcp->status = -EMSGSIZE;
> +		complete(&mcp->wait_in_report);
> +		return 1;
> +	}
> +

[Severity: High]
This is a pre-existing issue, but does mcp2221_raw_event() indiscriminately
complete pending waits for any incoming report without matching it to the
pending request?

If a previous request times out, and a delayed USB response arrives later,
it appears it could prematurely wake up a new command's wait queue with
incorrect status and data:

drivers/hid/hid-mcp2221.c:mcp2221_raw_event() {
	...
	case MCP2221_I2C_WR_DATA:
	...
		complete(&mcp->wait_in_report);
		break;
	...
}

[Severity: Critical]
This isn't a bug introduced by this patch, but can this lead to stack
corruption if a request times out?

In mcp_i2c_smbus_read(), if mcp_send_data_req_status() fails or times out,
it returns immediately without clearing mcp->rxbuf:

drivers/hid/hid-mcp2221.c:mcp_i2c_smbus_read() {
	...
	ret = mcp_send_data_req_status(mcp, mcp->txbuf, 4);
	if (ret)
		return ret;
	...
}

Since mcp->rxbuf can point to a caller-provided stack buffer (like
union i2c_smbus_data), the stack frame is popped upon return. If a delayed
USB response arrives later, wouldn't mcp2221_raw_event() blindly write into
this dangling mcp->rxbuf pointer?

drivers/hid/hid-mcp2221.c:mcp2221_raw_event() {
	...
			if (data[2] == MCP2221_I2C_READ_COMPL ||
			    data[2] == MCP2221_I2C_READ_PARTIAL) {
				...
				buf = mcp->rxbuf;
				memcpy(&buf[mcp->rxbuf_idx], &data[4], data[3]);
	...
}

[Severity: Critical]
This is a pre-existing issue, but does mcp_smbus_xfer() use uninitialized
stack memory as the I2C read length during a block read?

drivers/hid/hid-mcp2221.c:mcp_smbus_xfer() {
	...
			ret = mcp_i2c_smbus_read(mcp, NULL,
						MCP2221_I2C_RD_RPT_START,
						addr, data->block[0] + 1,
						data->block);
	...
}

The union i2c_smbus_data provided by the caller is uninitialized stack memory
on read. This configures mcp->rxbuf_size to an arbitrarily large value. Could
this lead to a massive stack buffer overflow when mcp2221_raw_event() copies
up to this value into the statically sized 34-byte data->block stack buffer?

[Severity: High]
This is a pre-existing issue, but could the statically scoped retries variable
in mcp_init_work() cause an infinite reschedule loop?

drivers/hid/hid-mcp2221.c:mcp_init_work() {
	static int retries = 5;
	...
reschedule_task:
	...
	if (!retries--)
		return;

	/* Device is not ready to read SRAM or FLASH data, try again */
	schedule_delayed_work(&mcp->init_work, msecs_to_jiffies(100));
}

Since retries is static, when initialization fails 6 times, it reaches 0,
returns, and decrements to -1. On a subsequent failure, retries starts at -1.
Because !(-1) is false, it bypasses the return, decrements to -2, and
unconditionally schedules the delayed work every 100ms forever.

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

^ permalink raw reply

* Re: [git pull] Input updates for v7.2-rc0 (part 2)
From: pr-tracker-bot @ 2026-06-28 11:43 UTC (permalink / raw)
  To: Dmitry Torokhov; +Cc: Linus Torvalds, linux-kernel, linux-input
In-Reply-To: <akCs9sqkWvpJnCpK@google.com>

The pull request you sent on Sat, 27 Jun 2026 22:11:31 -0700:

> git://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git tags/input-for-v7.2-rc0-2

has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/8b69c047587112f7bcb4b0d83f2729d8dd29ebe2

Thank you!

-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/prtracker.html

^ permalink raw reply

* [PATCH] HID: intel-thc-hid: intel-quickspi: validate report size before copy
From: HyeongJun An @ 2026-06-28 13:37 UTC (permalink / raw)
  To: Even Xu, Xinpeng Sun, Jiri Kosina, Benjamin Tissoires
  Cc: linux-input, linux-kernel, HyeongJun An, stable

write_cmd_to_txdma() builds an output report in qsdev->report_buf, a heap
buffer allocated in quickspi_alloc_report_buf() to the device-descriptor
derived max_report_len (a few hundred bytes for a touch controller).  It
copies the caller-supplied report into that buffer:

	memcpy(write_buf->content, report_buf, report_buf_len);

The HID core caps a report at HID_MAX_BUFFER_SIZE (16384) by default, and
quickspi_hid_ll_driver does not set max_buffer_size, so the length reaches
the driver unbounded.  A hidraw SET_REPORT/SET_FEATURE ioctl carrying a
report larger than max_report_len therefore overflows report_buf with
attacker-controlled length and content.

Record the report_buf allocation size and reject reports that do not fit
before copying, matching the equivalent guard in the intel-quicki2c
sibling (quicki2c_init_write_buf()) and the hid-goodix-spi fix.

Fixes: 9d8d51735a3a ("HID: intel-thc-hid: intel-quickspi: Add HIDSPI protocol implementation")
Cc: stable@vger.kernel.org
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: HyeongJun An <sammiee5311@gmail.com>
---
 drivers/hid/intel-thc-hid/intel-quickspi/pci-quickspi.c      | 2 ++
 drivers/hid/intel-thc-hid/intel-quickspi/quickspi-dev.h      | 1 +
 drivers/hid/intel-thc-hid/intel-quickspi/quickspi-protocol.c | 3 +++
 3 files changed, 6 insertions(+)

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 4ae2e1718b30..1695efd5961d 100644
--- a/drivers/hid/intel-thc-hid/intel-quickspi/pci-quickspi.c
+++ b/drivers/hid/intel-thc-hid/intel-quickspi/pci-quickspi.c
@@ -559,6 +559,8 @@ static int quickspi_alloc_report_buf(struct quickspi_device *qsdev)
 	if (!qsdev->report_buf)
 		return -ENOMEM;
 
+	qsdev->report_buf_size = max_report_len;
+
 	return 0;
 }
 
diff --git a/drivers/hid/intel-thc-hid/intel-quickspi/quickspi-dev.h b/drivers/hid/intel-thc-hid/intel-quickspi/quickspi-dev.h
index bf5e18f5a5f4..0ed964bfe3dd 100644
--- a/drivers/hid/intel-thc-hid/intel-quickspi/quickspi-dev.h
+++ b/drivers/hid/intel-thc-hid/intel-quickspi/quickspi-dev.h
@@ -157,6 +157,7 @@ struct quickspi_device {
 	u8 *report_descriptor;
 	u8 *input_buf;
 	u8 *report_buf;
+	u32 report_buf_size;
 	u32 report_len;
 
 	wait_queue_head_t reset_ack_wq;
diff --git a/drivers/hid/intel-thc-hid/intel-quickspi/quickspi-protocol.c b/drivers/hid/intel-thc-hid/intel-quickspi/quickspi-protocol.c
index cb19057f1191..db6054843e77 100644
--- a/drivers/hid/intel-thc-hid/intel-quickspi/quickspi-protocol.c
+++ b/drivers/hid/intel-thc-hid/intel-quickspi/quickspi-protocol.c
@@ -30,6 +30,9 @@ static int write_cmd_to_txdma(struct quickspi_device *qsdev,
 
 	write_buf = (struct output_report *)qsdev->report_buf;
 
+	if (HIDSPI_OUTPUT_REPORT_SIZE(report_buf_len) > qsdev->report_buf_size)
+		return -EINVAL;
+
 	write_buf->output_hdr.report_type = report_type;
 	write_buf->output_hdr.content_len = cpu_to_le16(report_buf_len);
 	write_buf->output_hdr.content_id = report_id;
-- 
2.43.0


^ permalink raw reply related

* Re: [PATCH] HID: intel-thc-hid: intel-quickspi: validate report size before copy
From: sashiko-bot @ 2026-06-28 13:51 UTC (permalink / raw)
  To: HyeongJun An; +Cc: dmitry.torokhov, linux-input
In-Reply-To: <20260628133717.941389-1-sammiee5311@gmail.com>

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

New issues:
- [High] The newly introduced bounds check rejects valid, maximum-sized output reports.

Pre-existing issues:
- [Critical] Heap buffer overflow in GET_REPORT requests due to ignored user space length parameter.
- [High] Data races and waitqueue corruption due to concurrent lockless HID requests.
- [High] Heap buffer overflow in incoming report processing due to missing buffer bounds check.
- [High] Heap buffer overflow in `read_dma_buffer()` due to 4K max packet size rounding.
--

commit c642b7d6dc654ce88acbb51328c011ab2d5e04f0
Author: HyeongJun An <sammiee5311@gmail.com>

    HID: intel-thc-hid: intel-quickspi: validate report size before copy

    write_cmd_to_txdma() builds an output report in qsdev->report_buf, a heap
    buffer allocated in quickspi_alloc_report_buf() to the device-descriptor
    derived max_report_len. This patch records the report_buf allocation size
    and rejects reports that do not fit before copying.

> 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..1695efd5961d7 100644
> --- a/drivers/hid/intel-thc-hid/intel-quickspi/pci-quickspi.c
> +++ b/drivers/hid/intel-thc-hid/intel-quickspi/pci-quickspi.c
> @@ -559,6 +559,8 @@ static int quickspi_alloc_report_buf(struct quickspi_device *qsdev)
>  	if (!qsdev->report_buf)
>  		return -ENOMEM;
>  
> +	qsdev->report_buf_size = max_report_len;
> +

[Severity: High]
Since this size is based on the unadjusted device descriptor maximums, will
this reject valid maximum-sized output reports?

When the driver builds an output report in write_cmd_to_txdma(), it prepends
HIDSPI_OUTPUT_REPORT_HEADER_SIZE. If a maximum-sized report is sent, the
combined size will exceed qsdev->report_buf_size and trigger the new bounds
check.

>  	return 0;
>  }
>  

[ ... ]

> diff --git a/drivers/hid/intel-thc-hid/intel-quickspi/quickspi-protocol.c b/drivers/hid/intel-thc-hid/intel-quickspi/quickspi-protocol.c
> index cb19057f1191b..db6054843e777 100644
> --- a/drivers/hid/intel-thc-hid/intel-quickspi/quickspi-protocol.c
> +++ b/drivers/hid/intel-thc-hid/intel-quickspi/quickspi-protocol.c
> @@ -30,6 +30,9 @@ static int write_cmd_to_txdma(struct quickspi_device *qsdev,
>  
>  	write_buf = (struct output_report *)qsdev->report_buf;
>  
> +	if (HIDSPI_OUTPUT_REPORT_SIZE(report_buf_len) > qsdev->report_buf_size)
> +		return -EINVAL;
> +
>  	write_buf->output_hdr.report_type = report_type;
>  	write_buf->output_hdr.content_len = cpu_to_le16(report_buf_len);
>  	write_buf->output_hdr.content_id = report_id;
>  
>  	if (report_buf && report_buf_len > 0)
>  		memcpy(write_buf->content, report_buf, report_buf_len);

[Severity: High]
This is a pre-existing issue, but can multiple threads corrupt
qsdev->report_buf by executing this concurrently?

Since HID core does not serialize raw_request, multiple threads performing
concurrent HID ioctls (like HIDIOCGFEATURE and HIDIOCSFEATURE) could enter
write_cmd_to_txdma() simultaneously. Without locking, they would overwrite the
shared qsdev->report_buf with their respective headers and payloads.

This concurrent access could also corrupt the shared waitqueue state when
handling the responses:

drivers/hid/intel-thc-hid/intel-quickspi/quickspi-protocol.c:quickspi_handle_input_data() {
    ...
    qsdev->get_report_cmpl = true;
    wake_up_interruptible(&qsdev->get_report_cmpl_wq);
    ...
}

[Severity: Critical]
This is a pre-existing issue, but does quickspi_get_report() overflow the
caller's buffer during GET_REPORT requests?

When a user space process issues a GET_REPORT ioctl with a smaller buffer,
hid_hw_raw_request() allocates a kernel buffer matching the requested length.
However, quickspi_hid_raw_request() discards this length parameter:

drivers/hid/intel-thc-hid/intel-quickspi/quickspi-hid.c:quickspi_hid_raw_request() {
    ...
    ret = quickspi_get_report(qsdev, rtype, reportnum, buf);
    ...
}

Then quickspi_get_report() copies the full device-supplied response into this
potentially smaller buffer:

drivers/hid/intel-thc-hid/intel-quickspi/quickspi-protocol.c:quickspi_get_report() {
    ...
    memcpy(buf, qsdev->report_buf, qsdev->report_len);
    ...
}

[Severity: High]
This is a pre-existing issue, but does quickspi_handle_input_data() lack
bounds checking before copying incoming reports?

When a device responds to a GET_FEATURE or GET_INPUT_REPORT, the function
trusts the incoming input_len:

drivers/hid/intel-thc-hid/intel-quickspi/quickspi-protocol.c:quickspi_handle_input_data() {
    ...
    qsdev->report_len = sizeof(body_hdr->content_id) + input_len;
    input_report = input_body->content - sizeof(body_hdr->content_id);

    memcpy(qsdev->report_buf, input_report, qsdev->report_len);
    ...
}

If a compromised or malicious SPI device sends an artificially large input_len
that fits within the DMA transaction's packet bounds (up to 4K) but exceeds
qsdev->report_buf_size, this memcpy will overflow the strictly allocated
qsdev->report_buf.

[Severity: High]
This is a pre-existing issue, but can the 4K packet size rounding in the DMA
configuration lead to a heap overflow?

dma_set_max_packet_size() rounds up the requested DMA max packet sizes (such
as max_input_len) to a 4K boundary:

drivers/hid/intel-thc-hid/intel-thc/intel-thc-dma.c:dma_set_max_packet_size() {
    ...
    dma_config->max_packet_size = ALIGN(size, SZ_4K);
    ...
}

When read_dma_buffer() receives data, it bounds-checks mes_len against this
4K-aligned max_packet_size before copying into the caller's buffer:

drivers/hid/intel-thc-hid/intel-thc/intel-thc-dma.c:read_dma_buffer() {
    ...
    if (mes_len > read_config->max_packet_size) {
        ...
    }
    ret = sg_copy_to_buffer(sg, nent, read_buff, mes_len);
    ...
}

If a malicious device returns more data than max_input_len but less than the
4K boundary, the check passes. Since qsdev->input_buf was allocated without
this 4K alignment, the copy will overflow the buffer.

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

^ permalink raw reply


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