[PATCH 0/2] HID: iio: Fix stale or zero quaternion reads with multi-byte read helper

[PATCH 0/2] HID: iio: Fix stale or zero quaternion reads with multi-byte read helper

[Zhang Lixu]

When reading in_rot_quaternion_raw from hid-sensor-rotation, the driver
returns either all zeros (if the sensor was never enabled) or stale data
(if previously enabled), because it reads from an internal buffer without
requesting a fresh sample from the device.

This series fixes the issue in two steps:

1. Add sensor_hub_input_attr_read_values() to the HID sensor hub core.
   Unlike sensor_hub_input_attr_get_raw_value(), which is limited to a
   single 32-bit value, the new helper accepts a caller-provided buffer
   and accumulates incoming input report data until the buffer is full.
   The two code paths are distinguished in sensor_hub_raw_event() by
   pending.max_raw_size being non-zero, preserving backward compatibility.

2. Switch hid-sensor-rotation to use the new helper for raw quaternion
   reads. It now powers up the sensor, issues a synchronous GET_REPORT,
   and receives all four quaternion components directly into a local
   buffer before decoding them, eliminating stale or zero output.

Srinivas Pandruvada (1):
  HID: sensor-hub: Add sensor_hub_input_attr_read_values() for
    multi-byte reads

Zhang Lixu (1):
  iio: hid-sensor-rotation: Fix stale or zero output when reading raw
    values

 drivers/hid/hid-sensor-hub.c                  | 77 +++++++++++++++++--
 drivers/iio/orientation/hid-sensor-rotation.c | 40 +++++++++-
 include/linux/hid-sensor-hub.h                | 25 ++++++
 3 files changed, 134 insertions(+), 8 deletions(-)


base-commit: 49e02880ec0a8c378e811bc9d85da188d7c6204c
-- 
2.43.0

[PATCH 1/2] HID: sensor-hub: Add sensor_hub_input_attr_read_values() for multi-byte reads

[Zhang Lixu]

From: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>

sensor_hub_input_attr_get_raw_value() is limited to returning a single
32-bit value, which is insufficient for sensors that report data larger
than 32 bits, such as a quaternion with four s16 elements.

Add sensor_hub_input_attr_read_values() that accepts a caller-provided
buffer and accumulates incoming data until the buffer is full. The two
paths are distinguished in sensor_hub_raw_event() by pending.max_raw_size
being non-zero, preserving backward compatibility.

Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
Co-developed-by: Zhang Lixu <lixu.zhang@intel.com>
Signed-off-by: Zhang Lixu <lixu.zhang@intel.com>
---
 drivers/hid/hid-sensor-hub.c   | 77 +++++++++++++++++++++++++++++++---
 include/linux/hid-sensor-hub.h | 25 +++++++++++
 2 files changed, 96 insertions(+), 6 deletions(-)

diff --git a/drivers/hid/hid-sensor-hub.c b/drivers/hid/hid-sensor-hub.c
index 90666ff629def..34f710c465b80 100644
--- a/drivers/hid/hid-sensor-hub.c
+++ b/drivers/hid/hid-sensor-hub.c
@@ -286,6 +286,54 @@ int sensor_hub_get_feature(struct hid_sensor_hub_device *hsdev, u32 report_id,
 }
 EXPORT_SYMBOL_GPL(sensor_hub_get_feature);
 
+int sensor_hub_input_attr_read_values(struct hid_sensor_hub_device *hsdev,
+				      u32 usage_id, u32 attr_usage_id,
+				      u32 report_id,
+				      enum sensor_hub_read_flags flag,
+				      u32 buffer_size, u8 *buffer)
+{
+	struct sensor_hub_data *data = hid_get_drvdata(hsdev->hdev);
+	struct hid_report *report;
+	unsigned long flags;
+	long cycles;
+	int ret;
+
+	report = sensor_hub_report(report_id, hsdev->hdev, HID_INPUT_REPORT);
+	if (!report)
+		return -EINVAL;
+
+	mutex_lock(hsdev->mutex_ptr);
+	if (flag == SENSOR_HUB_SYNC) {
+		memset(&hsdev->pending, 0, sizeof(hsdev->pending));
+		init_completion(&hsdev->pending.ready);
+		hsdev->pending.usage_id = usage_id;
+		hsdev->pending.attr_usage_id = attr_usage_id;
+		hsdev->pending.max_raw_size = buffer_size;
+		hsdev->pending.raw_data = buffer;
+
+		spin_lock_irqsave(&data->lock, flags);
+		hsdev->pending.status = true;
+		spin_unlock_irqrestore(&data->lock, flags);
+	}
+	mutex_lock(&data->mutex);
+	hid_hw_request(hsdev->hdev, report, HID_REQ_GET_REPORT);
+	mutex_unlock(&data->mutex);
+	ret = 0;
+	if (flag == SENSOR_HUB_SYNC) {
+		cycles = wait_for_completion_interruptible_timeout(&hsdev->pending.ready,
+								   HZ * 5);
+		if (cycles == 0)
+			ret = -ETIMEDOUT;
+		else if (cycles < 0)
+			ret = cycles;
+
+		hsdev->pending.status = false;
+	}
+	mutex_unlock(hsdev->mutex_ptr);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(sensor_hub_input_attr_read_values);
 
 int sensor_hub_input_attr_get_raw_value(struct hid_sensor_hub_device *hsdev,
 					u32 usage_id,
@@ -478,6 +526,8 @@ static int sensor_hub_raw_event(struct hid_device *hdev,
 	struct hid_collection *collection = NULL;
 	void *priv = NULL;
 	struct hid_sensor_hub_device *hsdev = NULL;
+	u32 copy_size;
+	u32 avail;
 
 	hid_dbg(hdev, "sensor_hub_raw_event report id:0x%x size:%d type:%d\n",
 			 report->id, size, report->type);
@@ -518,12 +568,27 @@ static int sensor_hub_raw_event(struct hid_device *hdev,
 					      hsdev->pending.attr_usage_id ==
 					      report->field[i]->logical)) {
 			hid_dbg(hdev, "data was pending ...\n");
-			hsdev->pending.raw_data = kmemdup(ptr, sz, GFP_ATOMIC);
-			if (hsdev->pending.raw_data)
-				hsdev->pending.raw_size = sz;
-			else
-				hsdev->pending.raw_size = 0;
-			complete(&hsdev->pending.ready);
+			if (hsdev->pending.max_raw_size) {
+				if (hsdev->pending.index < hsdev->pending.max_raw_size) {
+					avail = hsdev->pending.max_raw_size - hsdev->pending.index;
+					copy_size = clamp(sz, 0U, avail);
+
+					memcpy(hsdev->pending.raw_data + hsdev->pending.index,
+					       ptr, copy_size);
+					hsdev->pending.index += copy_size;
+					if (hsdev->pending.index >= hsdev->pending.max_raw_size) {
+						hsdev->pending.raw_size = hsdev->pending.index;
+						complete(&hsdev->pending.ready);
+					}
+				}
+			} else {
+				hsdev->pending.raw_data = kmemdup(ptr, sz, GFP_ATOMIC);
+				if (hsdev->pending.raw_data)
+					hsdev->pending.raw_size = sz;
+				else
+					hsdev->pending.raw_size = 0;
+				complete(&hsdev->pending.ready);
+			}
 		}
 		if (callback->capture_sample) {
 			if (report->field[i]->logical)
diff --git a/include/linux/hid-sensor-hub.h b/include/linux/hid-sensor-hub.h
index e710565531085..ab5cc8db3fbb3 100644
--- a/include/linux/hid-sensor-hub.h
+++ b/include/linux/hid-sensor-hub.h
@@ -43,6 +43,8 @@ struct hid_sensor_hub_attribute_info {
  * @attr_usage_id:	Usage Id of a field, e.g. X-axis for a gyro.
  * @raw_size:		Response size 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.
  */
 struct sensor_hub_pending {
 	bool status;
@@ -51,6 +53,8 @@ struct sensor_hub_pending {
 	u32 attr_usage_id;
 	int raw_size;
 	u8  *raw_data;
+	u32 index;
+	u32 max_raw_size;
 };
 
 /**
@@ -183,6 +187,27 @@ int sensor_hub_input_attr_get_raw_value(struct hid_sensor_hub_device *hsdev,
 					bool is_signed
 );
 
+/**
+ * sensor_hub_input_attr_read_values() - Synchronous multi-byte read request
+ * @hsdev:		Hub device instance.
+ * @usage_id:		Attribute usage id of parent physical device as per spec
+ * @attr_usage_id:	Attribute usage id as per spec
+ * @report_id:		Report id to look for
+ * @flag:		Synchronous or asynchronous read
+ * @buffer_size:	Size of the buffer in bytes
+ * @buffer:		Buffer to store the read data
+ *
+ * Issues a synchronous or asynchronous read request for an input attribute,
+ * accumulating data into the provided buffer until it is full.
+ * Return: 0 on success, -ETIMEDOUT if the device did not respond, or a
+ * negative error code.
+ */
+int sensor_hub_input_attr_read_values(struct hid_sensor_hub_device *hsdev,
+				      u32 usage_id, u32 attr_usage_id,
+				      u32 report_id,
+				      enum sensor_hub_read_flags flag,
+				      u32 buffer_size, u8 *buffer);
+
 /**
 * sensor_hub_set_feature() - Feature set request
 * @hsdev:	Hub device instance.
-- 
2.43.0

[PATCH 2/2] iio: hid-sensor-rotation: Fix stale or zero output when reading raw values

[Zhang Lixu]

When reading the raw quaternion attribute (in_rot_quaternion_raw), the
driver currently returns either all zeros (if the sensor was never enabled)
or stale data (if the sensor was previously enabled) because it reads from
the internal buffer without explicitly requesting a new sample from the
sensor.

To fix this, power up the sensor, call sensor_hub_input_attr_read_values()
to issue a synchronous GET_REPORT and receive the full quaternion data
directly into a local buffer, then decode the four components.

Signed-off-by: Zhang Lixu <lixu.zhang@intel.com>
---
 drivers/iio/orientation/hid-sensor-rotation.c | 40 ++++++++++++++++++-
 1 file changed, 38 insertions(+), 2 deletions(-)

diff --git a/drivers/iio/orientation/hid-sensor-rotation.c b/drivers/iio/orientation/hid-sensor-rotation.c
index 4a11e45550991..1c6f02374f3ce 100644
--- a/drivers/iio/orientation/hid-sensor-rotation.c
+++ b/drivers/iio/orientation/hid-sensor-rotation.c
@@ -86,6 +86,13 @@ static int dev_rot_read_raw(struct iio_dev *indio_dev,
 				long mask)
 {
 	struct dev_rot_state *rot_state = iio_priv(indio_dev);
+	struct hid_sensor_hub_device *hsdev = rot_state->common_attributes.hsdev;
+	struct hid_sensor_hub_attribute_info *info = &rot_state->quaternion;
+	u32 usage_id = HID_USAGE_SENSOR_ORIENT_QUATERNION;
+	union {
+		s16 val16[4];
+		s32 val32[4];
+	} raw_buf;
 	int ret_type;
 	int i;
 
@@ -95,8 +102,37 @@ static int dev_rot_read_raw(struct iio_dev *indio_dev,
 	switch (mask) {
 	case IIO_CHAN_INFO_RAW:
 		if (size >= 4) {
-			for (i = 0; i < 4; ++i)
-				vals[i] = rot_state->scan.sampled_vals[i];
+			if (info->size <= 0 || info->size > sizeof(raw_buf))
+				return -EINVAL;
+
+			hid_sensor_power_state(&rot_state->common_attributes, true);
+
+			ret_type = sensor_hub_input_attr_read_values(hsdev,
+								     hsdev->usage,
+								     usage_id,
+								     info->report_id,
+								     SENSOR_HUB_SYNC,
+								     info->size,
+								     (u8 *)&raw_buf);
+
+			hid_sensor_power_state(&rot_state->common_attributes, false);
+
+			if (ret_type < 0)
+				return ret_type;
+
+			switch (info->size) {
+			case sizeof(raw_buf.val16):
+				for (i = 0; i < ARRAY_SIZE(raw_buf.val16); i++)
+					vals[i] = raw_buf.val16[i];
+				break;
+			case sizeof(raw_buf.val32):
+				for (i = 0; i < ARRAY_SIZE(raw_buf.val32); i++)
+					vals[i] = raw_buf.val32[i];
+				break;
+			default:
+				return -EINVAL;
+			}
+
 			ret_type = IIO_VAL_INT_MULTIPLE;
 			*val_len =  4;
 		} else
-- 
2.43.0

Re: [PATCH 1/2] HID: sensor-hub: Add sensor_hub_input_attr_read_values() for multi-byte reads

[sashiko-bot]

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

New issues:
- [Critical] Missing spinlock acquisition in the timeout/abort path of `sensor_hub_input_attr_read_values` leads to race conditions, use-after-free, and stack memory corruption.

Pre-existing issues:
- [High] Reading past the end of short HID reports exposes uninitialized kernel heap memory.
- [High] Repeated field matches in `sensor_hub_raw_event()` cause uncontrolled `kmemdup()` allocations, leaking memory.
--

commit b3186306e6632f94df0275d5e172210ae3044b54
Author: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>

HID: sensor-hub: Add sensor_hub_input_attr_read_values() for multi-byte reads

This commit introduces sensor_hub_input_attr_read_values() to allow
multi-byte reads for sensors reporting data larger than 32 bits. It
uses a caller-provided buffer to accumulate incoming data in
sensor_hub_raw_event() until the buffer is full.

> diff --git a/drivers/hid/hid-sensor-hub.c b/drivers/hid/hid-sensor-hub.c
> --- a/drivers/hid/hid-sensor-hub.c
> +++ b/drivers/hid/hid-sensor-hub.c

[ ... ]

> +	if (flag == SENSOR_HUB_SYNC) {
> +		cycles = wait_for_completion_interruptible_timeout(&hsdev->pending.ready,
> +								   HZ * 5);
> +		if (cycles == 0)
> +			ret = -ETIMEDOUT;
> +		else if (cycles < 0)
> +			ret = cycles;
> +
> +		hsdev->pending.status = false;

[Severity: Critical]
Does setting this status flag without acquiring the data->lock introduce a
race condition?

If wait_for_completion_interruptible_timeout() times out or is interrupted,
this locklessly sets hsdev->pending.status = false and returns. Concurrently,
the interrupt handler sensor_hub_raw_event() might have already observed
status as true and could be mid-copy into the caller-provided buffer.

Returning early would invalidate the caller-provided buffer, and the
interrupt handler might subsequently overwrite the destroyed stack frame.

> +	}
> +	mutex_unlock(hsdev->mutex_ptr);
> +
> +	return ret;
> +}

[ ... ]

> @@ -518,12 +568,27 @@ static int sensor_hub_raw_event(struct hid_device *hdev,
>  					      hsdev->pending.attr_usage_id ==
>  					      report->field[i]->logical)) {
>  			hid_dbg(hdev, "data was pending ...\n");
> -			hsdev->pending.raw_data = kmemdup(ptr, sz, GFP_ATOMIC);
> -			if (hsdev->pending.raw_data)
> -				hsdev->pending.raw_size = sz;
> -			else
> -				hsdev->pending.raw_size = 0;
> -			complete(&hsdev->pending.ready);
> +			if (hsdev->pending.max_raw_size) {
> +				if (hsdev->pending.index < hsdev->pending.max_raw_size) {
> +					avail = hsdev->pending.max_raw_size - hsdev->pending.index;
> +					copy_size = clamp(sz, 0U, avail);
> +
> +					memcpy(hsdev->pending.raw_data + hsdev->pending.index,
> +					       ptr, copy_size);

[Severity: Critical]
As mentioned above, if sensor_hub_input_attr_read_values() aborts early,
could this memcpy() write into an invalidated stack buffer?

> +					hsdev->pending.index += copy_size;
> +					if (hsdev->pending.index >= hsdev->pending.max_raw_size) {
> +						hsdev->pending.raw_size = hsdev->pending.index;
> +						complete(&hsdev->pending.ready);
> +					}
> +				}
> +			} else {
> +				hsdev->pending.raw_data = kmemdup(ptr, sz, GFP_ATOMIC);

[Severity: High]
This is a pre-existing issue, but does this allocation leak memory if a
crafted HID report contains multiple duplicate fields matching the
requested usage?

If the device rapidly spams events while a reader is waiting, each match
could trigger a new kmemdup, overwriting the hsdev->pending.raw_data
pointer and permanently leaking the previously allocated blocks.

> +				if (hsdev->pending.raw_data)
> +					hsdev->pending.raw_size = sz;
> +				else
> +					hsdev->pending.raw_size = 0;
> +				complete(&hsdev->pending.ready);
> +			}
>  		}
>  		if (callback->capture_sample) {
>  			if (report->field[i]->logical)

[Severity: High]
This is a pre-existing issue, but does the loop advancing ptr expose
uninitialized heap memory on short reports?

Looking at the loop in sensor_hub_raw_event():

	for (i = 0; i < report->maxfield; ++i) {
		...
		sz = (report->field[i]->report_size *
					report->field[i]->report_count)/8;
		...
		ptr += sz;
	}

The pointer is unconditionally advanced by the expected sizes defined in
the report descriptor. Since sensor_hub_raw_event() ignores the actual
size argument representing the received packet length, a maliciously
short packet could cause the driver to read past the initialized packet
bounds.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260610082911.157232-1-lixu.zhang@intel.com?part=1

Re: [PATCH 2/2] iio: hid-sensor-rotation: Fix stale or zero output when reading raw values

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Critical] Stack Use-After-Free / Memory Corruption via synchronous HID read
- [High] PM Runtime Usage Counter Underflow due to missing error check
--

commit 4651e39f1dd649401d6eaaef3b3ae792bc120ba0
Author: Zhang Lixu <lixu.zhang@intel.com>

iio: hid-sensor-rotation: Fix stale or zero output when reading raw values

This commit fixes stale or zero output in dev_rot_read_raw by explicitly
powering up the sensor and issuing a synchronous GET_REPORT to receive
the full quaternion data into a local buffer.

> diff --git a/drivers/iio/orientation/hid-sensor-rotation.c b/drivers/iio/orientation/hid-sensor-rotation.c
> index 4a11e45550991..1c6f02374f3ce 100644
> --- a/drivers/iio/orientation/hid-sensor-rotation.c
> +++ b/drivers/iio/orientation/hid-sensor-rotation.c
> @@ -86,6 +86,13 @@ static int dev_rot_read_raw(struct iio_dev *indio_dev,
>  				long mask)
>  {
>  	struct dev_rot_state *rot_state = iio_priv(indio_dev);
> +	struct hid_sensor_hub_device *hsdev = rot_state->common_attributes.hsdev;
> +	struct hid_sensor_hub_attribute_info *info = &rot_state->quaternion;
> +	u32 usage_id = HID_USAGE_SENSOR_ORIENT_QUATERNION;
> +	union {
> +		s16 val16[4];
> +		s32 val32[4];
> +	} raw_buf;
>  	int ret_type;
>  	int i;
>  
> @@ -95,8 +102,37 @@ static int dev_rot_read_raw(struct iio_dev *indio_dev,
>  	switch (mask) {
>  	case IIO_CHAN_INFO_RAW:
>  		if (size >= 4) {
> -			for (i = 0; i < 4; ++i)
> -				vals[i] = rot_state->scan.sampled_vals[i];
> +			if (info->size <= 0 || info->size > sizeof(raw_buf))
> +				return -EINVAL;
> +
> +			hid_sensor_power_state(&rot_state->common_attributes, true);

[Severity: High]
Should the return value of this power-on call be checked?

If hid_sensor_power_state fails, pm_runtime_resume_and_get cleans up its
PM usage count increment, leaving the net usage count unchanged.

Because this error is ignored, the code unconditionally calls
hid_sensor_power_state false later on, which invokes
pm_runtime_put_autosuspend and drops the usage counter by 1.

Does this cause the PM usage counter to underflow when power-on fails?

> +
> +			ret_type = sensor_hub_input_attr_read_values(hsdev,
> +								     hsdev->usage,
> +								     usage_id,
> +								     info->report_id,
> +								     SENSOR_HUB_SYNC,
> +								     info->size,
> +								     (u8 *)&raw_buf);

[Severity: Critical]
Can this corrupt the stack if the synchronous read is interrupted?

By passing a pointer to the stack-allocated raw_buf into
sensor_hub_input_attr_read_values, the driver relies on the wait
completing successfully.

If the wait is interrupted by a signal sent to the userspace process,
or if it times out:

drivers/hid/hid-sensor-hub.c:sensor_hub_input_attr_read_values() {
    ...
    cycles = wait_for_completion_interruptible_timeout(&hsdev->pending.ready,
                                                       HZ * 5);
    if (cycles == 0)
        ret = -ETIMEDOUT;
    else if (cycles < 0)
        ret = cycles;

    hsdev->pending.status = false;
    ...
}

The function locklessly clears hsdev->pending.status and returns, destroying
the raw_buf stack frame.

However, the concurrent HID interrupt handler sensor_hub_raw_event might
have already evaluated hsdev->pending.status as true under the lock, and
could proceed to memcpy into the now-freed stack pointer.

> +
> +			hid_sensor_power_state(&rot_state->common_attributes, false);
> +
> +			if (ret_type < 0)
> +				return ret_type;

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260610082911.157232-1-lixu.zhang@intel.com?part=2

Re: [PATCH 1/2] HID: sensor-hub: Add sensor_hub_input_attr_read_values() for multi-byte reads

[Andy Shevchenko]

On Wed, Jun 10, 2026 at 04:29:09PM +0800, Zhang Lixu wrote:

> sensor_hub_input_attr_get_raw_value() is limited to returning a single
> 32-bit value, which is insufficient for sensors that report data larger
> than 32 bits, such as a quaternion with four s16 elements.
> 
> Add sensor_hub_input_attr_read_values() that accepts a caller-provided
> buffer and accumulates incoming data until the buffer is full. The two
> paths are distinguished in sensor_hub_raw_event() by pending.max_raw_size
> being non-zero, preserving backward compatibility.

I have reviewed that internally and checked public version which LGTM,
Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 2/2] iio: hid-sensor-rotation: Fix stale or zero output when reading raw values

[Andy Shevchenko]

On Wed, Jun 10, 2026 at 04:29:10PM +0800, Zhang Lixu wrote:
> When reading the raw quaternion attribute (in_rot_quaternion_raw), the
> driver currently returns either all zeros (if the sensor was never enabled)
> or stale data (if the sensor was previously enabled) because it reads from
> the internal buffer without explicitly requesting a new sample from the
> sensor.
> 
> To fix this, power up the sensor, call sensor_hub_input_attr_read_values()
> to issue a synchronous GET_REPORT and receive the full quaternion data
> directly into a local buffer, then decode the four components.

I have reviewed that internally and checked public version which LGTM,
Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>

-- 
With Best Regards,
Andy Shevchenko