On Wed, 5 May 2021 14:57:12 +0200
Linus Walleij <linus.walleij@xxxxxxxxxx> wrote:
> On Sat, May 1, 2021 at 7:03 PM Jonathan Cameron <jic23@xxxxxxxxxx> wrote:
>
> > From: Jonathan Cameron <Jonathan.Cameron@xxxxxxxxxx>
> >
> > To make code more readable, use a structure to express the channel
> > layout and ensure the timestamp is 8 byte aligned.
> >
> > Found during an audit of all calls of uses of
> > iio_push_to_buffers_with_timestamp()
> >
> > Fixes: c91746a2361d ("iio: magn: Add support for BMC150 magnetometer")
> > Signed-off-by: Jonathan Cameron <Jonathan.Cameron@xxxxxxxxxx>
> > Cc: Stephan Gerhold <stephan@xxxxxxxxxxx>
> > Cc: Linus Walleij <linus.walleij@xxxxxxxxxx>
>
> Excellent work Jonathan.
> Reviewed-by: Linus Walleij <linus.walleij@xxxxxxxxxx>
>
> I wonder if there is some way for us to abstract this into the core so
> we can't get it wrong.
Abstracting is a bit of a pain, because we end up creating unnecessary
limitations on what can be done. Often having buffer a lot larger than
it needs to be is sensible for example...
However, I'm definitely thinking we should look at what checks we can
add once all these cases are fixed and there might be a nice
pattern to use for those cases where we currently have a bounce buffer
anyway due to hardware restrictions. In most others, moving to the pattern
where the timestamp is explicit in the structure makes it obvious (subject
to the fun question of x86_32 alignment and whether that matters - we don't
know of any bugs as a result but it's possible some buffer consumer will assume
8 byte alignment - hence the hardening in these cases).
The size being too small case for example should be easy - we just augment
iio_push_to_buffers_with_timestamp() to take the size and check it against
scan_bytes. Alignment is trickier...
Jonathan
> Yours,
> Linus Walleij
