Re: [PATCH 00/11] IIO: Alignment fixes part 3 - __aligned(8) used to ensure alignment

[Jonathan Cameron <jic23@kernel.org>]

On Thu, 13 May 2021 18:58:32 +0100
Jonathan Cameron <jic23@kernel.org> wrote:

> On Sat,  1 May 2021 18:13:41 +0100
> Jonathan Cameron <jic23@kernel.org> wrote:
> 
> > From: Jonathan Cameron <Jonathan.Cameron@huawei.com>  
> 
> Thanks to those who have already provided reviews on this set.
> If anyone has time to do a quick sanity check of the remaining patches
> it would be much appreciated.  I'll pick up the ones which have been reviewed
> in a few mins.
> 
> +Cc Andy who was kind enough to look at the other 'parts' of this mega series.

Anyone bored?  Still looking for a sanity check of the rest of this
series before I apply it.

I aim to revisit part 4 in the next few days.

Jonathan

> 
> Thanks
> 
> Jonathan
> 
> > 
> > I finally got around to do a manual audit of all the calls to
> > iio_push_to_buffers_with_timestamp() which has the somewhat odd requirements
> > of:
> > 1. 8 byte alignment of the provided buffer.
> > 2. space for an 8 byte naturally aligned timestamp to be inserted at the
> >    end.
> > 
> > Unfortunately there were rather a lot of these left, but time to bite the bullet
> > and clean them up.
> > 
> > As discussed previous in
> > https://lore.kernel.org/linux-iio/20200920112742.170751-1-jic23@kernel.org/
> > it is not easy to fix the alignment issue without requiring a bounce buffer
> > (see part 4 of the alignment fixes for a proposal for that where it is
> > absolutely necessary).
> > 
> > Part 3 contains the cases where the struct approach in part 2 did not seem
> > appropriate.  Normally there are two possible reasons for this.
> > 1.  Would have required an additional memset operation to avoid any
> >     possibility of leaking kernel data.
> > 2.  The location of the timestamp may depend on the channels enabled.
> >     This normally happens when the max sizeof channels is greater than
> >     8 bytes.
> > 
> > Once all cases are fixes, I'll take a look at hardening against any
> > accidental reintroductions. Note that on many platforms and usecases the
> > bug in question will never occur.
> > 
> > Cc: Eugen Hristev <eugen.hristev@microchip.com>
> > Cc: Andreas Klinger <ak@it-klinger.d>
> > Cc: Matt Ranostay <matt.ranostay@konsulko.com>
> > Cc: Gwendal Grignou <gwendal@chromium.org>
> > Cc: Song Qiang <songqiang1304521@gmail.com>
> > Cc: Mathieu Othacehe <m.othacehe@gmail.com>
> > Cc: Parthiban Nallathambi <pn@denx.de>
> > 
> > Jonathan Cameron (11):
> >   iio: adc: at91-sama5d2: Fix buffer alignment in
> >     iio_push_to_buffers_with_timestamp()
> >   iio: adc: hx711: Fix buffer alignment in
> >     iio_push_to_buffers_with_timestamp()
> >   iio: adc: mxs-lradc: Fix buffer alignment in
> >     iio_push_to_buffers_with_timestamp()
> >   iio: adc: ti-ads8688: Fix alignment of buffer in
> >     iio_push_to_buffers_with_timestamp()
> >   iio: chemical: atlas: Fix buffer alignment in
> >     iio_push_to_buffers_with_timestamp()
> >   iio: cros_ec_sensors: Fix alignment of buffer in
> >     iio_push_to_buffers_with_timestamp()
> >   iio: potentiostat: lmp91000: Fix alignment of buffer in
> >     iio_push_to_buffers_with_timestamp()
> >   iio: magn: rm3100: Fix alignment of buffer in
> >     iio_push_to_buffers_with_timestamp()
> >   iio: light: vcnl4000: Fix buffer alignment in
> >     iio_push_to_buffers_with_timestamp()
> >   iio: light: vcnl4035: Fix buffer alignment in
> >     iio_push_to_buffers_with_timestamp()
> >   iio: prox: isl29501: Fix buffer alignment in
> >     iio_push_to_buffers_with_timestamp()
> > 
> >  drivers/iio/adc/at91-sama5d2_adc.c              | 3 ++-
> >  drivers/iio/adc/hx711.c                         | 4 ++--
> >  drivers/iio/adc/mxs-lradc-adc.c                 | 3 ++-
> >  drivers/iio/adc/ti-ads8688.c                    | 3 ++-
> >  drivers/iio/chemical/atlas-sensor.c             | 4 ++--
> >  drivers/iio/light/vcnl4000.c                    | 2 +-
> >  drivers/iio/light/vcnl4035.c                    | 3 ++-
> >  drivers/iio/magnetometer/rm3100-core.c          | 3 ++-
> >  drivers/iio/potentiostat/lmp91000.c             | 4 ++--
> >  drivers/iio/proximity/isl29501.c                | 2 +-
> >  include/linux/iio/common/cros_ec_sensors_core.h | 2 +-
> >  11 files changed, 19 insertions(+), 14 deletions(-)
> >   
>