Linux Input/HID development
 help / color / mirror / Atom feed
* [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; 11+ 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] 11+ 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; 11+ 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] 11+ 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; 11+ 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] 11+ 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; 11+ 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] 11+ 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; 11+ 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] 11+ 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; 11+ 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] 11+ 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; 11+ 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] 11+ 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; 11+ 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] 11+ 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; 11+ 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] 11+ 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
  2026-07-21  5:04     ` Zhang, Lixu
  1 sibling, 1 reply; 11+ 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] 11+ messages in thread

* RE: [PATCH v2] HID: intel-ish-hid: ignore post-init ENUM_DEVICES from firmware
  2026-07-20 20:59   ` srinivas pandruvada
@ 2026-07-21  5:04     ` Zhang, Lixu
  0 siblings, 0 replies; 11+ messages in thread
From: Zhang, Lixu @ 2026-07-21  5:04 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, July 21, 2026 5:00 AM
>To: Wahid7852 <wahidzk0091@gmail.com>; linux-input@vger.kernel.org
>Cc: jikos@kernel.org; Zhang, Lixu <lixu.zhang@intel.com>
>Subject: Re: [PATCH v2] HID: intel-ish-hid: ignore post-init ENUM_DEVICES from
>firmware
>
>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>

Tested-by: Zhang Lixu <lixu.zhang@intel.com>

>
>
>Lixu, please add your tested by. I think you already tested this.

Yes, thanks.

Best regards,
Lixu

>
>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] 11+ messages in thread

end of thread, other threads:[~2026-07-21  5:04 UTC | newest]

Thread overview: 11+ 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
2026-07-21  5:04     ` Zhang, Lixu

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