[PATCH v1] HID: sensor-hub: Fail unfinished multi-value reads on removal

[PATCH v1] HID: sensor-hub: Fail unfinished multi-value reads on removal

[Yibo Tan]

sensor_hub_remove() completes pending reads after stopping the HID device,
but does not record why they completed.  A successful completion wait
therefore returns zero even if no complete input report was received.
Multi-value IIO callers then format their untouched automatic buffer as a
successful result.

With a valid four-element signed 32-bit quaternion report descriptor, an
unprivileged reader received all 16 bytes of the untouched buffer.  Across
11 independent KASLR-enabled boots, four reads exposed exact pointers to
dev_rot_channels or dev_sysfs_ops.  Subtracting the matching link-time
symbol address recovered the kernel KASLR slide in all four cases.

The reader ran as UID/GID 65534 with no effective capabilities through the
mode-0644 IIO attribute.  The test used a privileged UHID broker to create
and remove the provider; it does not demonstrate unprivileged provider
removal.

Publish -ENODEV through pending.raw_size before completing an unfinished
multi-value request, and propagate the error after a successful wait.  A
fully received response already has a positive raw_size and remains
successful; max_raw_size is zero for the separate single-value path.

The Root B-only repair returned -ENODEV with no payload or kernel
diagnostic in 3/3 matching signed-32-bit runs.

A source reproducer, complete vulnerable and fixed serial logs, result
tables, and checksums are available at:

https://github.com/kimaiden1984-boop/linux-kernel-poc-collections/tree/main/cases/hid-sensor-quaternion-root-b-kaslr

Fixes: f784fcea4506 ("HID: sensor-hub: Add sensor_hub_input_attr_read_values() for multi-byte reads")
Cc: stable@vger.kernel.org
Assisted-by: Codex:GPT-5
Signed-off-by: Yibo Tan <lhfff@tju.edu.cn>
---
 drivers/hid/hid-sensor-hub.c   | 7 ++++++-
 include/linux/hid-sensor-hub.h | 2 +-
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/hid/hid-sensor-hub.c b/drivers/hid/hid-sensor-hub.c
index 6470a290ebfc..687f8defa2f0 100644
--- a/drivers/hid/hid-sensor-hub.c
+++ b/drivers/hid/hid-sensor-hub.c
@@ -334,6 +334,8 @@ int sensor_hub_input_attr_read_values(struct hid_sensor_hub_device *hsdev,
 			ret = -ETIMEDOUT;
 		else if (cycles < 0)
 			ret = cycles;
+		else if (hsdev->pending.raw_size < 0)
+			ret = hsdev->pending.raw_size;
 
 		hsdev->pending.status = false;
 	}
@@ -805,8 +807,11 @@ static int sensor_hub_finalize_pending_fn(struct device *dev, void *data)
 {
 	struct hid_sensor_hub_device *hsdev = dev->platform_data;
 
-	if (hsdev->pending.status)
+	if (hsdev->pending.status) {
+		if (hsdev->pending.max_raw_size && !hsdev->pending.raw_size)
+			hsdev->pending.raw_size = -ENODEV;
 		complete(&hsdev->pending.ready);
+	}
 
 	return 0;
 }
diff --git a/include/linux/hid-sensor-hub.h b/include/linux/hid-sensor-hub.h
index ab5cc8db3fbb..4ad53304bb0b 100644
--- a/include/linux/hid-sensor-hub.h
+++ b/include/linux/hid-sensor-hub.h
@@ -41,7 +41,7 @@ struct hid_sensor_hub_attribute_info {
  * @ready:		Completion synchronization data.
  * @usage_id:		Usage id for physical device, e.g. gyro usage id.
  * @attr_usage_id:	Usage Id of a field, e.g. X-axis for a gyro.
- * @raw_size:		Response size for a read request.
+ * @raw_size:		Response size or negative error for a read request.
  * @raw_data:		Place holder for received response.
  * @index:		Current write index into raw_data for multi-byte reads.
  * @max_raw_size:	Total buffer size for multi-byte reads; 0 for single-value reads.
-- 
2.39.5

Re: [PATCH v1] HID: sensor-hub: Fail unfinished multi-value reads on removal

[sashiko-bot]

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

Pre-existing issues:
- [Critical] Unsynchronized cancellation of pending requests leads to stack corruption or memory leaks when wait_for_completion_interruptible_timeout() exits early.
- [Critical] Missing validation of `field->maxusage` in `sensor_hub_raw_event()` allows an Out-Of-Bounds read on maliciously crafted HID descriptors.
- [Critical] Unconditional 16-byte read from HID report payload regardless of field size causes an Out-Of-Bounds read in dev_rot_capture_sample().
- [High] Overwriting hsdev->pending.raw_data without freeing previous allocations causes a memory leak when a report contains duplicate usages.
--

commit df43a8787a3e2f8bf5173b9ad940b826c008809a
Author: Yibo Tan <lhfff@tju.edu.cn>
Subject: HID: sensor-hub: Fail unfinished multi-value reads on removal

sensor_hub_remove() completes pending reads after stopping the HID device,
but does not record why they completed. A successful completion wait
therefore returns zero even if no complete input report was received.
Multi-value IIO callers then format their untouched automatic buffer as a
successful result.

Link: https://github.com/kimaiden1984-boop/linux-kernel-poc-collections/tree/main/cases/hid-sensor-quaternion-root-b-kaslr

> diff --git a/drivers/hid/hid-sensor-hub.c b/drivers/hid/hid-sensor-hub.c
> index 6470a290ebfc5..687f8defa2f0d 100644
> --- a/drivers/hid/hid-sensor-hub.c
> +++ b/drivers/hid/hid-sensor-hub.c
> @@ -334,6 +334,8 @@ int sensor_hub_input_attr_read_values(struct hid_sensor_hub_device *hsdev,
>  			ret = -ETIMEDOUT;
>  		else if (cycles < 0)
>  			ret = cycles;
> +		else if (hsdev->pending.raw_size < 0)
> +			ret = hsdev->pending.raw_size;
>  
>  		hsdev->pending.status = false;
>  	}

[Severity: Critical]
This is a pre-existing issue, but can clearing hsdev->pending.status locklessly
here and in sensor_hub_input_attr_get_raw_value() race with hardware event
processing?

If wait_for_completion_interruptible_timeout() returns early (due to a signal
or timeout), the waiter thread cleans up locklessly and returns, popping the
stack frame if the buffer was stack-allocated.

Concurrently, an incoming HID report could trigger sensor_hub_raw_event() which
reads status locklessly or races before it is cleared. The interrupt handler
then writes to the now-popped stack frame using memcpy(), causing stack
corruption, or allocates new memory using kmemdup() that will never be freed
after the waiter thread calls kfree().

[ ... ]

Also looking at sensor_hub_raw_event() in drivers/hid/hid-sensor-hub.c:

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,

[Severity: Critical]
This is a pre-existing issue, but is it safe to dereference usage[0] here
without verifying that the array contains any elements (i.e. maxusage > 0)?

If a maliciously crafted HID device descriptor specifies an input field with 0
usages, the HID core allocates exactly sizeof(struct hid_field) without any
trailing usages array, but field->usage is set to point directly after the
struct. Accessing report->field[i]->usage->collection_index (which is
usage[0].collection_index) would read out of bounds of the allocated slab
object.


				(report->field[i]->report_size *
					report->field[i]->report_count)/8);
		...
		if (hsdev->pending.status && ...) {
			...
			} else {
				hsdev->pending.raw_data = kmemdup(ptr, sz, GFP_ATOMIC);

[Severity: High]
This is a pre-existing issue, but could overwriting hsdev->pending.raw_data
without freeing previous allocations cause a memory leak if a report contains
duplicate usages?

If a single report is received containing multiple fields sharing the same usage
ID, sensor_hub_raw_event() iterates over all of them. For each match, if
max_raw_size is 0, it executes kmemdup() and assigns the pointer to
hsdev->pending.raw_data without calling kfree() on the previous pointer.

Since the entire loop executes synchronously under a spinlock, the waiter thread
cannot free the intermediate allocations, leading to permanent leakage of all
but the last allocation.


				if (hsdev->pending.raw_data)
					hsdev->pending.raw_size = sz;
				else
					hsdev->pending.raw_size = 0;
				complete(&hsdev->pending.ready);
			}
		}
	}
}

Also looking at dev_rot_capture_sample() in drivers/iio/orientation/hid-sensor-rotation.c:

drivers/iio/orientation/hid-sensor-rotation.c:dev_rot_capture_sample() {
	...
	if (usage_id == HID_USAGE_SENSOR_ORIENT_QUATERNION) {
		if (raw_len / 4 == sizeof(s16)) {
			...
		} else {
			memcpy(&rot_state->scan.sampled_vals, raw_data,
			       sizeof(rot_state->scan.sampled_vals));

[Severity: Critical]
This is a pre-existing issue, but are we performing an unconditional 16-byte
read here without validating that the incoming raw_data buffer is at least 16
bytes?

If a maliciously crafted HID device descriptor specifies a quaternion field with
a small size (e.g. 4 bytes total), the HID core allocates a small report buffer.
When a report is received, sensor_hub_raw_event() passes this small buffer to
the capture_sample callback, passing raw_len = 4.

Because raw_len / 4 != sizeof(s16) (1 != 2), the callback takes the else branch
and calls memcpy(&rot_state->scan.sampled_vals, raw_data, 16), reading 12 bytes
past the end of the small HID report buffer. This can leak kernel heap contents
to the userspace IIO buffer.


		}
	...
}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260911092039.234253-1-lhfff@tju.edu.cn?part=1

Re: [PATCH v1] HID: sensor-hub: Fail unfinished multi-value reads on removal

[Andy Shevchenko]

On Fri, Sep 11, 2026 at 05:20:39PM +0800, Yibo Tan wrote:
> sensor_hub_remove() completes pending reads after stopping the HID device,
> but does not record why they completed.  A successful completion wait
> therefore returns zero even if no complete input report was received.
> Multi-value IIO callers then format their untouched automatic buffer as a
> successful result.
> 
> With a valid four-element signed 32-bit quaternion report descriptor, an
> unprivileged reader received all 16 bytes of the untouched buffer.  Across
> 11 independent KASLR-enabled boots, four reads exposed exact pointers to
> dev_rot_channels or dev_sysfs_ops.  Subtracting the matching link-time
> symbol address recovered the kernel KASLR slide in all four cases.
> 
> The reader ran as UID/GID 65534 with no effective capabilities through the
> mode-0644 IIO attribute.  The test used a privileged UHID broker to create
> and remove the provider; it does not demonstrate unprivileged provider
> removal.
> 
> Publish -ENODEV through pending.raw_size before completing an unfinished
> multi-value request, and propagate the error after a successful wait.  A
> fully received response already has a positive raw_size and remains
> successful; max_raw_size is zero for the separate single-value path.
> 
> The Root B-only repair returned -ENODEV with no payload or kernel
> diagnostic in 3/3 matching signed-32-bit runs.
> 
> A source reproducer, complete vulnerable and fixed serial logs, result
> tables, and checksums are available at:

> https://github.com/kimaiden1984-boop/linux-kernel-poc-collections/tree/main/cases/hid-sensor-quaternion-root-b-kaslr

Make it a Link tag and add reference to the text.

> Fixes: f784fcea4506 ("HID: sensor-hub: Add sensor_hub_input_attr_read_values() for multi-byte reads")
> Cc: stable@vger.kernel.org
> Assisted-by: Codex:GPT-5

Assisted-by: LLM

-- 
With Best Regards,
Andy Shevchenko

[PATCH v2] HID: sensor-hub: Fail unfinished multi-value reads on removal

[Yibo Tan]

sensor_hub_remove() completes pending reads after stopping the HID device,
but does not record why they completed.  A successful completion wait
therefore returns zero even if no complete input report was received.
Multi-value IIO callers then format their untouched automatic buffer as a
successful result.

With a valid four-element signed 32-bit quaternion report descriptor, an
unprivileged reader received all 16 bytes of the untouched buffer.  Across
11 independent KASLR-enabled boots, four reads exposed exact pointers to
dev_rot_channels or dev_sysfs_ops.  Subtracting the matching link-time
symbol address recovered the kernel KASLR slide in all four cases.

The reader ran as UID/GID 65534 with no effective capabilities through the
mode-0644 IIO attribute.  The test used a privileged UHID broker to create
and remove the provider; it does not demonstrate unprivileged provider
removal.

Publish -ENODEV through pending.raw_size before completing an unfinished
multi-value request, and propagate the error after a successful wait.  A
fully received response already has a positive raw_size and remains
successful; max_raw_size is zero for the separate single-value path.

The Root B-only repair returned -ENODEV with no payload or kernel
diagnostic in 3/3 matching signed-32-bit runs.  The source reproducer,
complete vulnerable and fixed serial logs, result tables, and checksums are
available in [1].

Link: https://github.com/kimaiden1984-boop/linux-kernel-poc-collections/tree/main/cases/hid-sensor-quaternion-root-b-kaslr [1]
Fixes: f784fcea4506 ("HID: sensor-hub: Add sensor_hub_input_attr_read_values() for multi-byte reads")
Cc: stable@vger.kernel.org
Assisted-by: LLM
Signed-off-by: Yibo Tan <lhfff@tju.edu.cn>
---
Changes in v2:
- Turn the reproducer URL into a Link trailer and cite it from the text.
- Use the reviewer-requested generic LLM assistance trailer.

v1: https://lore.kernel.org/r/20260911092039.234253-1-lhfff@tju.edu.cn/

 drivers/hid/hid-sensor-hub.c   | 7 ++++++-
 include/linux/hid-sensor-hub.h | 2 +-
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/hid/hid-sensor-hub.c b/drivers/hid/hid-sensor-hub.c
index 6470a290ebfc..687f8defa2f0 100644
--- a/drivers/hid/hid-sensor-hub.c
+++ b/drivers/hid/hid-sensor-hub.c
@@ -334,6 +334,8 @@ int sensor_hub_input_attr_read_values(struct hid_sensor_hub_device *hsdev,
 			ret = -ETIMEDOUT;
 		else if (cycles < 0)
 			ret = cycles;
+		else if (hsdev->pending.raw_size < 0)
+			ret = hsdev->pending.raw_size;
 
 		hsdev->pending.status = false;
 	}
@@ -805,8 +807,11 @@ static int sensor_hub_finalize_pending_fn(struct device *dev, void *data)
 {
 	struct hid_sensor_hub_device *hsdev = dev->platform_data;
 
-	if (hsdev->pending.status)
+	if (hsdev->pending.status) {
+		if (hsdev->pending.max_raw_size && !hsdev->pending.raw_size)
+			hsdev->pending.raw_size = -ENODEV;
 		complete(&hsdev->pending.ready);
+	}
 
 	return 0;
 }
diff --git a/include/linux/hid-sensor-hub.h b/include/linux/hid-sensor-hub.h
index ab5cc8db3fbb..4ad53304bb0b 100644
--- a/include/linux/hid-sensor-hub.h
+++ b/include/linux/hid-sensor-hub.h
@@ -41,7 +41,7 @@ struct hid_sensor_hub_attribute_info {
  * @ready:		Completion synchronization data.
  * @usage_id:		Usage id for physical device, e.g. gyro usage id.
  * @attr_usage_id:	Usage Id of a field, e.g. X-axis for a gyro.
- * @raw_size:		Response size for a read request.
+ * @raw_size:		Response size or negative error for a read request.
  * @raw_data:		Place holder for received response.
  * @index:		Current write index into raw_data for multi-byte reads.
  * @max_raw_size:	Total buffer size for multi-byte reads; 0 for single-value reads.
-- 
2.39.5

Re: [PATCH v2] HID: sensor-hub: Fail unfinished multi-value reads on removal

[sashiko-bot]

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

Pre-existing issues:
- [Critical] Missing spinlock synchronization when aborting synchronous reads on timeout or signal allows a concurrent HID report to overwrite a popped stack frame or leak heap memory.
--

commit d72580296b294439ca880b1a4de796414e3195a6
Author: Yibo Tan <lhfff@tju.edu.cn>

    HID: sensor-hub: Fail unfinished multi-value reads on removal
    
    sensor_hub_remove() completes pending reads after stopping the HID device,
    but does not record why they completed.  A successful completion wait
    therefore returns zero even if no complete input report was received.

Link: https://github.com/kimaiden1984-boop/linux-kernel-poc-collections/tree/main/cases/hid-sensor-quaternion-root-b-kaslr [1]

> diff --git a/drivers/hid/hid-sensor-hub.c b/drivers/hid/hid-sensor-hub.c
> index 6470a290ebfc5..687f8defa2f0d 100644
> --- a/drivers/hid/hid-sensor-hub.c
> +++ b/drivers/hid/hid-sensor-hub.c
> @@ -334,6 +334,8 @@ int sensor_hub_input_attr_read_values(struct hid_sensor_hub_device *hsdev,
>  			ret = -ETIMEDOUT;
>  		else if (cycles < 0)
>  			ret = cycles;
> +		else if (hsdev->pending.raw_size < 0)
> +			ret = hsdev->pending.raw_size;
>  
>  		hsdev->pending.status = false;

[Severity: Critical]
This is a pre-existing issue, but missing spinlock synchronization when aborting
synchronous reads on timeout or signal allows a concurrent HID report to
overwrite a popped stack frame.

Can a local unprivileged user trigger a stack buffer overflow here by reading
a multi-value IIO attribute, sleeping in
wait_for_completion_interruptible_timeout(), and then sending a signal to
their own process?

If the wait is aborted by a signal, this code locklessly clears
hsdev->pending.status.

If a hardware HID report interrupt arrives concurrently, it acquires
pdata->lock in sensor_hub_raw_event():

drivers/hid/hid-sensor-hub.c:sensor_hub_raw_event() {
    ...
    spin_lock_irqsave(&pdata->lock, flags);
    ...
    if (hsdev->pending.status && ...) {
        if (hsdev->pending.max_raw_size) {
            ...
            memcpy(hsdev->pending.raw_data + hsdev->pending.index,
                   ptr, copy_size);

Could the interrupt observe status as true and execute memcpy() copying
attacker-influenced HID data into hsdev->pending.raw_data, which now points to
a freed stack frame since sensor_hub_input_attr_read_values() already returned?

This same pattern also appears in sensor_hub_input_attr_get_raw_value():

drivers/hid/hid-sensor-hub.c:sensor_hub_input_attr_get_raw_value() {
    ...
    if (flag == SENSOR_HUB_SYNC) {
        wait_for_completion_interruptible_timeout(&hsdev->pending.ready, HZ*5);
        ...
        kfree(hsdev->pending.raw_data);
        hsdev->pending.status = false;
    }

If an interrupt arrives immediately after the wait is aborted and executes:

drivers/hid/hid-sensor-hub.c:sensor_hub_raw_event() {
    ...
    } else {
        hsdev->pending.raw_data = kmemdup(ptr, sz, GFP_ATOMIC);
        ...
    }

Does this memory leak, since the kmemdup() allocates a buffer that the waiting
thread has already abandoned?

>  	}
>  	mutex_unlock(hsdev->mutex_ptr);
>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260912050257.837340-1-lhfff@tju.edu.cn?part=1

Re: [PATCH v2] HID: sensor-hub: Fail unfinished multi-value reads on remo

[Jonathan Cameron]

On Sat, 12 Sep 2026 13:02:57 +0800
Yibo Tan <lhfff@tju.edu.cn> wrote:

> sensor_hub_remove() completes pending reads after stopping the HID device,
> but does not record why they completed.  A successful completion wait
> therefore returns zero even if no complete input report was received.
> Multi-value IIO callers then format their untouched automatic buffer as a
> successful result.
> 
> With a valid four-element signed 32-bit quaternion report descriptor, an
> unprivileged reader received all 16 bytes of the untouched buffer.  Across
> 11 independent KASLR-enabled boots, four reads exposed exact pointers to
> dev_rot_channels or dev_sysfs_ops.  Subtracting the matching link-time
> symbol address recovered the kernel KASLR slide in all four cases.
> 
> The reader ran as UID/GID 65534 with no effective capabilities through the
> mode-0644 IIO attribute.  The test used a privileged UHID broker to create
> and remove the provider; it does not demonstrate unprivileged provider
> removal.
> 
> Publish -ENODEV through pending.raw_size before completing an unfinished
> multi-value request, and propagate the error after a successful wait.  A
> fully received response already has a positive raw_size and remains
> successful; max_raw_size is zero for the separate single-value path.
> 
> The Root B-only repair returned -ENODEV with no payload or kernel
> diagnostic in 3/3 matching signed-32-bit runs.  The source reproducer,
> complete vulnerable and fixed serial logs, result tables, and checksums are
> available in [1].
> 
> Link: https://github.com/kimaiden1984-boop/linux-kernel-poc-collections/tree/main/cases/hid-sensor-quaternion-root-b-kaslr [1]
> Fixes: f784fcea4506 ("HID: sensor-hub: Add sensor_hub_input_attr_read_values() for multi-byte reads")
> Cc: stable@vger.kernel.org
> Assisted-by: LLM
> Signed-off-by: Yibo Tan <lhfff@tju.edu.cn>

Thanks for the report and fix. Looks good to me, but I'm a little
concerned about fragility of relying on ordering of data being set
and the update of raw_size (in sensorhub_raw_event()). It is messy 
for a compiler to reorder it but I think is allowed to in theory
at least.

Perhaps it is safer for these tear down paths to just set a flag before
the shutdown extra completion and ensure an error is returned in
anything that is waiting on that completion if that flag is set?
That flag would be before the new competion so I think ordering
would be fine.  Might result in extra errors to userspace but we don't
really care when ripping things down.

Jonathan


> ---
> Changes in v2:
> - Turn the reproducer URL into a Link trailer and cite it from the text.
> - Use the reviewer-requested generic LLM assistance trailer.
> 
> v1: https://lore.kernel.org/r/20260911092039.234253-1-lhfff@tju.edu.cn/
> 
>  drivers/hid/hid-sensor-hub.c   | 7 ++++++-
>  include/linux/hid-sensor-hub.h | 2 +-
>  2 files changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/hid/hid-sensor-hub.c b/drivers/hid/hid-sensor-hub.c
> index 6470a290ebfc..687f8defa2f0 100644
> --- a/drivers/hid/hid-sensor-hub.c
> +++ b/drivers/hid/hid-sensor-hub.c
> @@ -334,6 +334,8 @@ int sensor_hub_input_attr_read_values(struct hid_sensor_hub_device *hsdev,
>  			ret = -ETIMEDOUT;
>  		else if (cycles < 0)
>  			ret = cycles;
> +		else if (hsdev->pending.raw_size < 0)
> +			ret = hsdev->pending.raw_size;
>  
>  		hsdev->pending.status = false;
>  	}
> @@ -805,8 +807,11 @@ static int sensor_hub_finalize_pending_fn(struct device *dev, void *data)
>  {
>  	struct hid_sensor_hub_device *hsdev = dev->platform_data;
>  
> -	if (hsdev->pending.status)
> +	if (hsdev->pending.status) {
> +		if (hsdev->pending.max_raw_size && !hsdev->pending.raw_size)
> +			hsdev->pending.raw_size = -ENODEV;
>  		complete(&hsdev->pending.ready);
> +	}
>  
>  	return 0;
>  }
> diff --git a/include/linux/hid-sensor-hub.h b/include/linux/hid-sensor-hub.h
> index ab5cc8db3fbb..4ad53304bb0b 100644
> --- a/include/linux/hid-sensor-hub.h
> +++ b/include/linux/hid-sensor-hub.h
> @@ -41,7 +41,7 @@ struct hid_sensor_hub_attribute_info {
>   * @ready:		Completion synchronization data.
>   * @usage_id:		Usage id for physical device, e.g. gyro usage id.
>   * @attr_usage_id:	Usage Id of a field, e.g. X-axis for a gyro.
> - * @raw_size:		Response size for a read request.
> + * @raw_size:		Response size or negative error for a read request.
>   * @raw_data:		Place holder for received response.
>   * @index:		Current write index into raw_data for multi-byte reads.
>   * @max_raw_size:	Total buffer size for multi-byte reads; 0 for single-value reads.

[PATCH v3] HID: sensor-hub: Fail unfinished multi-value reads on removal

[Yibo Tan]

sensor_hub_remove() completes pending reads after stopping the HID device,
but does not record why they completed.  A successful completion wait
therefore returns zero even if no complete input report was received.
Multi-value IIO callers then format their untouched automatic buffer as a
successful result.

With a valid four-element signed 32-bit quaternion report descriptor, an
unprivileged reader received all 16 bytes of the untouched buffer.  Across
11 independent KASLR-enabled boots, four reads exposed exact pointers to
dev_rot_channels or dev_sysfs_ops.  Subtracting the matching link-time
symbol address recovered the kernel KASLR slide in all four cases.

The reader ran as UID/GID 65534 with no effective capabilities through the
mode-0644 IIO attribute.  The test used a privileged UHID broker to create
and remove the provider; it does not demonstrate unprivileged provider
removal.

Mark a pending request as shut down before completing it from the removal
path, and return -ENODEV from a multi-value read that observes the marker
after a successful wait.  Let removal win even if a response raced with
teardown, since the device is no longer available.

The Root B-only repair returned -ENODEV with no payload or kernel
diagnostic in 3/3 matching signed-32-bit runs.  The source reproducer,
complete vulnerable and fixed serial logs, result tables, and checksums are
available in [1].

Link: https://github.com/kimaiden1984-boop/linux-kernel-poc-collections/tree/main/cases/hid-sensor-quaternion-root-b-kaslr [1]
Fixes: f784fcea4506 ("HID: sensor-hub: Add sensor_hub_input_attr_read_values() for multi-byte reads")
Cc: stable@vger.kernel.org
Suggested-by: Jonathan Cameron <jic23@kernel.org>
Assisted-by: LLM
Signed-off-by: Yibo Tan <lhfff@tju.edu.cn>
---
Changes in v3:
- Replace the raw_size error sentinel with a dedicated teardown flag, as
  suggested by Jonathan Cameron.
- Let teardown win if it races with a completed response.

v2: https://lore.kernel.org/r/20260912050257.837340-1-lhfff@tju.edu.cn/

 drivers/hid/hid-sensor-hub.c   | 6 +++++-
 include/linux/hid-sensor-hub.h | 2 ++
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/hid/hid-sensor-hub.c b/drivers/hid/hid-sensor-hub.c
index 6470a290ebfc..a9bd72218c07 100644
--- a/drivers/hid/hid-sensor-hub.c
+++ b/drivers/hid/hid-sensor-hub.c
@@ -334,6 +334,8 @@ int sensor_hub_input_attr_read_values(struct hid_sensor_hub_device *hsdev,
 			ret = -ETIMEDOUT;
 		else if (cycles < 0)
 			ret = cycles;
+		else if (hsdev->pending.shutdown)
+			ret = -ENODEV;
 
 		hsdev->pending.status = false;
 	}
@@ -805,8 +807,10 @@ static int sensor_hub_finalize_pending_fn(struct device *dev, void *data)
 {
 	struct hid_sensor_hub_device *hsdev = dev->platform_data;
 
-	if (hsdev->pending.status)
+	if (hsdev->pending.status) {
+		hsdev->pending.shutdown = true;
 		complete(&hsdev->pending.ready);
+	}
 
 	return 0;
 }
diff --git a/include/linux/hid-sensor-hub.h b/include/linux/hid-sensor-hub.h
index ab5cc8db3fbb..5aecf4474183 100644
--- a/include/linux/hid-sensor-hub.h
+++ b/include/linux/hid-sensor-hub.h
@@ -38,6 +38,7 @@ struct hid_sensor_hub_attribute_info {
 /**
  * struct sensor_hub_pending - Synchronous read pending information
  * @status:		Pending status true/false.
+ * @shutdown:		The device is being removed.
  * @ready:		Completion synchronization data.
  * @usage_id:		Usage id for physical device, e.g. gyro usage id.
  * @attr_usage_id:	Usage Id of a field, e.g. X-axis for a gyro.
@@ -48,6 +49,7 @@ struct hid_sensor_hub_attribute_info {
  */
 struct sensor_hub_pending {
 	bool status;
+	bool shutdown;
 	struct completion ready;
 	u32 usage_id;
 	u32 attr_usage_id;
-- 
2.39.5

Re: [PATCH v3] HID: sensor-hub: Fail unfinished multi-value reads on removal

[sashiko-bot]

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

Pre-existing issues:
- [High] Lockless update of hsdev->pending.status upon timeout leads to stack Use-After-Free, memory leaks, and memory corruption.
--

commit a0af07f0a44e5b5c9aefab2afc1fc904fb7bc800
Author: Yibo Tan <lhfff@tju.edu.cn>

    HID: sensor-hub: Fail unfinished multi-value reads on removal
    
    sensor_hub_remove() completes pending reads after stopping the HID device,
    but does not record why they completed.  A successful completion wait
    therefore returns zero even if no complete input report was received.
    Multi-value IIO callers then format their untouched automatic buffer as a
    successful result.

Link: https://github.com/kimaiden1984-boop/linux-kernel-poc-collections/tree/main/cases/hid-sensor-quaternion-root-b-kaslr [1]

> diff --git a/drivers/hid/hid-sensor-hub.c b/drivers/hid/hid-sensor-hub.c
> index 6470a290ebfc5..a9bd72218c070 100644
> --- a/drivers/hid/hid-sensor-hub.c
> +++ b/drivers/hid/hid-sensor-hub.c
> @@ -334,6 +334,8 @@ int sensor_hub_input_attr_read_values(struct hid_sensor_hub_device *hsdev,
>  			ret = -ETIMEDOUT;
>  		else if (cycles < 0)
>  			ret = cycles;
> +		else if (hsdev->pending.shutdown)
> +			ret = -ENODEV;
>  
>  		hsdev->pending.status = false;

[Severity: High]
This is a pre-existing issue, but does clearing hsdev->pending.status here
without acquiring data->lock create a race condition with the interrupt
handler?

If a read request times out, this function locklessly assigns status to false
and returns, freeing the caller's stack frame. 

Can the interrupt handler in sensor_hub_raw_event() then concurrently read
status as true and call memcpy() into the now-popped stack frame?

drivers/hid/hid-sensor-hub.c:sensor_hub_raw_event() {
	...
	memcpy(hsdev->pending.raw_data + hsdev->pending.index,
	       ptr, copy_size);
	...
}

Would a similar race occur in sensor_hub_input_attr_get_raw_value(), where
the interrupt handler might allocate memory via kmemdup() after the waiter
has already exited, permanently leaking the memory?

>  	}
> @@ -805,8 +807,10 @@ static int sensor_hub_finalize_pending_fn(struct device *dev, void *data)
>  {
>  	struct hid_sensor_hub_device *hsdev = dev->platform_data;
>  
> -	if (hsdev->pending.status)
> +	if (hsdev->pending.status) {
> +		hsdev->pending.shutdown = true;
>  		complete(&hsdev->pending.ready);
> +	}
>  
>  	return 0;
>  }

[Severity: High]
This is also a pre-existing issue, but can this read a stale status due to
the lockless update on the timeout path?

If a timeout just occurred and the previous reader locklessly set status to
false, could sensor_hub_finalize_pending_fn() still observe status as true
during device teardown?

If a new reader then concurrently starts and calls memset() and
init_completion(), could the complete() call here corrupt the completion's
internal spinlock?

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260913072910.1944300-1-lhfff@tju.edu.cn?part=1

Re: [PATCH v3] HID: sensor-hub: Fail unfinished multi-value reads on removal

[srinivas pandruvada]

On Sun, 2026-09-13 at 15:29 +0800, Yibo Tan wrote:
> sensor_hub_remove() completes pending reads after stopping the HID
> device,
> but does not record why they completed.  A successful completion wait
> therefore returns zero even if no complete input report was received.
> Multi-value IIO callers then format their untouched automatic buffer
> as a
> successful result.
> 
> With a valid four-element signed 32-bit quaternion report descriptor,
> an
> unprivileged reader received all 16 bytes of the untouched buffer. 
> Across
> 11 independent KASLR-enabled boots, four reads exposed exact pointers
> to
> dev_rot_channels or dev_sysfs_ops.  Subtracting the matching link-
> time
> symbol address recovered the kernel KASLR slide in all four cases.
> 
> The reader ran as UID/GID 65534 with no effective capabilities
> through the
> mode-0644 IIO attribute.  The test used a privileged UHID broker to
> create
> and remove the provider; it does not demonstrate unprivileged
> provider
> removal.
> 
> Mark a pending request as shut down before completing it from the
> removal
> path, and return -ENODEV from a multi-value read that observes the
> marker
> after a successful wait.  Let removal win even if a response raced
> with
> teardown, since the device is no longer available.
> 
> The Root B-only repair returned -ENODEV with no payload or kernel
> diagnostic in 3/3 matching signed-32-bit runs.  The source
> reproducer,
> complete vulnerable and fixed serial logs, result tables, and
> checksums are
> available in [1].
> 
> Link:
> https://github.com/kimaiden1984-boop/linux-kernel-poc-collections/tree/main/cases/hid-sensor-quaternion-root-b-kaslr
>  [1]
> Fixes: f784fcea4506 ("HID: sensor-hub: Add
> sensor_hub_input_attr_read_values() for multi-byte reads")
> Cc: stable@vger.kernel.org
> Suggested-by: Jonathan Cameron <jic23@kernel.org>
> Assisted-by: LLM
> Signed-off-by: Yibo Tan <lhfff@tju.edu.cn>


Acked-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>


> ---
> Changes in v3:
> - Replace the raw_size error sentinel with a dedicated teardown flag,
> as
>   suggested by Jonathan Cameron.
> - Let teardown win if it races with a completed response.
> 
> v2:
> https://lore.kernel.org/r/20260912050257.837340-1-lhfff@tju.edu.cn/
> 
>  drivers/hid/hid-sensor-hub.c   | 6 +++++-
>  include/linux/hid-sensor-hub.h | 2 ++
>  2 files changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/hid/hid-sensor-hub.c b/drivers/hid/hid-sensor-
> hub.c
> index 6470a290ebfc..a9bd72218c07 100644
> --- a/drivers/hid/hid-sensor-hub.c
> +++ b/drivers/hid/hid-sensor-hub.c
> @@ -334,6 +334,8 @@ int sensor_hub_input_attr_read_values(struct
> hid_sensor_hub_device *hsdev,
>  			ret = -ETIMEDOUT;
>  		else if (cycles < 0)
>  			ret = cycles;
> +		else if (hsdev->pending.shutdown)
> +			ret = -ENODEV;
>  
>  		hsdev->pending.status = false;
>  	}
> @@ -805,8 +807,10 @@ static int sensor_hub_finalize_pending_fn(struct
> device *dev, void *data)
>  {
>  	struct hid_sensor_hub_device *hsdev = dev->platform_data;
>  
> -	if (hsdev->pending.status)
> +	if (hsdev->pending.status) {
> +		hsdev->pending.shutdown = true;
>  		complete(&hsdev->pending.ready);
> +	}
>  
>  	return 0;
>  }
> diff --git a/include/linux/hid-sensor-hub.h b/include/linux/hid-
> sensor-hub.h
> index ab5cc8db3fbb..5aecf4474183 100644
> --- a/include/linux/hid-sensor-hub.h
> +++ b/include/linux/hid-sensor-hub.h
> @@ -38,6 +38,7 @@ struct hid_sensor_hub_attribute_info {
>  /**
>   * struct sensor_hub_pending - Synchronous read pending information
>   * @status:		Pending status true/false.
> + * @shutdown:		The device is being removed.
>   * @ready:		Completion synchronization data.
>   * @usage_id:		Usage id for physical device, e.g. gyro
> usage id.
>   * @attr_usage_id:	Usage Id of a field, e.g. X-axis for a gyro.
> @@ -48,6 +49,7 @@ struct hid_sensor_hub_attribute_info {
>   */
>  struct sensor_hub_pending {
>  	bool status;
> +	bool shutdown;
>  	struct completion ready;
>  	u32 usage_id;
>  	u32 attr_usage_id;

Re: [PATCH v3] HID: sensor-hub: Fail unfinished multi-value reads on removal

[Jonathan Cameron]

On Sun, 13 Sep 2026 08:50:35 -0700
srinivas pandruvada <srinivas.pandruvada@linux.intel.com> wrote:

> On Sun, 2026-09-13 at 15:29 +0800, Yibo Tan wrote:
> > sensor_hub_remove() completes pending reads after stopping the HID
> > device,
> > but does not record why they completed.  A successful completion wait
> > therefore returns zero even if no complete input report was received.
> > Multi-value IIO callers then format their untouched automatic buffer
> > as a
> > successful result.
> > 
> > With a valid four-element signed 32-bit quaternion report descriptor,
> > an
> > unprivileged reader received all 16 bytes of the untouched buffer. 
> > Across
> > 11 independent KASLR-enabled boots, four reads exposed exact pointers
> > to
> > dev_rot_channels or dev_sysfs_ops.  Subtracting the matching link-
> > time
> > symbol address recovered the kernel KASLR slide in all four cases.
> > 
> > The reader ran as UID/GID 65534 with no effective capabilities
> > through the
> > mode-0644 IIO attribute.  The test used a privileged UHID broker to
> > create
> > and remove the provider; it does not demonstrate unprivileged
> > provider
> > removal.
> > 
> > Mark a pending request as shut down before completing it from the
> > removal
> > path, and return -ENODEV from a multi-value read that observes the
> > marker
> > after a successful wait.  Let removal win even if a response raced
> > with
> > teardown, since the device is no longer available.
> > 
> > The Root B-only repair returned -ENODEV with no payload or kernel
> > diagnostic in 3/3 matching signed-32-bit runs.  The source
> > reproducer,
> > complete vulnerable and fixed serial logs, result tables, and
> > checksums are
> > available in [1].
> > 
> > Link:
> > https://github.com/kimaiden1984-boop/linux-kernel-poc-collections/tree/main/cases/hid-sensor-quaternion-root-b-kaslr
> >  [1]
> > Fixes: f784fcea4506 ("HID: sensor-hub: Add
> > sensor_hub_input_attr_read_values() for multi-byte reads")
> > Cc: stable@vger.kernel.org
> > Suggested-by: Jonathan Cameron <jic23@kernel.org>
> > Assisted-by: LLM
> > Signed-off-by: Yibo Tan <lhfff@tju.edu.cn>  
> 
> 
> Acked-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>

Applied to the fixes branch of iio.git.

It might or might not go upstream before the next merge windows. In practice
I'm going to assume these devices are typically not hotplugable except in
dev kits so we are unlikely to see this on production devices unless
someone manually removes?  That does affect whether to fix it, but only how
fast we rush it upstream!

Jonathan

> 
> 
> > ---
> > Changes in v3:
> > - Replace the raw_size error sentinel with a dedicated teardown flag,
> > as
> >   suggested by Jonathan Cameron.
> > - Let teardown win if it races with a completed response.
> > 
> > v2:
> > https://lore.kernel.org/r/20260912050257.837340-1-lhfff@tju.edu.cn/
> > 
> >  drivers/hid/hid-sensor-hub.c   | 6 +++++-
> >  include/linux/hid-sensor-hub.h | 2 ++
> >  2 files changed, 7 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/hid/hid-sensor-hub.c b/drivers/hid/hid-sensor-
> > hub.c
> > index 6470a290ebfc..a9bd72218c07 100644
> > --- a/drivers/hid/hid-sensor-hub.c
> > +++ b/drivers/hid/hid-sensor-hub.c
> > @@ -334,6 +334,8 @@ int sensor_hub_input_attr_read_values(struct
> > hid_sensor_hub_device *hsdev,
> >  			ret = -ETIMEDOUT;
> >  		else if (cycles < 0)
> >  			ret = cycles;
> > +		else if (hsdev->pending.shutdown)
> > +			ret = -ENODEV;
> >  
> >  		hsdev->pending.status = false;
> >  	}
> > @@ -805,8 +807,10 @@ static int sensor_hub_finalize_pending_fn(struct
> > device *dev, void *data)
> >  {
> >  	struct hid_sensor_hub_device *hsdev = dev->platform_data;
> >  
> > -	if (hsdev->pending.status)
> > +	if (hsdev->pending.status) {
> > +		hsdev->pending.shutdown = true;
> >  		complete(&hsdev->pending.ready);
> > +	}
> >  
> >  	return 0;
> >  }
> > diff --git a/include/linux/hid-sensor-hub.h b/include/linux/hid-
> > sensor-hub.h
> > index ab5cc8db3fbb..5aecf4474183 100644
> > --- a/include/linux/hid-sensor-hub.h
> > +++ b/include/linux/hid-sensor-hub.h
> > @@ -38,6 +38,7 @@ struct hid_sensor_hub_attribute_info {
> >  /**
> >   * struct sensor_hub_pending - Synchronous read pending information
> >   * @status:		Pending status true/false.
> > + * @shutdown:		The device is being removed.
> >   * @ready:		Completion synchronization data.
> >   * @usage_id:		Usage id for physical device, e.g. gyro
> > usage id.
> >   * @attr_usage_id:	Usage Id of a field, e.g. X-axis for a gyro.
> > @@ -48,6 +49,7 @@ struct hid_sensor_hub_attribute_info {
> >   */
> >  struct sensor_hub_pending {
> >  	bool status;
> > +	bool shutdown;
> >  	struct completion ready;
> >  	u32 usage_id;
> >  	u32 attr_usage_id;  
>