linux-kernel - Re: [PATCH v5 1/3] iio: imu: inv_icm42600: move structure DMA buffers at the end

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID:
 <FR3P281MB175703F651131703019D7295CE78A@FR3P281MB1757.DEUP281.PROD.OUTLOOK.COM>
Date: Tue, 24 Jun 2025 08:43:37 +0000
From: Jean-Baptiste Maneyrol <Jean-Baptiste.Maneyrol@....com>
To: David Lechner <dlechner@...libre.com>
CC: Jonathan Cameron <jic23@...nel.org>, Lars-Peter Clausen <lars@...afoo.de>,
        Nuno Sá <nuno.sa@...log.com>,
        Andy Shevchenko
	<andy@...nel.org>,
        "linux-iio@...r.kernel.org" <linux-iio@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v5 1/3] iio: imu: inv_icm42600: move structure DMA buffers
 at the end

>
>________________________________________
>From: David Lechner <dlechner@...libre.com>
>Sent: Tuesday, June 24, 2025 00:13
>To: Jean-Baptiste Maneyrol <Jean-Baptiste.Maneyrol@....com>
>Cc: Jonathan Cameron <jic23@...nel.org>; Lars-Peter Clausen <lars@...afoo.de>; Nuno Sá <nuno.sa@...log.com>; Andy Shevchenko <andy@...nel.org>; linux-iio@...r.kernel.org <linux-iio@...r.kernel.org>; linux-kernel@...r.kernel.org <linux-kernel@...r.kernel.org>
>Subject: Re: [PATCH v5 1/3] iio: imu: inv_icm42600: move structure DMA buffers at the end
> 
>This Message Is From an External Sender
>This message came from outside your organization.
> 
>On Mon, Jun 23, 2025 at 6:56 AM Jean-Baptiste Maneyrol via B4 Relay
><devnull+jean-baptiste.maneyrol.tdk.com@...nel.org> wrote:
>>
>> From: Jean-Baptiste Maneyrol <jean-baptiste.maneyrol@....com>
>>
>> Move DMA buffers at the end of the structure to avoid overflow
>> bugs with unexpected effect.
>
>If there is an overflow bug, we should fix that rather than hiding it.
>
>If I misunderstood the problem and timestamp and fifo should not be in
>the DMA aligned area and there is a problem with DMA cache writing
>over them, then I think we should reword the commit message.
>
Hello David,

this change was asked by Jonathan so that potential DMA overflow would be more
easily caught by writing outside of the structure rather than writing inside
and do unexpected behavior.

>>
>> struct inv_icm42600_fifo has a DMA buffer at the end.
>>
>> Signed-off-by: Jean-Baptiste Maneyrol <jean-baptiste.maneyrol@....com>
>> ---
>>  drivers/iio/imu/inv_icm42600/inv_icm42600.h | 8 ++++----
>>  1 file changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/iio/imu/inv_icm42600/inv_icm42600.h b/drivers/iio/imu/inv_icm42600/inv_icm42600.h
>> index 55ed1ddaa8cb5dd410d17db3866fa0f22f18e9d2..9b2cce172670c5513f18d5979a5ff563e9af4cb3 100644
>> --- a/drivers/iio/imu/inv_icm42600/inv_icm42600.h
>> +++ b/drivers/iio/imu/inv_icm42600/inv_icm42600.h
>> @@ -148,9 +148,9 @@ struct inv_icm42600_suspended {
>>   *  @suspended:                suspended sensors configuration.
>>   *  @indio_gyro:       gyroscope IIO device.
>>   *  @indio_accel:      accelerometer IIO device.
>> - *  @buffer:           data transfer buffer aligned for DMA.
>> - *  @fifo:             FIFO management structure.
>>   *  @timestamp:                interrupt timestamps.
>> + *  @fifo:             FIFO management structure.
>> + *  @buffer:           data transfer buffer aligned for DMA.
>>   */
>>  struct inv_icm42600_state {
>>         struct mutex lock;
>> @@ -164,12 +164,12 @@ struct inv_icm42600_state {
>>         struct inv_icm42600_suspended suspended;
>>         struct iio_dev *indio_gyro;
>>         struct iio_dev *indio_accel;
>> -       u8 buffer[2] __aligned(IIO_DMA_MINALIGN);
>> -       struct inv_icm42600_fifo fifo;
>>         struct {
>>                 s64 gyro;
>>                 s64 accel;
>>         } timestamp;
>> +       struct inv_icm42600_fifo fifo;
>
>I didn't look at how the drivers use timestamp and fifo, but if they
>are passed as a buffer to SPI, then they need to stay in the DMA
>aligned area of the struct.

struct inv_icm42600_fifo has a buffer at its end that is passed to SPI.
Same things for buffer below. That's why both buffers are DMA
aligned.

>
>> +       u8 buffer[2] __aligned(IIO_DMA_MINALIGN);
>>  };
>>
>>
>>
>> --
>> 2.49.0
>>
>>
>

Thanks,
JB