On Wed, 22 Jul 2020 16:50:43 +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 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@xxxxxxxxxx>
> Cc: Narcisa Ana Maria Vasile <narcisaanamaria12@xxxxxxxxx>
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@xxxxxxxxxx>
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:
