| Message ID | 20210501171352.512953-1-jic23@kernel.org (mailing list archive) |
|---|---|
| Headers | show |
| Series | IIO: Alignment fixes part 3 - __aligned(8) used to ensure alignment | expand |
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. 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(-) >
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(-) > > >
On Wed, May 26, 2021 at 8:07 PM Jonathan Cameron <jic23@kernel.org> wrote: > > 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. Sorry, I have some high priority tasks, and this one is not in the list :-(
From: Jonathan Cameron <Jonathan.Cameron@huawei.com> 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(-)