sensor_hub_input_attr_read_values() publishes a caller-owned buffer to the
raw-event path. If its interruptible wait times out or is interrupted, it
clears pending.status without taking data->lock and returns.
sensor_hub_raw_event() may already have observed pending.status while
holding that lock. The caller can then release its buffer before raw-event
finishes copying into it.
Take data->lock when cancelling the request. The raw-event path now either
sees the request retired or finishes the copy before cancellation can
return.
On an uninstrumented PREEMPT_RT kernel, a valid 16-byte quaternion report
overwrote a live futex waiter's plist node with the report's 0x41 payload.
Two vulnerable runs produced the same general protection fault in
plist_del(), after 471 and 91 completed trials. The locking fix completed
two 10,000-trial runs without an Oops, panic, warning or payload signature.
The virtual provider setup and FIFO assignment require privilege. The IIO
read, signal handling and futex operations run as uid 65534 without
effective capabilities. No physical-device or normal-priority hit was
tested.
A source reproducer, kernel configuration, complete serial logs and the
vulnerable/fixed result table are available at:
https://github.com/kimaiden1984-boop/linux-kernel-poc-collections/tree/main/cases/hid-sensor-quaternion-root-a
Fixes: f784fcea4506 ("HID: sensor-hub: Add sensor_hub_input_attr_read_values() for multi-byte reads")
Reported-by: Sashiko <sashiko-bot@kernel.org>
Link: https://lore.kernel.org/r/20260610083849.067A11F00893@smtp.kernel.org/
Cc: stable@vger.kernel.org
Assisted-by: Codex:GPT-5
Signed-off-by: Yibo Tan <redacted>
---
drivers/hid/hid-sensor-hub.c | 2 ++
1 file changed, 2 insertions(+)
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
Pre-existing issues:
- [Critical] The patch fixes a missing lock race condition in `sensor_hub_input_attr_read_values` but fails to apply the same fix to `sensor_hub_input_attr_get_raw_value`, leaving an identical race condition that causes memory leaks and completion queue corruption.
- [Critical] Unvalidated access to `report->field[i]->usage` in `sensor_hub_raw_event` causes an out-of-bounds read if a field defines zero usages.
- [Critical] Missing bounds check on `ptr` against the total received `size` in `sensor_hub_raw_event` allows an out-of-bounds read of kernel memory.
--
commit c2fc661d8850deb7960ddffa43fdc7a6bea63778
Author: Yibo Tan [off-list ref]
HID: sensor-hub: synchronize multi-value read cancellation
sensor_hub_input_attr_read_values() publishes a caller-owned buffer to the
raw-event path. If its interruptible wait times out or is interrupted, it
clears pending.status without taking data->lock and returns.
Link: https://lore.kernel.org/r/20260610083849.067A11F00893@smtp.kernel.org/
@@ -335,7 +335,9 @@ int sensor_hub_input_attr_read_values(struct hid_sensor_hub_device *hsdev,elseif(cycles<0)ret=cycles;+spin_lock_irqsave(&data->lock,flags);hsdev->pending.status=false;+spin_unlock_irqrestore(&data->lock,flags);}mutex_unlock(hsdev->mutex_ptr);
[Severity: Critical]
This is a pre-existing issue, but doesn't the exact same race condition
fixed here also exist in sensor_hub_input_attr_get_raw_value() in
drivers/hid/hid-sensor-hub.c?
If the wait times out or is interrupted, the state is modified and memory
freed without holding data->lock:
drivers/hid/hid-sensor-hub.c:sensor_hub_input_attr_get_raw_value() {
[ ... ]
kfree(hsdev->pending.raw_data);
hsdev->pending.status = false;
[ ... ]
}
Concurrently, sensor_hub_raw_event() can observe status == true while holding
the lock, and perform an allocation:
drivers/hid/hid-sensor-hub.c:sensor_hub_raw_event() {
[ ... ]
hsdev->pending.raw_data = kmemdup(ptr, sz, GFP_ATOMIC);
if (hsdev->pending.raw_data)
[ ... ]
complete(&hsdev->pending.ready);
[ ... ]
}
Can this cause a memory leak when the dynamically allocated buffer is
stranded, and waitqueue list corruption if the waiter has already exited
and reinitialized the completion queue for a new request?
[Severity: Critical]
This is a pre-existing issue, but could sensor_hub_raw_event() in
drivers/hid/hid-sensor-hub.c unconditionally access the usage array without
validating maxusage > 0?
drivers/hid/hid-sensor-hub.c:sensor_hub_raw_event() {
[ ... ]
for (i = 0; i < report->maxfield; ++i) {
hid_dbg(hdev, "%d collection_index:%x hid:%x sz:%x\n",
i, report->field[i]->usage->collection_index,
report->field[i]->usage->hid,
[ ... ]
report->field[i]->usage[0].collection_index,
[ ... ]
}
If a malicious USB device provides a crafted report descriptor defining a
field with zero usages, would this out-of-bounds array access to usage[0]
cause a kernel panic?
[Severity: Critical]
This is a pre-existing issue, but is it possible for sensor_hub_raw_event()
in drivers/hid/hid-sensor-hub.c to copy data from ptr without ensuring it
remains within the bounds of the total received report size?
drivers/hid/hid-sensor-hub.c:sensor_hub_raw_event() {
[ ... ]
memcpy(hsdev->pending.raw_data + hsdev->pending.index,
ptr, copy_size);
[ ... ]
hsdev->pending.raw_data = kmemdup(ptr, sz, GFP_ATOMIC);
[ ... ]
}
If a malicious device supplies a report descriptor defining large fields but
sends a truncated input report with a small size, could this read beyond the
end of the raw_data buffer and leak adjacent kernel memory?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260910112338.4171983-1-lhfff@tju.edu.cn?part=1
From: Andy Shevchenko <hidden> Date: 2026-09-10 15:32:26
On Thu, Sep 10, 2026 at 07:23:38PM +0800, Yibo Tan wrote:
sensor_hub_input_attr_read_values() publishes a caller-owned buffer to the
raw-event path. If its interruptible wait times out or is interrupted, it
clears pending.status without taking data->lock and returns.
sensor_hub_raw_event() may already have observed pending.status while
holding that lock. The caller can then release its buffer before raw-event
finishes copying into it.
Take data->lock when cancelling the request. The raw-event path now either
sees the request retired or finishes the copy before cancellation can
return.
On an uninstrumented PREEMPT_RT kernel, a valid 16-byte quaternion report
overwrote a live futex waiter's plist node with the report's 0x41 payload.
Two vulnerable runs produced the same general protection fault in
plist_del(), after 471 and 91 completed trials. The locking fix completed
two 10,000-trial runs without an Oops, panic, warning or payload signature.
The virtual provider setup and FIFO assignment require privilege. The IIO
read, signal handling and futex operations run as uid 65534 without
effective capabilities. No physical-device or normal-priority hit was
tested.
A source reproducer, kernel configuration, complete serial logs and the
vulnerable/fixed result table are available at:
Seems legit. Can you also amend the kernel-doc of this lock at the top of this
file? Currently it says
* @lock: Spin lock to protect pending request structure.
I would replace the tail and make it
* @lock: Spin lock to protect struct sensor_hub_pending request data.
--
With Best Regards,
Andy Shevchenko
On Thu, 2026-09-10 at 19:23 +0800, Yibo Tan wrote:
sensor_hub_input_attr_read_values() publishes a caller-owned buffer
to the
raw-event path. If its interruptible wait times out or is
interrupted, it
clears pending.status without taking data->lock and returns.
Hi Lixu,
Please give me quick test. Change itself looks good, not sure if we
need something more.
Thanks,
Srinivas
quoted hunk
sensor_hub_raw_event() may already have observed pending.status while
holding that lock. The caller can then release its buffer before raw-
event
finishes copying into it.
Take data->lock when cancelling the request. The raw-event path now
either
sees the request retired or finishes the copy before cancellation can
return.
On an uninstrumented PREEMPT_RT kernel, a valid 16-byte quaternion
report
overwrote a live futex waiter's plist node with the report's 0x41
payload.
Two vulnerable runs produced the same general protection fault in
plist_del(), after 471 and 91 completed trials. The locking fix
completed
two 10,000-trial runs without an Oops, panic, warning or payload
signature.
The virtual provider setup and FIFO assignment require privilege. The
IIO
read, signal handling and futex operations run as uid 65534 without
effective capabilities. No physical-device or normal-priority hit was
tested.
A source reproducer, kernel configuration, complete serial logs and
the
vulnerable/fixed result table are available at:
https://github.com/kimaiden1984-boop/linux-kernel-poc-collections/tree/main/cases/hid-sensor-quaternion-root-a
Fixes: f784fcea4506 ("HID: sensor-hub: Add
sensor_hub_input_attr_read_values() for multi-byte reads")
Reported-by: Sashiko <sashiko-bot@kernel.org>
Link:
https://lore.kernel.org/r/20260610083849.067A11F00893@smtp.kernel.org/
Cc: stable@vger.kernel.org
Assisted-by: Codex:GPT-5
Signed-off-by: Yibo Tan <redacted>
---
drivers/hid/hid-sensor-hub.c | 2 ++
1 file changed, 2 insertions(+)
-----Original Message-----
From: srinivas pandruvada <srinivas.pandruvada@linux.intel.com>
Sent: Friday, September 11, 2026 2:29 AM
To: Yibo Tan <redacted>; Jiri Kosina <jikos@kernel.org>; Jonathan
Cameron [off-list ref]; Benjamin Tissoires [off-list ref]
Cc: Zhang, Lixu <lixu.zhang@intel.com>; Shevchenko, Andriy
[off-list ref]; linux-input@vger.kernel.org; linux-
iio@vger.kernel.org; linux-kernel@vger.kernel.org
Subject: Re: [PATCH v1] HID: sensor-hub: synchronize multi-value read
cancellation
On Thu, 2026-09-10 at 19:23 +0800, Yibo Tan wrote:
quoted
sensor_hub_input_attr_read_values() publishes a caller-owned buffer to
the raw-event path. If its interruptible wait times out or is
interrupted, it clears pending.status without taking data->lock and
returns.
Hi Lixu,
Please give me quick test. Change itself looks good, not sure if we need
something more.
Hi Srinivas,
The machine is currently running other tests. Once they are done next week, I will run a quick test on this change and get back to you with feedback.
Thanks,
Lixu
-----Original Message-----
From: Zhang, Lixu
Sent: Friday, September 11, 2026 1:17 PM
To: srinivas pandruvada <srinivas.pandruvada@linux.intel.com>; Yibo Tan
[off-list ref]; Jiri Kosina [off-list ref]; Jonathan Cameron
[off-list ref]; Benjamin Tissoires [off-list ref]
Cc: Shevchenko, Andriy <redacted>; linux-
input@vger.kernel.org; linux-iio@vger.kernel.org; linux-kernel@vger.kernel.org
Subject: RE: [PATCH v1] HID: sensor-hub: synchronize multi-value read
cancellation
quoted
-----Original Message-----
From: srinivas pandruvada <srinivas.pandruvada@linux.intel.com>
Sent: Friday, September 11, 2026 2:29 AM
To: Yibo Tan <redacted>; Jiri Kosina <jikos@kernel.org>;
Jonathan Cameron [off-list ref]; Benjamin Tissoires
[off-list ref]
Cc: Zhang, Lixu <lixu.zhang@intel.com>; Shevchenko, Andriy
[off-list ref]; linux-input@vger.kernel.org; linux-
iio@vger.kernel.org; linux-kernel@vger.kernel.org
Subject: Re: [PATCH v1] HID: sensor-hub: synchronize multi-value read
cancellation
On Thu, 2026-09-10 at 19:23 +0800, Yibo Tan wrote:
quoted
sensor_hub_input_attr_read_values() publishes a caller-owned buffer
to the raw-event path. If its interruptible wait times out or is
interrupted, it clears pending.status without taking data->lock and
returns.
Hi Lixu,
Please give me quick test. Change itself looks good, not sure if we
need something more.
Hi Srinivas,
The machine is currently running other tests. Once they are done next week, I
will run a quick test on this change and get back to you with feedback.
Tested-by: Zhang Lixu <lixu.zhang@intel.com>
I did some basic validation on a real HID sensor hub / rotation sensor system and did not see a regression from this patch.
Thanks,
Lixu
-----Original Message-----
From: Zhang, Lixu
Sent: Friday, September 11, 2026 1:17 PM
To: srinivas pandruvada <srinivas.pandruvada@linux.intel.com>; Yibo Tan
[off-list ref]; Jiri Kosina [off-list ref]; Jonathan Cameron
[off-list ref]; Benjamin Tissoires [off-list ref]
Cc: Shevchenko, Andriy <redacted>; linux-
input@vger.kernel.org; linux-iio@vger.kernel.org; linux-kernel@vger.kernel.org
Subject: RE: [PATCH v1] HID: sensor-hub: synchronize multi-value read
cancellation
quoted
-----Original Message-----
From: srinivas pandruvada <srinivas.pandruvada@linux.intel.com>
Sent: Friday, September 11, 2026 2:29 AM
To: Yibo Tan <redacted>; Jiri Kosina <jikos@kernel.org>;
Jonathan Cameron [off-list ref]; Benjamin Tissoires
[off-list ref]
Cc: Zhang, Lixu <lixu.zhang@intel.com>; Shevchenko, Andriy
[off-list ref]; linux-input@vger.kernel.org; linux-
iio@vger.kernel.org; linux-kernel@vger.kernel.org
Subject: Re: [PATCH v1] HID: sensor-hub: synchronize multi-value read
cancellation
On Thu, 2026-09-10 at 19:23 +0800, Yibo Tan wrote:
quoted
sensor_hub_input_attr_read_values() publishes a caller-owned buffer
to the raw-event path. If its interruptible wait times out or is
interrupted, it clears pending.status without taking data->lock and
returns.
Hi Lixu,
Please give me quick test. Change itself looks good, not sure if we
need something more.
Hi Srinivas,
The machine is currently running other tests. Once they are done next week, I
will run a quick test on this change and get back to you with feedback.
Tested-by: Zhang Lixu <lixu.zhang@intel.com>
I did some basic validation on a real HID sensor hub / rotation sensor system and did not see a regression from this patch.
Great. Thanks!
Yibo Tan, please can you spin a v2 addressing the other feedback.
thanks,
Jonathan