* Re: [Bug 221077] New: [iio] [hid-sensor-rotation] Memory corruption due to alignment mismatch in scan buffer
2026-02-11 5:12 [Bug 221077] New: [iio] [hid-sensor-rotation] Memory corruption due to alignment mismatch in scan buffer bugzilla-daemon
@ 2026-02-14 19:24 ` Jonathan Cameron
2026-02-14 20:29 ` David Lechner
2026-02-14 19:24 ` [Bug 221077] " bugzilla-daemon
` (5 subsequent siblings)
6 siblings, 1 reply; 10+ messages in thread
From: Jonathan Cameron @ 2026-02-14 19:24 UTC (permalink / raw)
To: bugzilla-daemon; +Cc: linux-iio, David Lechner
On Wed, 11 Feb 2026 05:12:36 +0000
bugzilla-daemon@kernel.org wrote:
> 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**.
I agree it's 24 bytes, but worth being clear it has explicit alignment to 8
bytes. Without that on some platforms it would be 4 byte aligned only.
>
> 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`.
Ah. That is indeed a corner case and the ABI is IIRC not well documented
around that, let alone what we do in the kernel. We probably need to tighten that
up as well as fixing this. What userspace were you using to test this?
>
> 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);
Agreed that fix is correct.
However, we definitely also need to add some documentation on why it is there.
It's not actually an alignment force on sampled_vals, but rather on the timestamp.
We can't just mark the timestamp as then it will have two aligned markings on x86_32.
David, given we both missed this when 'tidying' this up wrongly what do you think
would make this clearest?
Thanks for reporting this with such a thorough and detailed investigation!
Makes our lives a lot easier.
Jonathan
> aligned_s64 timestamp;
> } scan;
> ```
>
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [Bug 221077] New: [iio] [hid-sensor-rotation] Memory corruption due to alignment mismatch in scan buffer
2026-02-14 19:24 ` Jonathan Cameron
@ 2026-02-14 20:29 ` David Lechner
2026-02-15 16:23 ` Jonathan Cameron
0 siblings, 1 reply; 10+ messages in thread
From: David Lechner @ 2026-02-14 20:29 UTC (permalink / raw)
To: Jonathan Cameron, bugzilla-daemon; +Cc: linux-iio
On 2/14/26 1:24 PM, Jonathan Cameron wrote:
> On Wed, 11 Feb 2026 05:12:36 +0000
> bugzilla-daemon@kernel.org wrote:
>
>> 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**.
>
> I agree it's 24 bytes, but worth being clear it has explicit alignment to 8
> bytes. Without that on some platforms it would be 4 byte aligned only.
>
>>
>> 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`.
>
> Ah. That is indeed a corner case and the ABI is IIRC not well documented
> around that, let alone what we do in the kernel. We probably need to tighten that
> up as well as fixing this. What userspace were you using to test this?
I didn't even know we had IIO_MOD_QUATERNION as a special data type until
Francesco's recent patches. :-o
>
>
>>
>> 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);
> Agreed that fix is correct.
> However, we definitely also need to add some documentation on why it is there.
>
> It's not actually an alignment force on sampled_vals, but rather on the timestamp.
> We can't just mark the timestamp as then it will have two aligned markings on x86_32.
>
> David, given we both missed this when 'tidying' this up wrongly what do you think
> would make this clearest?
I think it would make sense to have an IIO_DECLARE_REPEAT_ELEMENT()
similar to IIO_DECLARE_BUFFER_WITH_TS().
#define IIO_DECLARE_REPEAT_ELEMENT(type, name, count) \
type name[count] __aligned(sizeof(type) * count)
And it would be used like:
struct {
IIO_DECLARE_REPEAT_ELEMENT(s32, sampled_vals, 4);
aligned_s64 timestamp;
} scan;
>
>
> Thanks for reporting this with such a thorough and detailed investigation!
> Makes our lives a lot easier.
Should I spin up a fix?
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [Bug 221077] New: [iio] [hid-sensor-rotation] Memory corruption due to alignment mismatch in scan buffer
2026-02-14 20:29 ` David Lechner
@ 2026-02-15 16:23 ` Jonathan Cameron
0 siblings, 0 replies; 10+ messages in thread
From: Jonathan Cameron @ 2026-02-15 16:23 UTC (permalink / raw)
To: David Lechner; +Cc: bugzilla-daemon, linux-iio
On Sat, 14 Feb 2026 14:29:19 -0600
David Lechner <dlechner@baylibre.com> wrote:
> On 2/14/26 1:24 PM, Jonathan Cameron wrote:
> > On Wed, 11 Feb 2026 05:12:36 +0000
> > bugzilla-daemon@kernel.org wrote:
> >
> >> 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**.
> >
> > I agree it's 24 bytes, but worth being clear it has explicit alignment to 8
> > bytes. Without that on some platforms it would be 4 byte aligned only.
> >
> >>
> >> 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`.
> >
> > Ah. That is indeed a corner case and the ABI is IIRC not well documented
> > around that, let alone what we do in the kernel. We probably need to tighten that
> > up as well as fixing this. What userspace were you using to test this?
>
> I didn't even know we had IIO_MOD_QUATERNION as a special data type until
> Francesco's recent patches. :-o
It's not common. Was interestingly something that looked likely to be so
in relatively early IMUs but then faded away -> probably because the algorithms
to get a good estimate are complex and best done on the host CPU or somewhere
off sensor.
>
> >
> >
> >>
> >> 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);
> > Agreed that fix is correct.
> > However, we definitely also need to add some documentation on why it is there.
> >
> > It's not actually an alignment force on sampled_vals, but rather on the timestamp.
> > We can't just mark the timestamp as then it will have two aligned markings on x86_32.
> >
> > David, given we both missed this when 'tidying' this up wrongly what do you think
> > would make this clearest?
>
> I think it would make sense to have an IIO_DECLARE_REPEAT_ELEMENT()
> similar to IIO_DECLARE_BUFFER_WITH_TS().
It will be worth checking if we have more than one user. When quaternion's
turn up there are always other channels. The oddity is that sometimes the fusion
algorithms used produce the data at a different rate to the underlying channel
types (or like in HID's case) don't tell you what that relationship is, data just
shows up tagged with what type it is and we split it to the different IIO devices.
As such they at least sometimes share a buffer with non repeated channels.
>
> #define IIO_DECLARE_REPEAT_ELEMENT(type, name, count) \
> type name[count] __aligned(sizeof(type) * count)
>
> And it would be used like:
>
> struct {
> IIO_DECLARE_REPEAT_ELEMENT(s32, sampled_vals, 4);
> aligned_s64 timestamp;
Isn't our timestamp ending up in the wrong place anyway vs that
definition (and the original one pre bug)? There is a lazy hack in
iio_push_to_buffers_with_ts() that assumes the structure is 8 byte aligned :(
and puts the timestamp in the last 8 bytes but per normal channel packing each
element is naturally aligned so it should be buffer + 16, not buffer + 32 - 8
We still need the extra padding the aligned is bringing because the kfifo when
the configuration matches this layout has 32 byte entries, but the timestamp
should be where it this structure says it is not where it's currently going.
Given how common that iio_push_to_buffers_with_timestamp()/ts() is
(and fiddly computing the timestamp position is) I'd suggest in this case we
open code iio_push_to_buffers_with_ts(). We can swing by after this bug is
fixed and try to work out if there is a cleaner solution (probably compute as
part of the demux setup and stash it for reuse).
+ Change to _ts() version as it would have screamed about this without the
corruption see by the reporter as the buffer would have been smaller than the
kfifo was expecting.
This is a userspace ABI bug fix / change. So we need to know what userspace is making
use of quaternion channels and fix it as well.
Jonathan
> } scan;
>
> >
> >
> > Thanks for reporting this with such a thorough and detailed investigation!
> > Makes our lives a lot easier.
>
> Should I spin up a fix?
>
If you have time to take on also landing the timestamp in the right place then
that would be great. I'm a touch snowed under for the next week or two.
Jonathan
^ permalink raw reply [flat|nested] 10+ messages in thread
* [Bug 221077] [iio] [hid-sensor-rotation] Memory corruption due to alignment mismatch in scan buffer
2026-02-11 5:12 [Bug 221077] New: [iio] [hid-sensor-rotation] Memory corruption due to alignment mismatch in scan buffer bugzilla-daemon
2026-02-14 19:24 ` Jonathan Cameron
@ 2026-02-14 19:24 ` bugzilla-daemon
2026-02-14 20:29 ` bugzilla-daemon
` (4 subsequent siblings)
6 siblings, 0 replies; 10+ messages in thread
From: bugzilla-daemon @ 2026-02-14 19:24 UTC (permalink / raw)
To: linux-iio
https://bugzilla.kernel.org/show_bug.cgi?id=221077
--- Comment #1 from Jonathan Cameron (jic23@kernel.org) ---
On Wed, 11 Feb 2026 05:12:36 +0000
bugzilla-daemon@kernel.org wrote:
> 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**.
I agree it's 24 bytes, but worth being clear it has explicit alignment to 8
bytes. Without that on some platforms it would be 4 byte aligned only.
>
> 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`.
Ah. That is indeed a corner case and the ABI is IIRC not well documented
around that, let alone what we do in the kernel. We probably need to tighten
that
up as well as fixing this. What userspace were you using to test this?
>
> 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);
Agreed that fix is correct.
However, we definitely also need to add some documentation on why it is there.
It's not actually an alignment force on sampled_vals, but rather on the
timestamp.
We can't just mark the timestamp as then it will have two aligned markings on
x86_32.
David, given we both missed this when 'tidying' this up wrongly what do you
think
would make this clearest?
Thanks for reporting this with such a thorough and detailed investigation!
Makes our lives a lot easier.
Jonathan
> 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.
^ permalink raw reply [flat|nested] 10+ messages in thread* [Bug 221077] [iio] [hid-sensor-rotation] Memory corruption due to alignment mismatch in scan buffer
2026-02-11 5:12 [Bug 221077] New: [iio] [hid-sensor-rotation] Memory corruption due to alignment mismatch in scan buffer bugzilla-daemon
2026-02-14 19:24 ` Jonathan Cameron
2026-02-14 19:24 ` [Bug 221077] " bugzilla-daemon
@ 2026-02-14 20:29 ` bugzilla-daemon
2026-02-15 16:23 ` bugzilla-daemon
` (3 subsequent siblings)
6 siblings, 0 replies; 10+ messages in thread
From: bugzilla-daemon @ 2026-02-14 20:29 UTC (permalink / raw)
To: linux-iio
https://bugzilla.kernel.org/show_bug.cgi?id=221077
--- Comment #2 from dlechner@baylibre.com ---
On 2/14/26 1:24 PM, Jonathan Cameron wrote:
> On Wed, 11 Feb 2026 05:12:36 +0000
> bugzilla-daemon@kernel.org wrote:
>
>> 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**.
>
> I agree it's 24 bytes, but worth being clear it has explicit alignment to 8
> bytes. Without that on some platforms it would be 4 byte aligned only.
>
>>
>> 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`.
>
> Ah. That is indeed a corner case and the ABI is IIRC not well documented
> around that, let alone what we do in the kernel. We probably need to tighten
> that
> up as well as fixing this. What userspace were you using to test this?
I didn't even know we had IIO_MOD_QUATERNION as a special data type until
Francesco's recent patches. :-o
>
>
>>
>> 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);
> Agreed that fix is correct.
> However, we definitely also need to add some documentation on why it is
> there.
>
> It's not actually an alignment force on sampled_vals, but rather on the
> timestamp.
> We can't just mark the timestamp as then it will have two aligned markings on
> x86_32.
>
> David, given we both missed this when 'tidying' this up wrongly what do you
> think
> would make this clearest?
I think it would make sense to have an IIO_DECLARE_REPEAT_ELEMENT()
similar to IIO_DECLARE_BUFFER_WITH_TS().
#define IIO_DECLARE_REPEAT_ELEMENT(type, name, count) \
type name[count] __aligned(sizeof(type) * count)
And it would be used like:
struct {
IIO_DECLARE_REPEAT_ELEMENT(s32, sampled_vals, 4);
aligned_s64 timestamp;
} scan;
>
>
> Thanks for reporting this with such a thorough and detailed investigation!
> Makes our lives a lot easier.
Should I spin up a fix?
--
You may reply to this email to add a comment.
You are receiving this mail because:
You are watching the assignee of the bug.
^ permalink raw reply [flat|nested] 10+ messages in thread* [Bug 221077] [iio] [hid-sensor-rotation] Memory corruption due to alignment mismatch in scan buffer
2026-02-11 5:12 [Bug 221077] New: [iio] [hid-sensor-rotation] Memory corruption due to alignment mismatch in scan buffer bugzilla-daemon
` (2 preceding siblings ...)
2026-02-14 20:29 ` bugzilla-daemon
@ 2026-02-15 16:23 ` bugzilla-daemon
2026-02-16 6:58 ` bugzilla-daemon
` (2 subsequent siblings)
6 siblings, 0 replies; 10+ messages in thread
From: bugzilla-daemon @ 2026-02-15 16:23 UTC (permalink / raw)
To: linux-iio
https://bugzilla.kernel.org/show_bug.cgi?id=221077
--- Comment #3 from Jonathan Cameron (jic23@kernel.org) ---
On Sat, 14 Feb 2026 14:29:19 -0600
David Lechner <dlechner@baylibre.com> wrote:
> On 2/14/26 1:24 PM, Jonathan Cameron wrote:
> > On Wed, 11 Feb 2026 05:12:36 +0000
> > bugzilla-daemon@kernel.org wrote:
> >
> >> 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**.
> >
> > I agree it's 24 bytes, but worth being clear it has explicit alignment to 8
> > bytes. Without that on some platforms it would be 4 byte aligned only.
> >
> >>
> >> 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`.
> >
> > Ah. That is indeed a corner case and the ABI is IIRC not well documented
> > around that, let alone what we do in the kernel. We probably need to
> tighten that
> > up as well as fixing this. What userspace were you using to test this?
>
> I didn't even know we had IIO_MOD_QUATERNION as a special data type until
> Francesco's recent patches. :-o
It's not common. Was interestingly something that looked likely to be so
in relatively early IMUs but then faded away -> probably because the algorithms
to get a good estimate are complex and best done on the host CPU or somewhere
off sensor.
>
> >
> >
> >>
> >> 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);
> > Agreed that fix is correct.
> > However, we definitely also need to add some documentation on why it is
> there.
> >
> > It's not actually an alignment force on sampled_vals, but rather on the
> timestamp.
> > We can't just mark the timestamp as then it will have two aligned markings
> on x86_32.
> >
> > David, given we both missed this when 'tidying' this up wrongly what do you
> think
> > would make this clearest?
>
> I think it would make sense to have an IIO_DECLARE_REPEAT_ELEMENT()
> similar to IIO_DECLARE_BUFFER_WITH_TS().
It will be worth checking if we have more than one user. When quaternion's
turn up there are always other channels. The oddity is that sometimes the
fusion
algorithms used produce the data at a different rate to the underlying channel
types (or like in HID's case) don't tell you what that relationship is, data
just
shows up tagged with what type it is and we split it to the different IIO
devices.
As such they at least sometimes share a buffer with non repeated channels.
>
> #define IIO_DECLARE_REPEAT_ELEMENT(type, name, count) \
> type name[count] __aligned(sizeof(type) * count)
>
> And it would be used like:
>
> struct {
> IIO_DECLARE_REPEAT_ELEMENT(s32, sampled_vals, 4);
> aligned_s64 timestamp;
Isn't our timestamp ending up in the wrong place anyway vs that
definition (and the original one pre bug)? There is a lazy hack in
iio_push_to_buffers_with_ts() that assumes the structure is 8 byte aligned :(
and puts the timestamp in the last 8 bytes but per normal channel packing each
element is naturally aligned so it should be buffer + 16, not buffer + 32 - 8
We still need the extra padding the aligned is bringing because the kfifo when
the configuration matches this layout has 32 byte entries, but the timestamp
should be where it this structure says it is not where it's currently going.
Given how common that iio_push_to_buffers_with_timestamp()/ts() is
(and fiddly computing the timestamp position is) I'd suggest in this case we
open code iio_push_to_buffers_with_ts(). We can swing by after this bug is
fixed and try to work out if there is a cleaner solution (probably compute as
part of the demux setup and stash it for reuse).
+ Change to _ts() version as it would have screamed about this without the
corruption see by the reporter as the buffer would have been smaller than the
kfifo was expecting.
This is a userspace ABI bug fix / change. So we need to know what userspace is
making
use of quaternion channels and fix it as well.
Jonathan
> } scan;
>
> >
> >
> > Thanks for reporting this with such a thorough and detailed investigation!
> > Makes our lives a lot easier.
>
> Should I spin up a fix?
>
If you have time to take on also landing the timestamp in the right place then
that would be great. I'm a touch snowed under for the next week or two.
Jonathan
--
You may reply to this email to add a comment.
You are receiving this mail because:
You are watching the assignee of the bug.
^ permalink raw reply [flat|nested] 10+ messages in thread* [Bug 221077] [iio] [hid-sensor-rotation] Memory corruption due to alignment mismatch in scan buffer
2026-02-11 5:12 [Bug 221077] New: [iio] [hid-sensor-rotation] Memory corruption due to alignment mismatch in scan buffer bugzilla-daemon
` (3 preceding siblings ...)
2026-02-15 16:23 ` bugzilla-daemon
@ 2026-02-16 6:58 ` bugzilla-daemon
2026-02-16 7:03 ` bugzilla-daemon
2026-02-24 2:41 ` bugzilla-daemon
6 siblings, 0 replies; 10+ messages in thread
From: bugzilla-daemon @ 2026-02-16 6:58 UTC (permalink / raw)
To: linux-iio
https://bugzilla.kernel.org/show_bug.cgi?id=221077
--- Comment #4 from Lixu Zhang (lixu.zhang@intel.com) ---
> What userspace were you using to test this?
An internal test tool similar to iio_generic_buffer.
Lixu
--
You may reply to this email to add a comment.
You are receiving this mail because:
You are watching the assignee of the bug.
^ permalink raw reply [flat|nested] 10+ messages in thread* [Bug 221077] [iio] [hid-sensor-rotation] Memory corruption due to alignment mismatch in scan buffer
2026-02-11 5:12 [Bug 221077] New: [iio] [hid-sensor-rotation] Memory corruption due to alignment mismatch in scan buffer bugzilla-daemon
` (4 preceding siblings ...)
2026-02-16 6:58 ` bugzilla-daemon
@ 2026-02-16 7:03 ` bugzilla-daemon
2026-02-24 2:41 ` bugzilla-daemon
6 siblings, 0 replies; 10+ messages in thread
From: bugzilla-daemon @ 2026-02-16 7:03 UTC (permalink / raw)
To: linux-iio
https://bugzilla.kernel.org/show_bug.cgi?id=221077
--- Comment #5 from Lixu Zhang (lixu.zhang@intel.com) ---
Hi David,
Thank you for the patch! I'm currently on Chinese New Year holiday and
attempted to remotely access my test machine to verify the fix, but
unfortunately couldn't establish a connection.
I will test the patch as soon as I'm back in the office and provide feedback.
Best regards,
Lixu
--
You may reply to this email to add a comment.
You are receiving this mail because:
You are watching the assignee of the bug.
^ permalink raw reply [flat|nested] 10+ messages in thread* [Bug 221077] [iio] [hid-sensor-rotation] Memory corruption due to alignment mismatch in scan buffer
2026-02-11 5:12 [Bug 221077] New: [iio] [hid-sensor-rotation] Memory corruption due to alignment mismatch in scan buffer bugzilla-daemon
` (5 preceding siblings ...)
2026-02-16 7:03 ` bugzilla-daemon
@ 2026-02-24 2:41 ` bugzilla-daemon
6 siblings, 0 replies; 10+ messages in thread
From: bugzilla-daemon @ 2026-02-24 2:41 UTC (permalink / raw)
To: linux-iio
https://bugzilla.kernel.org/show_bug.cgi?id=221077
Lixu Zhang (lixu.zhang@intel.com) changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|NEW |RESOLVED
Resolution|--- |PATCH_ALREADY_AVAILABLE
--- Comment #6 from Lixu Zhang (lixu.zhang@intel.com) ---
With `[PATCH 0/2] iio: orientation: hid-sensor-rotation: fix quaternion
alignment`, the bug is fixed. Thanks.
Lixu
--
You may reply to this email to add a comment.
You are receiving this mail because:
You are watching the assignee of the bug.
^ permalink raw reply [flat|nested] 10+ messages in thread