| Message ID | 20200607155408.958437-14-jic23@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | IIO: Fused set 1 and 2 of timestamp alignment fixes | expand |
On Sun, 7 Jun 2020 16:53:49 +0100 Jonathan Cameron <jic23@kernel.org> wrote: > From: Jonathan Cameron <Jonathan.Cameron@huawei.com> > > One of a class of bugs pointed out by Lars in a recent review. > iio_push_to_buffers_with_timestamp assumes the buffer used is aligned > to the size of the timestamp (8 bytes). This is not guaranteed in > this driver which uses an array of smaller elements on the stack. > As Lars also noted this anti pattern can involve a leak of data to > userspace and that indeed can happen here. We close both issues by > moving to a suitable structure in the iio_priv() data. > > This data is allocated with kzalloc so no data can leak appart from > previous readings. > > Fixes: 7c94a8b2ee8cf ("iio: magn: add a driver for AK8974") > Reported-by: Lars-Peter Clausen <lars@metafoo.de> > Reviewed-by: Linus Walleij <linus.walleij@linaro.org> > Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> Applied to the fixes-togreg branch of iio.git. I'm picking up all the ones in the series which I have had positive feedback on. Thanks, Jonathan > --- > drivers/iio/magnetometer/ak8974.c | 10 +++++++--- > 1 file changed, 7 insertions(+), 3 deletions(-) > > diff --git a/drivers/iio/magnetometer/ak8974.c b/drivers/iio/magnetometer/ak8974.c > index c2260c84f7f1..ea09b549ec4e 100644 > --- a/drivers/iio/magnetometer/ak8974.c > +++ b/drivers/iio/magnetometer/ak8974.c > @@ -192,6 +192,11 @@ struct ak8974 { > bool drdy_irq; > struct completion drdy_complete; > bool drdy_active_low; > + /* Ensure timestamp is naturally aligned */ > + struct { > + __le16 channels[3]; > + s64 ts __aligned(8); > + } scan; > }; > > static const char ak8974_reg_avdd[] = "avdd"; > @@ -657,7 +662,6 @@ static void ak8974_fill_buffer(struct iio_dev *indio_dev) > { > struct ak8974 *ak8974 = iio_priv(indio_dev); > int ret; > - __le16 hw_values[8]; /* Three axes + 64bit padding */ > > pm_runtime_get_sync(&ak8974->i2c->dev); > mutex_lock(&ak8974->lock); > @@ -667,13 +671,13 @@ static void ak8974_fill_buffer(struct iio_dev *indio_dev) > dev_err(&ak8974->i2c->dev, "error triggering measure\n"); > goto out_unlock; > } > - ret = ak8974_getresult(ak8974, hw_values); > + ret = ak8974_getresult(ak8974, ak8974->scan.channels); > if (ret) { > dev_err(&ak8974->i2c->dev, "error getting measures\n"); > goto out_unlock; > } > > - iio_push_to_buffers_with_timestamp(indio_dev, hw_values, > + iio_push_to_buffers_with_timestamp(indio_dev, &ak8974->scan, > iio_get_time_ns(indio_dev)); > > out_unlock:
diff --git a/drivers/iio/magnetometer/ak8974.c b/drivers/iio/magnetometer/ak8974.c index c2260c84f7f1..ea09b549ec4e 100644 --- a/drivers/iio/magnetometer/ak8974.c +++ b/drivers/iio/magnetometer/ak8974.c @@ -192,6 +192,11 @@ struct ak8974 { bool drdy_irq; struct completion drdy_complete; bool drdy_active_low; + /* Ensure timestamp is naturally aligned */ + struct { + __le16 channels[3]; + s64 ts __aligned(8); + } scan; }; static const char ak8974_reg_avdd[] = "avdd"; @@ -657,7 +662,6 @@ static void ak8974_fill_buffer(struct iio_dev *indio_dev) { struct ak8974 *ak8974 = iio_priv(indio_dev); int ret; - __le16 hw_values[8]; /* Three axes + 64bit padding */ pm_runtime_get_sync(&ak8974->i2c->dev); mutex_lock(&ak8974->lock); @@ -667,13 +671,13 @@ static void ak8974_fill_buffer(struct iio_dev *indio_dev) dev_err(&ak8974->i2c->dev, "error triggering measure\n"); goto out_unlock; } - ret = ak8974_getresult(ak8974, hw_values); + ret = ak8974_getresult(ak8974, ak8974->scan.channels); if (ret) { dev_err(&ak8974->i2c->dev, "error getting measures\n"); goto out_unlock; } - iio_push_to_buffers_with_timestamp(indio_dev, hw_values, + iio_push_to_buffers_with_timestamp(indio_dev, &ak8974->scan, iio_get_time_ns(indio_dev)); out_unlock: