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

[sashiko-bot@kernel.org]

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