* [PATCH] HID: intel-ish-hid: ignore post-init ENUM_DEVICES from firmware
@ 2026-06-27 22:47 Wahid7852
2026-06-27 22:57 ` sashiko-bot
` (3 more replies)
0 siblings, 4 replies; 10+ messages in thread
From: Wahid7852 @ 2026-06-27 22:47 UTC (permalink / raw)
To: linux-input; +Cc: srinivas.pandruvada, jikos, Wahid7852
Some ISH firmware versions (observed on Tiger Lake LP, 8086:a0fc,
GUID {33AECD58-B679-4E54-9BD9-A04D34F0C226}) periodically re-send an
unsolicited HOSTIF_DM_ENUM_DEVICES response roughly every 79 seconds.
The current guard collapses two distinct cases into one condition:
if (!(response_flag) || init_done) -> bad packet + ish_hw_reset
This incorrectly treats a valid post-init firmware announcement as a
corrupted packet, triggering an ISH soft-reset and a full ISHTP
re-initialisation cycle on each occurrence (~1100 times per day on
affected hardware).
Split the check: reject messages with no response flag as before, but
simply discard valid ENUM_DEVICES messages that arrive after init is
complete.
Signed-off-by: Wahid <wahidzk0091@gmail.com>
---
drivers/hid/intel-ish-hid/ishtp-hid-client.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/hid/intel-ish-hid/ishtp-hid-client.c b/drivers/hid/intel-ish-hid/ishtp-hid-client.c
index 6d64008f2..ec5e8afe7 100644
--- a/drivers/hid/intel-ish-hid/ishtp-hid-client.c
+++ b/drivers/hid/intel-ish-hid/ishtp-hid-client.c
@@ -113,8 +113,7 @@ static void process_recv(struct ishtp_cl *hid_ishtp_cl, void *recv_buf,
switch (recv_msg->hdr.command & CMD_MASK) {
case HOSTIF_DM_ENUM_DEVICES:
- if ((!(recv_msg->hdr.command & ~CMD_MASK) ||
- client_data->init_done)) {
+ if (!(recv_msg->hdr.command & ~CMD_MASK)) {
++client_data->bad_recv_cnt;
report_bad_packet(hid_ishtp_cl, recv_msg,
cur_pos,
@@ -122,6 +121,8 @@ static void process_recv(struct ishtp_cl *hid_ishtp_cl, void *recv_buf,
ish_hw_reset(ishtp_get_ishtp_device(hid_ishtp_cl));
break;
}
+ if (client_data->init_done)
+ break;
client_data->hid_dev_count = (unsigned int)*payload;
if (!client_data->hid_devices)
client_data->hid_devices = devm_kcalloc(
--
2.54.0
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH] HID: intel-ish-hid: ignore post-init ENUM_DEVICES from firmware
2026-06-27 22:47 [PATCH] HID: intel-ish-hid: ignore post-init ENUM_DEVICES from firmware Wahid7852
@ 2026-06-27 22:57 ` sashiko-bot
2026-06-29 17:35 ` srinivas pandruvada
` (2 subsequent siblings)
3 siblings, 0 replies; 10+ messages in thread
From: sashiko-bot @ 2026-06-27 22:57 UTC (permalink / raw)
To: Wahid7852; +Cc: linux-input, dmitry.torokhov
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
Pre-existing issues:
- [Critical] Duplicate firmware responses during initialization (ENUM_DEVICES, GET_HID_DESCRIPTOR, GET_REPORT_DESCRIPTOR) cause heap buffer overflows.
- [High] Missing payload length validation and flawed bounds checks in HOSTIF_DM_ENUM_DEVICES parsing lead to out-of-bounds memory reads.
- [High] Unscaled pointer arithmetic and integer underflow in HOSTIF_PUBLISH_INPUT_REPORT_LIST lead to massive out-of-bounds memory accesses.
--
commit 44b4c0eea0f41216fd9e39bd67a4356eeadc37c7
Author: Wahid <wahidzk0091@gmail.com>
HID: intel-ish-hid: ignore post-init ENUM_DEVICES from firmware
This commit splits the guard condition for HOSTIF_DM_ENUM_DEVICES messages
to safely discard valid ENUM_DEVICES messages that arrive after
initialization is complete. This prevents unnecessary ISH soft-resets
caused by periodic firmware announcements.
> diff --git a/drivers/hid/intel-ish-hid/ishtp-hid-client.c b/drivers/hid/intel-ish-hid/ishtp-hid-client.c
> --- a/drivers/hid/intel-ish-hid/ishtp-hid-client.c
> +++ b/drivers/hid/intel-ish-hid/ishtp-hid-client.c
> @@ -113,8 +113,7 @@ static void process_recv(struct ishtp_cl *hid_ishtp_cl, void *recv_buf,
>
> switch (recv_msg->hdr.command & CMD_MASK) {
> case HOSTIF_DM_ENUM_DEVICES:
> - if ((!(recv_msg->hdr.command & ~CMD_MASK) ||
> - client_data->init_done)) {
> + if (!(recv_msg->hdr.command & ~CMD_MASK)) {
> ++client_data->bad_recv_cnt;
> report_bad_packet(hid_ishtp_cl, recv_msg,
> cur_pos,
> @@ -122,6 +121,8 @@ static void process_recv(struct ishtp_cl *hid_ishtp_cl, void *recv_buf,
> ish_hw_reset(ishtp_get_ishtp_device(hid_ishtp_cl));
> break;
> }
> + if (client_data->init_done)
> + break;
> client_data->hid_dev_count = (unsigned int)*payload;
[Severity: High]
This is a pre-existing issue, but does this code read out-of-bounds memory
if the payload length is zero?
If a HOSTIF_DM_ENUM_DEVICES message is received with a zero-length payload,
*payload is dereferenced here without validation.
Furthermore, in the loop that follows:
process_recv() {
...
for (i = 0; i < client_data->hid_dev_count; ++i) {
...
if (1 + sizeof(struct device_info) * i >= data_len)
break;
dev_info = (struct device_info *)(payload + 1 +
sizeof(struct device_info) * i);
if (client_data->hid_devices)
memcpy(client_data->hid_devices + i, dev_info,
sizeof(struct device_info));
...
}
The bounds check compares the relative payload offset against the absolute
buffer size (data_len) instead of the payload size. This allows the offset
to read out-of-bounds memory, which is then copied into the persistent
device information cache, leaking the contents.
> if (!client_data->hid_devices)
> client_data->hid_devices = devm_kcalloc(
[Severity: Critical]
This is a pre-existing issue, but can duplicate firmware responses cause a
heap buffer overflow here?
If multiple HOSTIF_DM_ENUM_DEVICES or GET_HID_DESCRIPTOR messages arrive
back-to-back before init_done becomes true, this allocation is incorrectly
skipped because the pointer is already non-NULL.
The memory is then copied into the existing buffer based on a potentially
larger, unvalidated hid_dev_count or payload_len:
process_recv() {
...
memcpy(client_data->hid_devices + i, dev_info,
sizeof(struct device_info));
...
memcpy(client_data->hid_descr[curr_hid_dev], payload, payload_len);
...
}
This could cause memory to be copied into the original buffer without
reallocation, causing a heap buffer overflow.
[ ... ]
[Severity: High]
This is a pre-existing issue, but is there an integer underflow and pointer
arithmetic issue when parsing HOSTIF_PUBLISH_INPUT_REPORT_LIST?
Looking further down in process_recv():
process_recv() {
...
case HOSTIF_PUBLISH_INPUT_REPORT_LIST:
...
payload_len = report_len - sizeof(struct hostif_msg_hdr);
...
report += sizeof(*report) + payload_len;
...
}
If the firmware-controlled report_len is less than 6
(sizeof(struct hostif_msg_hdr)), payload_len underflows.
Additionally, because report is a typed pointer (struct report *), this
addition is implicitly scaled by sizeof(struct report) (8 bytes), advancing
the pointer out of bounds by a factor of 8.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260627224753.35075-1-wahidzk0091@gmail.com?part=1
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] HID: intel-ish-hid: ignore post-init ENUM_DEVICES from firmware
2026-06-27 22:47 [PATCH] HID: intel-ish-hid: ignore post-init ENUM_DEVICES from firmware Wahid7852
2026-06-27 22:57 ` sashiko-bot
@ 2026-06-29 17:35 ` srinivas pandruvada
2026-07-01 3:19 ` Zhang, Lixu
2026-07-16 6:11 ` Zhang, Lixu
2026-06-29 18:04 ` Wahid7852
2026-07-20 17:56 ` [PATCH v2] " Wahid7852
3 siblings, 2 replies; 10+ messages in thread
From: srinivas pandruvada @ 2026-06-29 17:35 UTC (permalink / raw)
To: Wahid7852, linux-input, Zhang, Lixu; +Cc: jikos
+Lixu
On Sun, 2026-06-28 at 04:17 +0530, Wahid7852 wrote:
> Some ISH firmware versions (observed on Tiger Lake LP, 8086:a0fc,
> GUID {33AECD58-B679-4E54-9BD9-A04D34F0C226}) periodically re-send an
> unsolicited HOSTIF_DM_ENUM_DEVICES response roughly every 79 seconds.
>
> The current guard collapses two distinct cases into one condition:
>
> if (!(response_flag) || init_done) -> bad packet + ish_hw_reset
>
> This incorrectly treats a valid post-init firmware announcement as a
> corrupted packet, triggering an ISH soft-reset and a full ISHTP
> re-initialisation cycle on each occurrence (~1100 times per day on
> affected hardware).
>
> Split the check: reject messages with no response flag as before, but
> simply discard valid ENUM_DEVICES messages that arrive after init is
> complete.
>
> Signed-off-by: Wahid <wahidzk0091@gmail.com>
Also you need first and last name for sign offs. Not sure this is
allowed format.
Thanks,
Srinivas
> ---
> drivers/hid/intel-ish-hid/ishtp-hid-client.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/hid/intel-ish-hid/ishtp-hid-client.c
> b/drivers/hid/intel-ish-hid/ishtp-hid-client.c
> index 6d64008f2..ec5e8afe7 100644
> --- a/drivers/hid/intel-ish-hid/ishtp-hid-client.c
> +++ b/drivers/hid/intel-ish-hid/ishtp-hid-client.c
> @@ -113,8 +113,7 @@ static void process_recv(struct ishtp_cl
> *hid_ishtp_cl, void *recv_buf,
>
> switch (recv_msg->hdr.command & CMD_MASK) {
> case HOSTIF_DM_ENUM_DEVICES:
> - if ((!(recv_msg->hdr.command & ~CMD_MASK) ||
> - client_data->init_done)) {
> + if (!(recv_msg->hdr.command & ~CMD_MASK)) {
> ++client_data->bad_recv_cnt;
> report_bad_packet(hid_ishtp_cl,
> recv_msg,
> cur_pos,
> @@ -122,6 +121,8 @@ static void process_recv(struct ishtp_cl
> *hid_ishtp_cl, void *recv_buf,
> ish_hw_reset(ishtp_get_ishtp_device(
> hid_ishtp_cl));
> break;
> }
> + if (client_data->init_done)
> + break;
> client_data->hid_dev_count = (unsigned
> int)*payload;
> if (!client_data->hid_devices)
> client_data->hid_devices =
> devm_kcalloc(
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] HID: intel-ish-hid: ignore post-init ENUM_DEVICES from firmware
2026-06-27 22:47 [PATCH] HID: intel-ish-hid: ignore post-init ENUM_DEVICES from firmware Wahid7852
2026-06-27 22:57 ` sashiko-bot
2026-06-29 17:35 ` srinivas pandruvada
@ 2026-06-29 18:04 ` Wahid7852
2026-06-29 20:46 ` srinivas pandruvada
2026-07-20 17:56 ` [PATCH v2] " Wahid7852
3 siblings, 1 reply; 10+ messages in thread
From: Wahid7852 @ 2026-06-29 18:04 UTC (permalink / raw)
To: linux-input, srinivas.pandruvada; +Cc: jikos, Lixu Zhang, Wahid Khan
From: Wahid Khan <wahidzk0091@gmail.com>
Thanks. Correct sign-off should be: Signed-off-by: Wahid Khan <wahidzk0091@gmail.com>.
Should i send a follow up as v2 for this?
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] HID: intel-ish-hid: ignore post-init ENUM_DEVICES from firmware
2026-06-29 18:04 ` Wahid7852
@ 2026-06-29 20:46 ` srinivas pandruvada
0 siblings, 0 replies; 10+ messages in thread
From: srinivas pandruvada @ 2026-06-29 20:46 UTC (permalink / raw)
To: Wahid7852, linux-input; +Cc: jikos, Lixu Zhang
On Mon, 2026-06-29 at 23:34 +0530, Wahid7852 wrote:
> From: Wahid Khan <wahidzk0091@gmail.com>
>
> Thanks. Correct sign-off should be: Signed-off-by: Wahid Khan
> <wahidzk0091@gmail.com>.
> Should i send a follow up as v2 for this?
Wait for Lixu's feedback, then send v2.
Thanks,
Srinivas
^ permalink raw reply [flat|nested] 10+ messages in thread
* RE: [PATCH] HID: intel-ish-hid: ignore post-init ENUM_DEVICES from firmware
2026-06-29 17:35 ` srinivas pandruvada
@ 2026-07-01 3:19 ` Zhang, Lixu
2026-07-16 6:11 ` Zhang, Lixu
1 sibling, 0 replies; 10+ messages in thread
From: Zhang, Lixu @ 2026-07-01 3:19 UTC (permalink / raw)
To: srinivas pandruvada, Wahid7852, linux-input@vger.kernel.org
Cc: jikos@kernel.org
>-----Original Message-----
>From: srinivas pandruvada <srinivas.pandruvada@linux.intel.com>
>Sent: Tuesday, June 30, 2026 1:35 AM
>To: Wahid7852 <wahidzk0091@gmail.com>; linux-input@vger.kernel.org; Zhang,
>Lixu <lixu.zhang@intel.com>
>Cc: jikos@kernel.org
>Subject: Re: [PATCH] HID: intel-ish-hid: ignore post-init ENUM_DEVICES from
>firmware
>
>+Lixu
Thanks for your patch. I'll do some power cycle tests and provide feedback when I have some free time.
Best regards,
Lixu
^ permalink raw reply [flat|nested] 10+ messages in thread
* RE: [PATCH] HID: intel-ish-hid: ignore post-init ENUM_DEVICES from firmware
2026-06-29 17:35 ` srinivas pandruvada
2026-07-01 3:19 ` Zhang, Lixu
@ 2026-07-16 6:11 ` Zhang, Lixu
1 sibling, 0 replies; 10+ messages in thread
From: Zhang, Lixu @ 2026-07-16 6:11 UTC (permalink / raw)
To: Wahid7852, linux-input@vger.kernel.org
Cc: jikos@kernel.org, srinivas pandruvada
>-----Original Message-----
>From: Zhang, Lixu
>Sent: Wednesday, July 1, 2026 11:20 AM
>To: srinivas pandruvada <srinivas.pandruvada@linux.intel.com>; Wahid7852
><wahidzk0091@gmail.com>; linux-input@vger.kernel.org
>Cc: jikos@kernel.org
>Subject: RE: [PATCH] HID: intel-ish-hid: ignore post-init ENUM_DEVICES from
>firmware
>
>>-----Original Message-----
>>From: srinivas pandruvada <srinivas.pandruvada@linux.intel.com>
>>Sent: Tuesday, June 30, 2026 1:35 AM
>>To: Wahid7852 <wahidzk0091@gmail.com>; linux-input@vger.kernel.org;
>>Zhang, Lixu <lixu.zhang@intel.com>
>>Cc: jikos@kernel.org
>>Subject: Re: [PATCH] HID: intel-ish-hid: ignore post-init ENUM_DEVICES
>>from firmware
>>
>>+Lixu
>
>Thanks for your patch. I'll do some power cycle tests and provide feedback
>when I have some free time.
Hi Wahid,
I completed power-cycle testing on ARL and NVL platforms and found no regressions. Please go ahead and send v2.
Best regard,
Lixu
>
>Best regards,
>Lixu
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v2] HID: intel-ish-hid: ignore post-init ENUM_DEVICES from firmware
2026-06-27 22:47 [PATCH] HID: intel-ish-hid: ignore post-init ENUM_DEVICES from firmware Wahid7852
` (2 preceding siblings ...)
2026-06-29 18:04 ` Wahid7852
@ 2026-07-20 17:56 ` Wahid7852
2026-07-20 18:18 ` sashiko-bot
2026-07-20 20:59 ` srinivas pandruvada
3 siblings, 2 replies; 10+ messages in thread
From: Wahid7852 @ 2026-07-20 17:56 UTC (permalink / raw)
To: linux-input; +Cc: srinivas.pandruvada, jikos, Lixu Zhang, Wahid Khan
From: Wahid Khan <wahidzk0091@gmail.com>
Some ISH firmware versions (observed on Tiger Lake LP, 8086:a0fc,
GUID {33AECD58-B679-4E54-9BD9-A04D34F0C226}) periodically re-send an
unsolicited HOSTIF_DM_ENUM_DEVICES response roughly every 79 seconds.
The current guard collapses two distinct cases into one condition:
if (!(response_flag) || init_done) -> bad packet + ish_hw_reset
This incorrectly treats a valid post-init firmware announcement as a
corrupted packet, triggering an ISH soft-reset and a full ISHTP
re-initialisation cycle on each occurrence (~1100 times per day on
affected hardware).
Split the check: reject messages with no response flag as before, but
simply discard valid ENUM_DEVICES messages that arrive after init is
complete.
Signed-off-by: Wahid Khan <wahidzk0091@gmail.com>
---
v2: added proper sign off
drivers/hid/intel-ish-hid/ishtp-hid-client.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/hid/intel-ish-hid/ishtp-hid-client.c b/drivers/hid/intel-ish-hid/ishtp-hid-client.c
index 6d64008f2..ec5e8afe7 100644
--- a/drivers/hid/intel-ish-hid/ishtp-hid-client.c
+++ b/drivers/hid/intel-ish-hid/ishtp-hid-client.c
@@ -113,8 +113,7 @@ static void process_recv(struct ishtp_cl *hid_ishtp_cl, void *recv_buf,
switch (recv_msg->hdr.command & CMD_MASK) {
case HOSTIF_DM_ENUM_DEVICES:
- if ((!(recv_msg->hdr.command & ~CMD_MASK) ||
- client_data->init_done)) {
+ if (!(recv_msg->hdr.command & ~CMD_MASK)) {
++client_data->bad_recv_cnt;
report_bad_packet(hid_ishtp_cl, recv_msg,
cur_pos,
@@ -122,6 +121,8 @@ static void process_recv(struct ishtp_cl *hid_ishtp_cl, void *recv_buf,
ish_hw_reset(ishtp_get_ishtp_device(hid_ishtp_cl));
break;
}
+ if (client_data->init_done)
+ break;
client_data->hid_dev_count = (unsigned int)*payload;
if (!client_data->hid_devices)
client_data->hid_devices = devm_kcalloc(
--
2.54.0
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v2] HID: intel-ish-hid: ignore post-init ENUM_DEVICES from firmware
2026-07-20 17:56 ` [PATCH v2] " Wahid7852
@ 2026-07-20 18:18 ` sashiko-bot
2026-07-20 20:59 ` srinivas pandruvada
1 sibling, 0 replies; 10+ messages in thread
From: sashiko-bot @ 2026-07-20 18:18 UTC (permalink / raw)
To: Wahid7852; +Cc: dmitry.torokhov, linux-input
Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
Pre-existing issues:
- [Critical] Integer underflow and pointer arithmetic bugs in `HOSTIF_PUBLISH_INPUT_REPORT_LIST` allow massive out-of-bounds memory reads.
- [Critical] Missing bounds check on `hid_dev_count` allows an out-of-bounds write on fixed-size arrays.
- [High] Missing state validation for duplicate initialization responses causes a heap buffer overflow due to skipped reallocations.
- [High] Inadequate bounds checking in `HOSTIF_DM_ENUM_DEVICES` allows out-of-bounds memory reads from the receive buffer.
- [High] The driver fails to cancel `client_data->work` and `client_data->resume_work` during device removal, leading to a potential use-after-free.
--
--- Patch [1]: [PATCH v2] HID: intel-ish-hid: ignore post-init ENUM_DEVICES from firmware ---
commit f5198129202a132955d59473ed96cfb8c860906b
Author: Wahid Khan <wahidzk0091@gmail.com>
HID: intel-ish-hid: ignore post-init ENUM_DEVICES from firmware
This commit modifies the guard in process_recv() for the
HOSTIF_DM_ENUM_DEVICES command. It splits the check to reject messages
with no response flag, while silently discarding valid ENUM_DEVICES
messages that arrive after initialization is complete.
> diff --git a/drivers/hid/intel-ish-hid/ishtp-hid-client.c b/drivers/hid/intel-ish-hid/ishtp-hid-client.c
> index 6d64008f2ce05..ec5e8afe77bd4 100644
> --- a/drivers/hid/intel-ish-hid/ishtp-hid-client.c
> +++ b/drivers/hid/intel-ish-hid/ishtp-hid-client.c
> @@ -113,8 +113,7 @@ static void process_recv(struct ishtp_cl *hid_ishtp_cl, void *recv_buf,
>
> switch (recv_msg->hdr.command & CMD_MASK) {
> case HOSTIF_DM_ENUM_DEVICES:
> - if ((!(recv_msg->hdr.command & ~CMD_MASK) ||
> - client_data->init_done)) {
> + if (!(recv_msg->hdr.command & ~CMD_MASK)) {
> ++client_data->bad_recv_cnt;
> report_bad_packet(hid_ishtp_cl, recv_msg,
> cur_pos,
> @@ -122,6 +121,8 @@ static void process_recv(struct ishtp_cl *hid_ishtp_cl, void *recv_buf,
> ish_hw_reset(ishtp_get_ishtp_device(hid_ishtp_cl));
> break;
> }
> + if (client_data->init_done)
> + break;
> client_data->hid_dev_count = (unsigned int)*payload;
> if (!client_data->hid_devices)
> client_data->hid_devices = devm_kcalloc(
[Severity: High]
This is a pre-existing issue, but could we experience a heap buffer overflow
in process_recv() if we receive a duplicate HOSTIF_DM_ENUM_DEVICES response
before initialization completes?
The payload length or count is updated unconditionally, but reallocation
is skipped if the buffer already exists:
client_data->hid_dev_count = (unsigned int)*payload;
if (!client_data->hid_devices)
client_data->hid_devices = devm_kcalloc(...)
Would a subsequent response with a larger device count cause the memcpy loop
to write beyond the initially allocated bounds of client_data->hid_devices?
[Severity: Critical]
This is a pre-existing issue, but is there a missing bounds check on the
unvalidated device count provided by the firmware in process_recv()?
The firmware provides an unvalidated hid_dev_count (up to 255) during
HOSTIF_DM_ENUM_DEVICES, which the driver stores and uses as num_hid_devices.
When processing responses for GET_HID_DESCRIPTOR or GET_REPORT_DESCRIPTOR,
the driver uses curr_hid_dev to allocate and write into statically sized
arrays like client_data->hid_descr:
client_data->hid_descr[curr_hid_dev] = devm_kmalloc(cl_data_to_dev(client_data), payload_len, GFP_KERNEL);
Since these arrays are statically sized to MAX_HID_DEVICES (32), could a
device count larger than 32 result in a critical out-of-bounds write on the
client_data heap structure?
[Severity: High]
This is a pre-existing issue, but does inadequate bounds checking during
the parsing of HOSTIF_DM_ENUM_DEVICES in process_recv() allow out-of-bounds
memory reads from the receive buffer?
When processing ENUM_DEVICES, the loop validates the bounds of incoming
device_info structures using data_len instead of payload_len:
if (1 + sizeof(struct device_info) * i >= data_len)
break;
Additionally, if the offset exceeds payload_len, the code logs an error
but fails to break out of the loop:
if (1 + sizeof(struct device_info) * i >= payload_len) {
dev_err(cl_data_to_dev(client_data), ...);
}
Can this cause the driver to read beyond the end of the current ISHTP packet,
and potentially beyond the recv_buf allocation entirely when computing the
pointer for memcpy?
dev_info = (struct device_info *)(payload + 1 + sizeof(struct device_info) * i);
[Severity: Critical]
This is a pre-existing issue, but are there integer underflow and pointer
arithmetic bugs in the HOSTIF_PUBLISH_INPUT_REPORT_LIST case of process_recv()
that could allow massive out-of-bounds memory reads?
If an incoming report->size is less than sizeof(struct hostif_msg_hdr),
can payload_len underflow to a massive value?
payload_len = report_len - sizeof(struct hostif_msg_hdr);
Furthermore, even without underflow, does advancing the pointer using C typed
pointer arithmetic implicitly multiply the increment by sizeof(struct report)?
report += sizeof(*report) + payload_len;
Would this advance the pointer wildly out-of-bounds, feeding garbage lengths
and pointers to hid_input_report(), potentially leading to kernel panics or
data leakage?
[Severity: High]
This is a pre-existing issue, but does the driver fail to cancel pending work
during device removal, leading to a potential use-after-free?
The driver queues hardware reset and resume handlers on an ISHTP-wide shared
workqueue. However, hid_ishtp_cl_remove() exits without calling
cancel_work_sync() for client_data->work and client_data->resume_work:
drivers/hid/intel-ish-hid/ishtp-hid-client.c:hid_ishtp_cl_remove() {
...
ishtp_hid_remove(client_data);
hid_ishtp_cl_deinit(hid_ishtp_cl);
...
}
When devres frees client_data, might any concurrently running or pending work
on the shared workqueue access the freed structure?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260720175656.559351-1-wahidzk0091@gmail.com?part=1
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] HID: intel-ish-hid: ignore post-init ENUM_DEVICES from firmware
2026-07-20 17:56 ` [PATCH v2] " Wahid7852
2026-07-20 18:18 ` sashiko-bot
@ 2026-07-20 20:59 ` srinivas pandruvada
1 sibling, 0 replies; 10+ messages in thread
From: srinivas pandruvada @ 2026-07-20 20:59 UTC (permalink / raw)
To: Wahid7852, linux-input; +Cc: jikos, Lixu Zhang
On Mon, 2026-07-20 at 23:26 +0530, Wahid7852 wrote:
> From: Wahid Khan <wahidzk0091@gmail.com>
>
> Some ISH firmware versions (observed on Tiger Lake LP, 8086:a0fc,
> GUID {33AECD58-B679-4E54-9BD9-A04D34F0C226}) periodically re-send an
> unsolicited HOSTIF_DM_ENUM_DEVICES response roughly every 79 seconds.
>
> The current guard collapses two distinct cases into one condition:
>
> if (!(response_flag) || init_done) -> bad packet + ish_hw_reset
>
> This incorrectly treats a valid post-init firmware announcement as a
> corrupted packet, triggering an ISH soft-reset and a full ISHTP
> re-initialisation cycle on each occurrence (~1100 times per day on
> affected hardware).
>
> Split the check: reject messages with no response flag as before, but
> simply discard valid ENUM_DEVICES messages that arrive after init is
> complete.
>
> Signed-off-by: Wahid Khan <wahidzk0091@gmail.com>
Acked-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
Lixu, please add your tested by. I think you already tested this.
Thanks,
Srinivas
> ---
> v2: added proper sign off
>
> drivers/hid/intel-ish-hid/ishtp-hid-client.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/hid/intel-ish-hid/ishtp-hid-client.c
> b/drivers/hid/intel-ish-hid/ishtp-hid-client.c
> index 6d64008f2..ec5e8afe7 100644
> --- a/drivers/hid/intel-ish-hid/ishtp-hid-client.c
> +++ b/drivers/hid/intel-ish-hid/ishtp-hid-client.c
> @@ -113,8 +113,7 @@ static void process_recv(struct ishtp_cl
> *hid_ishtp_cl, void *recv_buf,
>
> switch (recv_msg->hdr.command & CMD_MASK) {
> case HOSTIF_DM_ENUM_DEVICES:
> - if ((!(recv_msg->hdr.command & ~CMD_MASK) ||
> - client_data->init_done)) {
> + if (!(recv_msg->hdr.command & ~CMD_MASK)) {
> ++client_data->bad_recv_cnt;
> report_bad_packet(hid_ishtp_cl,
> recv_msg,
> cur_pos,
> @@ -122,6 +121,8 @@ static void process_recv(struct ishtp_cl
> *hid_ishtp_cl, void *recv_buf,
> ish_hw_reset(ishtp_get_ishtp_device(
> hid_ishtp_cl));
> break;
> }
> + if (client_data->init_done)
> + break;
> client_data->hid_dev_count = (unsigned
> int)*payload;
> if (!client_data->hid_devices)
> client_data->hid_devices =
> devm_kcalloc(
> --
> 2.54.0
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2026-07-20 20:59 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-06-27 22:47 [PATCH] HID: intel-ish-hid: ignore post-init ENUM_DEVICES from firmware Wahid7852
2026-06-27 22:57 ` sashiko-bot
2026-06-29 17:35 ` srinivas pandruvada
2026-07-01 3:19 ` Zhang, Lixu
2026-07-16 6:11 ` Zhang, Lixu
2026-06-29 18:04 ` Wahid7852
2026-06-29 20:46 ` srinivas pandruvada
2026-07-20 17:56 ` [PATCH v2] " Wahid7852
2026-07-20 18:18 ` sashiko-bot
2026-07-20 20:59 ` srinivas pandruvada
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.