RE: [PATCH RESEND 0/8] IIO: Alignment fixes part 3 - __aligned(8) used to ensure alignment

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Jonathan,

> From: Jonathan Cameron <jic23@xxxxxxxxxx>
> Sent: Sunday, June 13, 2021 5:23 PM
> To: linux-iio@xxxxxxxxxxxxxxx
> Cc: Jonathan Cameron <Jonathan.Cameron@xxxxxxxxxx>; Eugen
> Hristev <eugen.hristev@xxxxxxxxxxxxx>; Andreas Klinger <ak@it-
> klinger.d>; Song Qiang <songqiang1304521@xxxxxxxxx>; Mathieu
> Othacehe <m.othacehe@xxxxxxxxx>; Parthiban Nallathambi
> <pn@xxxxxxx>
> Subject: [PATCH RESEND 0/8] IIO: Alignment fixes part 3 - __aligned(8)
> used to ensure alignment
> 
> From: Jonathan Cameron <Jonathan.Cameron@xxxxxxxxxx>
> 
> Resent as none of these have had any comments since being sent out
> over
> a month ago.  Note these are very straight forward, but I've messed up
> such things in the past so really appreciate it if someone takes the time
> to do a quick sanity check.
> 
> I have picked up those patches that did get review in first posting and
> dropped them from this posting.
> 
> Cover letter from first posting.
> 
> 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://urldefense.com/v3/__https://lore.kernel.org/linux-
> iio/20200920112742.170751-1-
> jic23@xxxxxxxxxx/__;!!A3Ni8CS0y2Y!uCn6NSpGUvHYdcFazn7flxxjh8RA
> RqPCumitrRDDgGAjXF7crgi911KY2v_iyw$
> 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@xxxxxxxxxxxxx>
> Cc: Andreas Klinger <ak@it-klinger.d>
> Cc: Song Qiang <songqiang1304521@xxxxxxxxx>
> Cc: Mathieu Othacehe <m.othacehe@xxxxxxxxx>
> Cc: Parthiban Nallathambi <pn@xxxxxxx>
> 
> Jonathan Cameron (8):
>   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: 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/light/vcnl4000.c           | 2 +-
>  drivers/iio/light/vcnl4035.c           | 3 ++-
>  drivers/iio/magnetometer/rm3100-core.c | 3 ++-
>  drivers/iio/proximity/isl29501.c       | 2 +-
>  8 files changed, 14 insertions(+), 9 deletions(-)
> 
> --
> 2.32.0

LGTM. For the whole series:

Reviewed-by: Nuno Sá <nuno.sa@xxxxxxxxxx>

- Nuno Sá




[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Input]     [Linux Kernel]     [Linux SCSI]     [X.org]

  Powered by Linux