On Sun, 7 Jun 2020 16:53:49 +0100
Jonathan Cameron <jic23@xxxxxxxxxx> wrote:
> From: Jonathan Cameron <Jonathan.Cameron@xxxxxxxxxx>
>
> 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@xxxxxxxxxx>
> Reviewed-by: Linus Walleij <linus.walleij@xxxxxxxxxx>
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@xxxxxxxxxx>
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:
