Message ID | 20200722155103.979802-15-jic23@kernel.org (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | IIO: Fused set 1 and 2 of timestamp alignment fixes | expand |
On Wed, Jul 22, 2020 at 6:53 PM 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 apart from > previous readings. > > The explicit alignment of ts is not necessary in this case but > does make the code slightly less fragile so I have included it. > I broke my eyes and brain over temp. At first I thought it was temporary garbage like in the other patch, but now I'm guessing (looking into the comment) that this is temperature. Can we spell it fully? ... > + u8 temp; > - u8 buffer[16]; /* 3 16-bit channels + 1 byte temp + padding + ts */ > + data->scan.temp = ret;
diff --git a/drivers/iio/magnetometer/mag3110.c b/drivers/iio/magnetometer/mag3110.c index 4d305a21c379..e96113ca39bd 100644 --- a/drivers/iio/magnetometer/mag3110.c +++ b/drivers/iio/magnetometer/mag3110.c @@ -56,6 +56,12 @@ struct mag3110_data { int sleep_val; struct regulator *vdd_reg; struct regulator *vddio_reg; + /* Ensure natural alignment of timestamp */ + struct { + __be16 channels[3]; + u8 temp; + s64 ts __aligned(8); + } scan; }; static int mag3110_request(struct mag3110_data *data) @@ -387,10 +393,9 @@ static irqreturn_t mag3110_trigger_handler(int irq, void *p) struct iio_poll_func *pf = p; struct iio_dev *indio_dev = pf->indio_dev; struct mag3110_data *data = iio_priv(indio_dev); - u8 buffer[16]; /* 3 16-bit channels + 1 byte temp + padding + ts */ int ret; - ret = mag3110_read(data, (__be16 *) buffer); + ret = mag3110_read(data, data->scan.channels); if (ret < 0) goto done; @@ -399,10 +404,10 @@ static irqreturn_t mag3110_trigger_handler(int irq, void *p) MAG3110_DIE_TEMP); if (ret < 0) goto done; - buffer[6] = ret; + data->scan.temp = ret; } - iio_push_to_buffers_with_timestamp(indio_dev, buffer, + iio_push_to_buffers_with_timestamp(indio_dev, &data->scan, iio_get_time_ns(indio_dev)); done: