On Mon, 14 Jun 2021 08:03:20 +0000 "Sa, Nuno" <Nuno.Sa@xxxxxxxxxx> wrote: > 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> Thanks! Given how long they've been sitting around on list... Applied to the togreg branch of iio.git and pushed out as testing for all the normal reasons. Thanks, Jonathan > > - Nuno Sá