Re: [PATCH v2 6/7] iio: orientation: hid-sensor-rotation: use u32 instead of unsigned

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Out-of-bounds read and unaligned access in `dev_rot_capture_sample`
- [High] Use-After-Free (UAF) race condition between device removal and raw event processing
--

commit de9f9c2c99851d2c61cb3fce183da2297778f18f
Author: Sanjay Chitroda <sanjayembeddedse@gmail.com>

iio: orientation: hid-sensor-rotation: use u32 instead of unsigned

Prefer 'u32' instead of bare 'unsigned' for usage_id variable.
This matches expected callback API type and improves code clarity.
No functional change.

> diff --git a/drivers/iio/orientation/hid-sensor-rotation.c b/drivers/iio/orientation/hid-sensor-rotation.c
> index 4a11e45550991..20563d8efaf6c 100644
> --- a/drivers/iio/orientation/hid-sensor-rotation.c
> +++ b/drivers/iio/orientation/hid-sensor-rotation.c

[ ... ]

> @@ -209,7 +209,7 @@ static int dev_rot_proc_event(struct hid_sensor_hub_device *hsdev,
>  
>  /* Capture samples in local storage */
>  static int dev_rot_capture_sample(struct hid_sensor_hub_device *hsdev,
> -				unsigned usage_id,
> +				u32 usage_id,
>  				size_t raw_len, char *raw_data,
>  				void *priv)
>  {

[Severity: High]
This isn't a bug introduced by this patch, but does dev_rot_capture_sample()
blindly trust raw_len and raw_data derived from the HID device's report
descriptors?

If a malicious or faulty USB device provides a descriptor with a small report
size (e.g., 4 bytes), raw_len / 4 == sizeof(s16) evaluates to false. This
leads the code to execute:

    memcpy(&rot_state->scan.sampled_vals, raw_data,
           sizeof(rot_state->scan.sampled_vals));

Can this read out-of-bounds heap memory which is then pushed to IIO buffers
and leaked to userspace?

Furthermore, for HID_USAGE_SENSOR_TIME_TIMESTAMP, the code directly casts
and dereferences raw_data as a 64-bit integer:

    rot_state->timestamp = hid_sensor_convert_timestamp(
                            &rot_state->common_attributes, *(s64 *)raw_data);

As HID reports are packed, raw_data can be unaligned. Could this cause a
kernel panic (unaligned access exception) on architectures with strict
alignment requirements?


[Severity: High]
This is also a pre-existing issue, but is there a use-after-free race
condition between device removal and raw event processing?

During removal, hid_dev_rot_remove() calls sensor_hub_remove_callback(),
which deletes the callback from the list using dyn_callback_lock but does
not synchronize with active or concurrent readers:

drivers/hid/hid-sensor-hub.c:sensor_hub_remove_callback() {
    ...
    list_del(&callback->list);
    kfree(callback);
    ...
}

Meanwhile, in sensor_hub_raw_event(), sensor_hub_get_callback() fetches the
callback pointer and drops the lock. The function then continues to invoke
callback->capture_sample() without any locks or reference counting:

drivers/hid/hid-sensor-hub.c:sensor_hub_raw_event() {
    ...
    callback->capture_sample(hsdev, report->field[i]->logical, sz, ptr,
                             callback->pdev);
    ...
}

If device removal completes and devres frees indio_dev and dev_rot_state
before sensor_hub_raw_event() finishes execution on another CPU, will the
event handler dereference a freed function pointer or access the freed driver
state structure?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260610-6-june-hid-iio-correct-usage-id-v2-0-c3c5f0720493@gmail.com?part=6