| Message ID | 20200722155103.979802-8-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, 22 Jul 2020 16:50:43 +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 with alignment > explicitly requested. This data is allocated with kzalloc so no > data can leak appart from previous readings. > > The explicit alignment of ts is necessary to ensure consistent > padding for x86_32 in which the ts would otherwise be 4 byte aligned. > > Fixes: 283d26917ad6 ("iio: chemical: ccs811: Add triggered buffer support") > Reported-by: Lars-Peter Clausen <lars@metafoo.de> > Cc: Narcisa Ana Maria Vasile <narcisaanamaria12@gmail.com> > Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> Applied > --- > drivers/iio/chemical/ccs811.c | 13 +++++++++---- > 1 file changed, 9 insertions(+), 4 deletions(-) > > diff --git a/drivers/iio/chemical/ccs811.c b/drivers/iio/chemical/ccs811.c > index 2b007e7568b2..60dd87e96f5f 100644 > --- a/drivers/iio/chemical/ccs811.c > +++ b/drivers/iio/chemical/ccs811.c > @@ -78,6 +78,11 @@ struct ccs811_data { > struct iio_trigger *drdy_trig; > struct gpio_desc *wakeup_gpio; > bool drdy_trig_on; > + /* Ensures correct alignment of timestamp if present */ > + struct { > + s16 channels[2]; > + s64 ts __aligned(8); > + } scan; > }; > > static const struct iio_chan_spec ccs811_channels[] = { > @@ -327,17 +332,17 @@ static irqreturn_t ccs811_trigger_handler(int irq, void *p) > struct iio_dev *indio_dev = pf->indio_dev; > struct ccs811_data *data = iio_priv(indio_dev); > struct i2c_client *client = data->client; > - s16 buf[8]; /* s16 eCO2 + s16 TVOC + padding + 8 byte timestamp */ > int ret; > > - ret = i2c_smbus_read_i2c_block_data(client, CCS811_ALG_RESULT_DATA, 4, > - (u8 *)&buf); > + ret = i2c_smbus_read_i2c_block_data(client, CCS811_ALG_RESULT_DATA, > + sizeof(data->scan.channels), > + (u8 *)data->scan.channels); > if (ret != 4) { > 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:
diff --git a/drivers/iio/chemical/ccs811.c b/drivers/iio/chemical/ccs811.c index 2b007e7568b2..60dd87e96f5f 100644 --- a/drivers/iio/chemical/ccs811.c +++ b/drivers/iio/chemical/ccs811.c @@ -78,6 +78,11 @@ struct ccs811_data { struct iio_trigger *drdy_trig; struct gpio_desc *wakeup_gpio; bool drdy_trig_on; + /* Ensures correct alignment of timestamp if present */ + struct { + s16 channels[2]; + s64 ts __aligned(8); + } scan; }; static const struct iio_chan_spec ccs811_channels[] = { @@ -327,17 +332,17 @@ static irqreturn_t ccs811_trigger_handler(int irq, void *p) struct iio_dev *indio_dev = pf->indio_dev; struct ccs811_data *data = iio_priv(indio_dev); struct i2c_client *client = data->client; - s16 buf[8]; /* s16 eCO2 + s16 TVOC + padding + 8 byte timestamp */ int ret; - ret = i2c_smbus_read_i2c_block_data(client, CCS811_ALG_RESULT_DATA, 4, - (u8 *)&buf); + ret = i2c_smbus_read_i2c_block_data(client, CCS811_ALG_RESULT_DATA, + sizeof(data->scan.channels), + (u8 *)data->scan.channels); if (ret != 4) { 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: