| From a61817216bcc755eabbcb1cf281d84ccad267ed1 Mon Sep 17 00:00:00 2001 |
| From: Jonathan Cameron <Jonathan.Cameron@huawei.com> |
| Date: Sun, 20 Sep 2020 12:27:35 +0100 |
| Subject: iio:light:rpr0521: Fix timestamp alignment and prevent data leak. |
| |
| From: Jonathan Cameron <Jonathan.Cameron@huawei.com> |
| |
| commit a61817216bcc755eabbcb1cf281d84ccad267ed1 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(). |
| 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 |
| Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> |
| |
| --- |
| drivers/iio/light/rpr0521.c | 17 +++++++++++++---- |
| 1 file changed, 13 insertions(+), 4 deletions(-) |
| |
| --- a/drivers/iio/light/rpr0521.c |
| +++ b/drivers/iio/light/rpr0521.c |
| @@ -194,6 +194,17 @@ struct rpr0521_data { |
| bool pxs_need_dis; |
| |
| struct regmap *regmap; |
| + |
| + /* |
| + * Ensure correct naturally aligned timestamp. |
| + * Note that the read will put garbage data into |
| + * the padding but this should not be a problem |
| + */ |
| + struct { |
| + __le16 channels[3]; |
| + u8 garbage; |
| + s64 ts __aligned(8); |
| + } scan; |
| }; |
| |
| static IIO_CONST_ATTR(in_intensity_scale_available, RPR0521_ALS_SCALE_AVAIL); |
| @@ -449,8 +460,6 @@ static irqreturn_t rpr0521_trigger_consu |
| struct rpr0521_data *data = iio_priv(indio_dev); |
| int err; |
| |
| - u8 buffer[16]; /* 3 16-bit channels + padding + ts */ |
| - |
| /* Use irq timestamp when reasonable. */ |
| if (iio_trigger_using_own(indio_dev) && data->irq_timestamp) { |
| pf->timestamp = data->irq_timestamp; |
| @@ -461,11 +470,11 @@ static irqreturn_t rpr0521_trigger_consu |
| pf->timestamp = iio_get_time_ns(indio_dev); |
| |
| err = regmap_bulk_read(data->regmap, RPR0521_REG_PXS_DATA, |
| - &buffer, |
| + data->scan.channels, |
| (3 * 2) + 1); /* 3 * 16-bit + (discarded) int clear reg. */ |
| if (!err) |
| iio_push_to_buffers_with_timestamp(indio_dev, |
| - buffer, pf->timestamp); |
| + &data->scan, pf->timestamp); |
| else |
| dev_err(&data->client->dev, |
| "Trigger consumer can't read from sensor.\n"); |