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@xxxxxxxxxx>

> Link: https://lore.kernel.org/r/20260610083849.067A11F00893@xxxxxxxxxxxxxxx/

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

> Cc: stable@xxxxxxxxxxxxxxx
> Assisted-by: Codex:GPT-5
> Signed-off-by: Yibo Tan <lhfff@xxxxxxxxxx>
> ---

...

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