On Mon, May 25, 2020 at 10:09 AM 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.
> This data is allocated with kzalloc so no data can leak apart
> from previous readings.
>
Acked-by: Matt Ranostay <matt.ranostay@xxxxxxxxxxxx>
> Fixes: 16bf793f86b2 ("iio: humidity: hdc100x: add triggered buffer support for HDC100X")
> Reported-by: Lars-Peter Clausen <lars@xxxxxxxxxx>
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@xxxxxxxxxx>
> Cc: Alison Schofield <amsfield22@xxxxxxxxx>
> Cc: Matt Ranostay <matt.ranostay@xxxxxxxxxxxx>
> ---
> 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 7ecd2ffa3132..fd825e281d4f 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;
> + } 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);
> --
> 2.26.2
>
