[PATCH v1] HID: sensor-hub: synchronize multi-value read cancellation

[PATCH v1] HID: sensor-hub: synchronize multi-value read cancellation

[Yibo Tan]

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 <lhfff@tju.edu.cn>
---
 drivers/hid/hid-sensor-hub.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/hid/hid-sensor-hub.c b/drivers/hid/hid-sensor-hub.c
index 6470a290ebfc..80f18aff6f1f 100644
--- a/drivers/hid/hid-sensor-hub.c
+++ b/drivers/hid/hid-sensor-hub.c
@@ -335,7 +335,9 @@ int sensor_hub_input_attr_read_values(struct hid_sensor_hub_device *hsdev,
 		else if (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);
 
-- 
2.39.5

Re: [PATCH v1] HID: sensor-hub: synchronize multi-value read cancellation

[Andy Shevchenko]

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:

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

Make it a Link tag and refer in the text like [1].

Link: ...$URL... [1]

> 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/

What's this for? Make sure you have a reference in the text (see above the example).

> Cc: stable@vger.kernel.org
> Assisted-by: Codex:GPT-5
> Signed-off-by: Yibo Tan <lhfff@tju.edu.cn>
> ---

...

> +++ b/drivers/hid/hid-sensor-hub.c

> +		spin_lock_irqsave(&data->lock, flags);
>  		hsdev->pending.status = false;
> +		spin_unlock_irqrestore(&data->lock, flags);

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

Re: [PATCH v1] HID: sensor-hub: synchronize multi-value read cancellation

[srinivas pandruvada]

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


> 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 <lhfff@tju.edu.cn>
> ---
>  drivers/hid/hid-sensor-hub.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/hid/hid-sensor-hub.c b/drivers/hid/hid-sensor-
> hub.c
> index 6470a290ebfc..80f18aff6f1f 100644
> --- a/drivers/hid/hid-sensor-hub.c
> +++ b/drivers/hid/hid-sensor-hub.c
> @@ -335,7 +335,9 @@ int sensor_hub_input_attr_read_values(struct
> hid_sensor_hub_device *hsdev,
>  		else if (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);
>  

RE: [PATCH v1] HID: sensor-hub: synchronize multi-value read cancellation

[Zhang, Lixu]

>-----Original Message-----
>From: srinivas pandruvada <srinivas.pandruvada@linux.intel.com>
>Sent: Friday, September 11, 2026 2:29 AM
>To: Yibo Tan <lhfff@tju.edu.cn>; Jiri Kosina <jikos@kernel.org>; Jonathan
>Cameron <jic23@kernel.org>; Benjamin Tissoires <bentiss@kernel.org>
>Cc: Zhang, Lixu <lixu.zhang@intel.com>; Shevchenko, Andriy
><andriy.shevchenko@intel.com>; 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:
>> 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

>
>Thanks,
>Srinivas