All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 00/11] IIO: 1st set of timestamp alignment fixes.
@ 2020-05-17 17:29 jic23
  2020-05-17 17:29 ` [PATCH 01/11] iio: accel: kxsd9: Fix alignment of local buffer jic23
                   ` (10 more replies)
  0 siblings, 11 replies; 26+ messages in thread
From: jic23 @ 2020-05-17 17:29 UTC (permalink / raw)
  To: linux-iio; +Cc: Jonathan Cameron

From: Jonathan Cameron <Jonathan.Cameron@huawei.com>

Lars noted in a recent review [1] of the adis16475 that we had an issue around
the alignment requirements of iio_push_to_buffers_with_timestamp.
Whilst it's not documented, that function assumes that the overall buffer
is 8 byte aligned, to ensure the timestamp is itself naturally aligned.
We have drivers that use arrays (typically on the stack) that do
not guarantee this alignment.

We could have fixed this by using a put_unaligned to write the timestamp
but I think that just pushes the problem down the line.  If we were to
have a consumer buffer wanting all the channels in the current
active_scanmask then it will get the raw buffer from the driver passed
straight through.  It seems odd to me if we allow passing a buffer
that is not naturally aligned through to a consumer.
Hence I'm proposing to fix up all existing drivers that might pass
a buffer with insufficient alignment guarantees.
Sometimes the timestamp is guaranteed to be in a particular location,
in which case we can use C structure alignment guarantees to fix this
in a nice readable fashion.  In other cases, the timestamp location
depends on which channels are enabled, and in those case we can
use explicit alignment __aligned(8) to ensure the whole array is
appropriately aligned.

Lars-Peter also noted that, in many of these cases, there are holes
in the stack array that we never write.  Those provide a potential
leak of kernel data to userspace.  For drivers where this applies
we either need to zero those holes each time, or allocate the buffer
on the heap (only once), ensuring it is zeroed at that time.
We may leak previous values from the sensor but currently that seems
unlikely to present any form of security risk.

As such, this first set contains a mixture of fixes.  Where there
are no possible holes, the buffer is kept on the stack but a
c structure is used to guarantee appropriate alignment.  Where
there are holes, the buffer is moved into the iio_priv() accessed
data private structure. A c structure or __aligned(8) is used
as appropriate.

I've stopped at this point rather than doing all the drivers Lars
found in order to both throttle the review burden and also to
see find any general problems with the fixes before doign futher
similar series.  A few of the remaining ones will be rather more
complex to deal with.

These have been there a long time, so whilst they are fixes we
will want in stable I'm not that bothered if it takes us a little
while to get them there!

[1] https://www.spinics.net/lists/devicetree/msg350590.html

Jonathan Cameron (11):
  iio: accel: kxsd9: Fix alignment of local buffer.
  iio:accel:mma8452: Fix timestamp alignment and prevent data leak.
  iio:accel:bmc150-accel: Fix timestamp alignment and prevent data leak.
  iio:accel:mma7455: Fix timestamp alignment and prevent data leak.
  iio:gyro:itg3200: Fix timestamp alignment and prevent data leak.
  iio:proximity:mb1232: Fix timestamp alignment and prevent data leak.
  iio:health:afe4403 Fix timestamp alignment and prevent data leak.
  iio:health:afe4404 Fix timestamp alignment and prevent data leak.
  iio:chemical:ccs811: Fix timestamp alignment and prevent data leak.
  iio:chemical:sps30: Fix timestamp alignment
  iio:chemical:pms7003: Fix timestamp alignment and prevent data leak.

 drivers/iio/accel/bmc150-accel-core.c | 15 ++++++++++++---
 drivers/iio/accel/kxsd9.c             | 16 +++++++++++-----
 drivers/iio/accel/mma7455_core.c      | 16 ++++++++++++----
 drivers/iio/accel/mma8452.c           | 11 ++++++++---
 drivers/iio/chemical/ccs811.c         | 13 +++++++++----
 drivers/iio/chemical/pms7003.c        | 17 ++++++++++++-----
 drivers/iio/chemical/sps30.c          |  9 ++++++---
 drivers/iio/gyro/itg3200_buffer.c     | 15 +++++++++++----
 drivers/iio/health/afe4403.c          |  9 ++++++---
 drivers/iio/health/afe4404.c          |  8 +++++---
 drivers/iio/proximity/mb1232.c        | 17 +++++++++--------
 11 files changed, 101 insertions(+), 45 deletions(-)

-- 
2.26.2


^ permalink raw reply	[flat|nested] 26+ messages in thread

end of thread, other threads:[~2020-05-24 15:34 UTC | newest]

Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-17 17:29 [PATCH 00/11] IIO: 1st set of timestamp alignment fixes jic23
2020-05-17 17:29 ` [PATCH 01/11] iio: accel: kxsd9: Fix alignment of local buffer jic23
2020-05-17 17:29 ` [PATCH 02/11] iio:accel:mma8452: Fix timestamp alignment and prevent data leak jic23
2020-05-17 18:57   ` Lars-Peter Clausen
2020-05-18 16:37     ` Jonathan Cameron
2020-05-17 17:29 ` [PATCH 03/11] iio:accel:bmc150-accel: " jic23
2020-05-17 17:29 ` [PATCH 04/11] iio:accel:mma7455: " jic23
2020-05-17 17:29 ` [PATCH 05/11] iio:gyro:itg3200: " jic23
2020-05-17 17:29 ` [PATCH 06/11] iio:proximity:mb1232: " jic23
2020-05-17 17:29 ` [PATCH 07/11] iio:health:afe4403 " jic23
2020-05-18  0:09   ` Andrew F. Davis
2020-05-18 16:06     ` Jonathan Cameron
2020-05-21 13:11       ` Andrew F. Davis
2020-05-21 14:44         ` Jonathan Cameron
2020-05-21 18:45           ` Jonathan Cameron
2020-05-21 19:05             ` Andrew F. Davis
2020-05-24 15:33               ` Jonathan Cameron
2020-05-17 17:29 ` [PATCH 08/11] iio:health:afe4404 " jic23
2020-05-24 15:34   ` Jonathan Cameron
2020-05-17 17:29 ` [PATCH 09/11] iio:chemical:ccs811: " jic23
2020-05-17 17:29 ` [PATCH 10/11] iio:chemical:sps30: Fix timestamp alignment jic23
2020-05-17 19:07   ` Tomasz Duszynski
2020-05-21 18:43     ` Jonathan Cameron
2020-05-17 17:30 ` [PATCH 11/11] iio:chemical:pms7003: Fix timestamp alignment and prevent data leak jic23
2020-05-17 19:06   ` Tomasz Duszynski
2020-05-21 18:44     ` Jonathan Cameron

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.