[Bug 221077] New: [iio] [hid-sensor-rotation] Memory corruption due to alignment mismatch in scan buffer

[bugzilla-daemon@kernel.org]

https://bugzilla.kernel.org/show_bug.cgi?id=221077

            Bug ID: 221077
           Summary: [iio] [hid-sensor-rotation] Memory corruption due to
                    alignment mismatch in scan buffer
           Product: Drivers
           Version: 2.5
          Hardware: All
                OS: Linux
            Status: NEW
          Severity: normal
          Priority: P3
         Component: IIO
          Assignee: drivers_iio@kernel-bugs.kernel.org
          Reporter: lixu.zhang@intel.com
        Regression: Yes
           Bisected b31a74075cb4ca2bb202a2e17d133ef3c9ee891f
         commit-id:

### Problem
In `drivers/iio/orientation/hid-sensor-rotation.c`, the `scale_pre_decml` and
`scale_post_decml` fields in `struct dev_rot_state` get corrupted after the
first read from the device. This issue results in invalid scale values being
reported to userspace.

### Root Cause Analysis
The issue is caused by a size mismatch between what the IIO core expects for
the scan buffer and the actual size of the driver's scan structure.

1.  **Driver Structure**: The `scan` struct in `dev_rot_state` consists of a
quaternion (4 * s32 = 16 bytes) and a timestamp (8 bytes).
    ```c
    struct {
            s32 sampled_vals[4];
            aligned_s64 timestamp;
    } scan;
    ```
    Without explicit alignment, this structure is packed to **24 bytes**.

2.  **IIO Core Expectation**: The `iio_compute_scan_bytes` function calculates
the buffer size required. It aligns the total size to the size of the *largest
element* in the scan.
    - The quaternion channel is treated as a single 16-byte element.
    - Therefore, the core aligns the total size to 16 bytes: `ALIGN(24, 16) =
32 bytes`.

3.  **Corruption**: When `iio_push_to_buffers_with_timestamp()` is called:
    - It assumes a 32-byte buffer.
    - It writes the timestamp at the end of the aligned buffer (offset 24).
    - Since the driver allocated only 24 bytes for `scan`, the write at offset
24 overwrites the adjacent `scale_pre_decml` field in `struct dev_rot_state`.

### Evidence (Ftrace)
I verified this by tracing the return values of `iio_storage_bytes_for_si` and
`iio_compute_scan_bytes` using kretprobes:

```log
$ cat /sys/kernel/tracing/trace_pipe
r_store_bytes: (iio_compute_scan_bytes+0x30/0xd0 [industrialio] <-
iio_storage_bytes_for_si) arg1=0x10
r_store_bytes: (iio_compute_scan_bytes+0xa1/0xd0 [industrialio] <-
iio_storage_bytes_for_si) arg1=0x8
r_calc_bytes: (__iio_update_buffers+0x99d/0xd40 [industrialio] <-
iio_compute_scan_bytes) arg1=0x20
```

The trace confirms:
- Largest element = 16 bytes.
- Total raw size = 16 (quat) + 8 (ts) = 24 bytes.
- Final aligned size = ALIGN(24, 16) = 32 bytes.

The memory layout mismatch is:
- IIO Core needs: 32 bytes
- Driver struct has: 24 bytes

### Regression
This issue was introduced by commit `b31a74075cb4 ("iio: orientation:
hid-sensor-rotation: remove unnecessary alignment")`, which removed the
`__aligned(16)` attribute that previously ensured the struct was padded to 32
bytes.

### Proposed Fix
Revert the removal of `__aligned(16)` to ensure `struct dev_rot_state` has the
correct padding to match the IIO core's expectations.

```c
struct {
        s32 sampled_vals[4] __aligned(16);
        aligned_s64 timestamp;
} scan;
```

-- 
You may reply to this email to add a comment.

You are receiving this mail because:
You are watching the assignee of the bug.