linux-kernel - Re: [PATCH 00/10] iio: prefer aligned

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <20250421121752.5a7a178e@jic23-huawei>
Date: Mon, 21 Apr 2025 12:17:52 +0100
From: Jonathan Cameron <jic23@...nel.org>
To: David Lechner <dlechner@...libre.com>
Cc: Nuno Sá <nuno.sa@...log.com>, Andy Shevchenko
 <andy@...nel.org>, Eugen Hristev <eugen.hristev@...aro.org>, Nicolas Ferre
 <nicolas.ferre@...rochip.com>, Alexandre Belloni
 <alexandre.belloni@...tlin.com>, Claudiu Beznea <claudiu.beznea@...on.dev>,
 Andreas Klinger <ak@...klinger.de>, Shawn Guo <shawnguo@...nel.org>, Sascha
 Hauer <s.hauer@...gutronix.de>, Pengutronix Kernel Team
 <kernel@...gutronix.de>, Fabio Estevam <festevam@...il.com>, Maxime
 Coquelin <mcoquelin.stm32@...il.com>, Alexandre Torgue
 <alexandre.torgue@...s.st.com>, linux-iio@...r.kernel.org,
 linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
 imx@...ts.linux.dev, linux-stm32@...md-mailman.stormreply.com
Subject: Re: [PATCH 00/10] iio: prefer aligned_s64 timestamp (round 1)

On Fri, 18 Apr 2025 18:05:42 -0500
David Lechner <dlechner@...libre.com> wrote:

> On 4/18/25 2:58 PM, David Lechner wrote:
> > While reviewing the recent conversion to iio_push_to_buffers_with_ts(),
> > I found it very time-consuming to check the correctness of the buffers
> > passed to that function when they used an array with extra room at the
> > end for a timestamp. And we still managed find a few that were wrongly
> > sized or not properly aligned despite several efforts in the past to
> > audit these for correctness already.
> > 
> > Even though these ones look to be correct, it will still be easier for
> > future readers of the code if we follow the pattern of using a struct
> > with the array and timestamp instead.
> > 
> > For example, it is much easier to see that:
> > 
> > struct {
> > 	__be32 data[3];
> > 	aligned_s64 timestamp;
> > } buffer;
> >   
> After sending [1], I realized that some (perhaps many) of these would actually
> be a better candidate for the proposed IIO_DECLARE_BUFFER_WITH_TS macro rather
> that converting to the struct style as above.
> 
> Case in point: if the driver using that struct allows reading only one channel,
> then the offset of the timestamp when doing iio_push_to_buffers_with_ts() would
> be 8 bytes, not 16, so the struct would not always be the correct layout.
> 
> As long as the driver doesn't access the timestamp member of the struct, it
> doesn't really matter, but this could be a bit misleading to anyone who might
> unknowing try to use it in the future.
Agreed.  

These timestamp inserting functions have always been a bit weird. I kind
of regret not just leaving it as a per driver thing to do, but that ship
long sailed.  I definitely want to keep the layout apparent in the drivers
though so this approach only applied to 1 of the ones in this series.

Jonathan

> 
> [1]: https://lore.kernel.org/linux-iio/20250418-iio-introduce-iio_declare_buffer_with_ts-v1-0-ee0c62a33a0f@baylibre.com/