| From eb1a148ef41d8ae8d9201efc3f1b145976290331 Mon Sep 17 00:00:00 2001 |
| From: Jonathan Cameron <Jonathan.Cameron@huawei.com> |
| Date: Wed, 22 Jul 2020 16:50:43 +0100 |
| Subject: iio:chemical:ccs811: Fix timestamp alignment and prevent data leak. |
| |
| From: Jonathan Cameron <Jonathan.Cameron@huawei.com> |
| |
| commit eb1a148ef41d8ae8d9201efc3f1b145976290331 upstream. |
| |
| 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> |
| Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com> |
| Cc: <Stable@vger.kernel.org> |
| Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> |
| |
| --- |
| drivers/iio/chemical/ccs811.c | 13 +++++++++---- |
| 1 file changed, 9 insertions(+), 4 deletions(-) |
| |
| --- a/drivers/iio/chemical/ccs811.c |
| +++ b/drivers/iio/chemical/ccs811.c |
| @@ -75,6 +75,11 @@ struct ccs811_data { |
| struct ccs811_reading buffer; |
| struct iio_trigger *drdy_trig; |
| 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[] = { |
| @@ -306,17 +311,17 @@ static irqreturn_t ccs811_trigger_handle |
| 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: |