Re: [PATCH 06/25] iio:magnetometer:ak8974: Fix alignment and data leak issues

[Jonathan Cameron <jic23@kernel.org>]

On Tue, 26 May 2020 11:24:40 +0200
Linus Walleij <linus.walleij@linaro.org> wrote:

> On Mon, May 25, 2020 at 7:09 PM Jonathan Cameron <jic23@kernel.org> wrote:
> 
> > From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> >
> > One of a class of bugs pointed out by Lars in a recent review.
> > iio_push_to_buffers_with_timestamp assumes the buffer used is aligned
> > to the size of the timestamp (8 bytes).  This is not guaranteed in
> > this driver which uses an array of smaller elements on the stack.
> > As Lars also noted this anti pattern can involve a leak of data to
> > userspace and that indeed can happen here.  We close both issues by
> > moving to a suitable structure in the iio_priv() data.
> >
> > This data is allocated with kzalloc so no data can leak appart from
> > previous readings.
> >
> > Fixes: 7c94a8b2ee8cf ("iio: magn: add a driver for AK8974")
> > Reported-by: Lars-Peter Clausen <lars@metafoo.de>
> > Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> > Cc: Linus Walleij <linus.walleij@linaro.org>  
> 
> Whoa, good catch! Definitely my mindless coding.
> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
I've tweaked this slightly fro v2 to add an __aligned(8) to the ts.
This is driven by the need for some cases to be careful on x86_32
where the s64 might be 4 byte aligned and the padding come out wrong.
It doesn't actually matter in this case but I'd rather be explicit.

Have kept the reviewed-by as not a material change on this one.

Thanks,

Jonathan

> 
> Yours,
> Linus Walleij