| Message ID | 20200607155408.958437-17-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:52 +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 apart > from previous readings. > > Fixes: 16bf793f86b2 ("iio: humidity: hdc100x: add triggered buffer support for HDC100X") > Reported-by: Lars-Peter Clausen <lars@metafoo.de> > Acked-by: Matt Ranostay <matt.ranostay@konsulko.com> > Cc: Alison Schofield <amsfield22@gmail.com> > Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> Applied to the fixes-togreg branch of iio.git. I'm scooping up all the ones in this series that have have acks etc as then I can work on getting eyes on the remainder. Thanks, Jonathan > --- > drivers/iio/humidity/hdc100x.c | 10 +++++++--- > 1 file changed, 7 insertions(+), 3 deletions(-) > > diff --git a/drivers/iio/humidity/hdc100x.c b/drivers/iio/humidity/hdc100x.c > index 3331141734c8..e64af35f5f6f 100644 > --- a/drivers/iio/humidity/hdc100x.c > +++ b/drivers/iio/humidity/hdc100x.c > @@ -38,6 +38,11 @@ struct hdc100x_data { > > /* integration time of the sensor */ > int adc_int_us[2]; > + /* Ensure natural alignment of timestamp */ > + struct { > + __be16 channels[2]; > + s64 ts __aligned(8); > + } scan; > }; > > /* integration time in us */ > @@ -322,7 +327,6 @@ static irqreturn_t hdc100x_trigger_handler(int irq, void *p) > struct i2c_client *client = data->client; > int delay = data->adc_int_us[0] + data->adc_int_us[1]; > int ret; > - s16 buf[8]; /* 2x s16 + padding + 8 byte timestamp */ > > /* dual read starts at temp register */ > mutex_lock(&data->lock); > @@ -333,13 +337,13 @@ static irqreturn_t hdc100x_trigger_handler(int irq, void *p) > } > usleep_range(delay, delay + 1000); > > - ret = i2c_master_recv(client, (u8 *)buf, 4); > + ret = i2c_master_recv(client, (u8 *)data->scan.channels, 4); > if (ret < 0) { > dev_err(&client->dev, "cannot read sensor data\n"); > goto err; > } > > - iio_push_to_buffers_with_timestamp(indio_dev, buf, > + iio_push_to_buffers_with_timestamp(indio_dev, &data->scan, > iio_get_time_ns(indio_dev)); > err: > mutex_unlock(&data->lock);
diff --git a/drivers/iio/humidity/hdc100x.c b/drivers/iio/humidity/hdc100x.c index 3331141734c8..e64af35f5f6f 100644 --- a/drivers/iio/humidity/hdc100x.c +++ b/drivers/iio/humidity/hdc100x.c @@ -38,6 +38,11 @@ struct hdc100x_data { /* integration time of the sensor */ int adc_int_us[2]; + /* Ensure natural alignment of timestamp */ + struct { + __be16 channels[2]; + s64 ts __aligned(8); + } scan; }; /* integration time in us */ @@ -322,7 +327,6 @@ static irqreturn_t hdc100x_trigger_handler(int irq, void *p) struct i2c_client *client = data->client; int delay = data->adc_int_us[0] + data->adc_int_us[1]; int ret; - s16 buf[8]; /* 2x s16 + padding + 8 byte timestamp */ /* dual read starts at temp register */ mutex_lock(&data->lock); @@ -333,13 +337,13 @@ static irqreturn_t hdc100x_trigger_handler(int irq, void *p) } usleep_range(delay, delay + 1000); - ret = i2c_master_recv(client, (u8 *)buf, 4); + ret = i2c_master_recv(client, (u8 *)data->scan.channels, 4); if (ret < 0) { dev_err(&client->dev, "cannot read sensor data\n"); goto err; } - iio_push_to_buffers_with_timestamp(indio_dev, buf, + iio_push_to_buffers_with_timestamp(indio_dev, &data->scan, iio_get_time_ns(indio_dev)); err: mutex_unlock(&data->lock);