Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

iio:light:rpr0521: Fix timestamp alignment and prevent data leak.

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().
This data is allocated with kzalloc() so no data can leak apart
from previous readings and in this case the status byte from the device.

The forced alignment of ts is not necessary in this case but it
potentially makes the code less fragile.

>From personal communications with Mikko:

We could probably split the reading of the int register, but it
would mean a significant performance cost of 20 i2c clock cycles.

Fixes: e12ffd241c00 ("iio: light: rpr0521 triggered buffer")
Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Reviewed-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
Cc: Mikko Koivunen <mikko.koivunen@fi.rohmeurope.com>
Cc: <Stable@vger.kernel.org>
Link: https://lore.kernel.org/r/20200920112742.170751-2-jic23@kernel.org

+13 -4
+13 -4
drivers/iio/light/rpr0521.c
··· 194 194 bool pxs_need_dis; 195 195 196 196 struct regmap *regmap; 197 + 198 + /* 199 + * Ensure correct naturally aligned timestamp. 200 + * Note that the read will put garbage data into 201 + * the padding but this should not be a problem 202 + */ 203 + struct { 204 + __le16 channels[3]; 205 + u8 garbage; 206 + s64 ts __aligned(8); 207 + } scan; 197 208 }; 198 209 199 210 static IIO_CONST_ATTR(in_intensity_scale_available, RPR0521_ALS_SCALE_AVAIL); ··· 460 449 struct rpr0521_data *data = iio_priv(indio_dev); 461 450 int err; 462 451 463 - u8 buffer[16]; /* 3 16-bit channels + padding + ts */ 464 - 465 452 /* Use irq timestamp when reasonable. */ 466 453 if (iio_trigger_using_own(indio_dev) && data->irq_timestamp) { 467 454 pf->timestamp = data->irq_timestamp; ··· 470 461 pf->timestamp = iio_get_time_ns(indio_dev); 471 462 472 463 err = regmap_bulk_read(data->regmap, RPR0521_REG_PXS_DATA, 473 - &buffer, 464 + data->scan.channels, 474 465 (3 * 2) + 1); /* 3 * 16-bit + (discarded) int clear reg. */ 475 466 if (!err) 476 467 iio_push_to_buffers_with_timestamp(indio_dev, 477 - buffer, pf->timestamp); 468 + &data->scan, pf->timestamp); 478 469 else 479 470 dev_err(&data->client->dev, 480 471 "Trigger consumer can't read from sensor.\n");