This commit resolves a bug in the selection/discovery of more
specific USB device drivers for devices that are currently bound to
generic USB device drivers.
The bug is in the logic that determines whether a device currently
bound to a generic USB device driver should be re-probed by a
more specific USB device driver or not. The code in
__usb_bus_reprobe_drivers() used to have the following lines:
if (usb_device_match_id(udev, new_udriver->id_table) == NULL &&
(!new_udriver->match || new_udriver->match(udev) != 0))
return 0;
ret = device_reprobe(dev);
As the reader will notice, the code checks whether the USB device in
consideration matches the identifier table (id_table) of a specific
USB device_driver (new_udriver), followed by a similar check, but this
time with the USB device driver's match function. However, the match
function's return value is not checked correctly. When match() returns
zero, it means that the specific USB device driver is *not* applicable
to the USB device in question, but the code then goes on to reprobe the
device with the new USB device driver under consideration. All this to
say, the logic is inverted.
This bug was found by code inspection and instrumentation while
investigating the root cause of the issue reported by Andrey Konovalov,
where usbip took over syzkaller's virtual USB devices in an undesired
manner. The report is linked below.
Fixes: d5643d2249 ("USB: Fix device driver race")
Link: https://lore.kernel.org/linux-usb/CAAeHK+zOrHnxjRFs=OE8T=O9208B9HP_oo8RZpyV…
Cc: <stable(a)vger.kernel.org> # 5.8
Cc: Greg Kroah-Hartman <gregkh(a)linuxfoundation.org>
Cc: Alan Stern <stern(a)rowland.harvard.edu>
Cc: Bastien Nocera <hadess(a)hadess.net>
Cc: Shuah Khan <shuah(a)kernel.org>
Cc: Valentina Manea <valentina.manea.m(a)gmail.com>
Cc: <syzkaller(a)googlegroups.com>
Signed-off-by: M. Vefa Bicakci <m.v.b(a)runbox.com>
---
v3: No functional changes; only commit message changes.
v2: Following Bastien Nocera's suggestion, this is a new patch,
split from the patch published at:
https://lore.kernel.org/linux-usb/20200917095959.174378-1-m.v.b@runbox.com/
---
drivers/usb/core/driver.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/usb/core/driver.c b/drivers/usb/core/driver.c
index c976ea9f9582..950044a6e77f 100644
--- a/drivers/usb/core/driver.c
+++ b/drivers/usb/core/driver.c
@@ -924,7 +924,7 @@ static int __usb_bus_reprobe_drivers(struct device *dev, void *data)
udev = to_usb_device(dev);
if (usb_device_match_id(udev, new_udriver->id_table) == NULL &&
- (!new_udriver->match || new_udriver->match(udev) != 0))
+ (!new_udriver->match || new_udriver->match(udev) == 0))
return 0;
ret = device_reprobe(dev);
--
2.26.2
Handle broken union functional descriptors where the master-interface
doesn't exist or where its class is of neither Communication or Data
type (as required by the specification) by falling back to
"combined-interface" probing.
Note that this still allows for handling union descriptors with switched
interfaces.
This specifically makes the Whistler radio scanners TRX series devices
work with the driver without adding further quirks to the device-id
table.
Link: https://lore.kernel.org/r/5f4ca4f8.1c69fb81.a4487.0f5f@mx.google.com
Reported-by: Daniel Caujolle-Bert <f1rmb.daniel(a)gmail.com>
Cc: stable(a)vger.kernel.org
Signed-off-by: Johan Hovold <johan(a)kernel.org>
---
drivers/usb/class/cdc-acm.c | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/drivers/usb/class/cdc-acm.c b/drivers/usb/class/cdc-acm.c
index e28ac640ff9c..f42ade505569 100644
--- a/drivers/usb/class/cdc-acm.c
+++ b/drivers/usb/class/cdc-acm.c
@@ -1240,9 +1240,21 @@ static int acm_probe(struct usb_interface *intf,
}
}
} else {
+ int class = -1;
+
data_intf_num = union_header->bSlaveInterface0;
control_interface = usb_ifnum_to_if(usb_dev, union_header->bMasterInterface0);
data_interface = usb_ifnum_to_if(usb_dev, data_intf_num);
+
+ if (control_interface)
+ class = control_interface->cur_altsetting->desc.bInterfaceClass;
+
+ if (class != USB_CLASS_COMM && class != USB_CLASS_CDC_DATA) {
+ dev_dbg(&intf->dev, "Broken union descriptor, assuming single interface\n");
+ combined_interfaces = 1;
+ control_interface = data_interface = intf;
+ goto look_for_collapsed_interface;
+ }
}
if (!control_interface || !data_interface) {
--
2.26.2
This is a note to let you know that I've just added the patch titled
iio:imu:inv_mpu6050 Fix dma and ts alignment and data leak issues.
to my staging git tree which can be found at
git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git
in the staging-testing branch.
The patch will show up in the next release of the linux-next tree
(usually sometime within the next 24 hours during the week.)
The patch will be merged to the staging-next branch sometime soon,
after it passes testing, and the merge window is open.
If you have any questions about this process, please let me know.
>From 6b0cc5dce0725ae8f1a2883514da731c55eeb35e Mon Sep 17 00:00:00 2001
From: Jonathan Cameron <Jonathan.Cameron(a)huawei.com>
Date: Wed, 22 Jul 2020 16:50:53 +0100
Subject: iio:imu:inv_mpu6050 Fix dma and ts alignment and data leak issues.
This case is a bit different to the rest of the series. The driver
was doing a regmap_bulk_read into a buffer that wasn't dma safe
as it was on the stack with no guarantee of it being in a cacheline
on it's own. Fixing that also dealt with the data leak and
alignment issues that Lars-Peter pointed out.
Also removed some unaligned handling as we are now aligned.
Fixes tag is for the dma safe buffer issue. Potentially we would
need to backport timestamp alignment futher but that is a totally
different patch.
Fixes: fd64df16f40e ("iio: imu: inv_mpu6050: Add SPI support for MPU6000")
Reported-by: Lars-Peter Clausen <lars(a)metafoo.de>
Signed-off-by: Jonathan Cameron <Jonathan.Cameron(a)huawei.com>
Reviewed-by: Andy Shevchenko <andy.shevchenko(a)gmail.com>
Reviewed-by: Jean-Baptiste Maneyrol <jmaneyrol(a)invensense.com>
Cc: <Stable(a)vger.kernel.org>
Link: https://lore.kernel.org/r/20200722155103.979802-18-jic23@kernel.org
---
drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h | 12 +++++++++---
drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c | 12 +++++-------
2 files changed, 14 insertions(+), 10 deletions(-)
diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h b/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
index cd38b3fccc7b..eb522b38acf3 100644
--- a/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
+++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
@@ -122,6 +122,13 @@ struct inv_mpu6050_chip_config {
u8 user_ctrl;
};
+/*
+ * Maximum of 6 + 6 + 2 + 7 (for MPU9x50) = 21 round up to 24 and plus 8.
+ * May be less if fewer channels are enabled, as long as the timestamp
+ * remains 8 byte aligned
+ */
+#define INV_MPU6050_OUTPUT_DATA_SIZE 32
+
/**
* struct inv_mpu6050_hw - Other important hardware information.
* @whoami: Self identification byte from WHO_AM_I register
@@ -165,6 +172,7 @@ struct inv_mpu6050_hw {
* @magn_raw_to_gauss: coefficient to convert mag raw value to Gauss.
* @magn_orient: magnetometer sensor chip orientation if available.
* @suspended_sensors: sensors mask of sensors turned off for suspend
+ * @data: dma safe buffer used for bulk reads.
*/
struct inv_mpu6050_state {
struct mutex lock;
@@ -190,6 +198,7 @@ struct inv_mpu6050_state {
s32 magn_raw_to_gauss[3];
struct iio_mount_matrix magn_orient;
unsigned int suspended_sensors;
+ u8 data[INV_MPU6050_OUTPUT_DATA_SIZE] ____cacheline_aligned;
};
/*register and associated bit definition*/
@@ -334,9 +343,6 @@ struct inv_mpu6050_state {
#define INV_ICM20608_TEMP_OFFSET 8170
#define INV_ICM20608_TEMP_SCALE 3059976
-/* 6 + 6 + 2 + 7 (for MPU9x50) = 21 round up to 24 and plus 8 */
-#define INV_MPU6050_OUTPUT_DATA_SIZE 32
-
#define INV_MPU6050_REG_INT_PIN_CFG 0x37
#define INV_MPU6050_ACTIVE_HIGH 0x00
#define INV_MPU6050_ACTIVE_LOW 0x80
diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c
index b533fa2dad0a..d8e6b88ddffc 100644
--- a/drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c
+++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c
@@ -13,7 +13,6 @@
#include <linux/interrupt.h>
#include <linux/poll.h>
#include <linux/math64.h>
-#include <asm/unaligned.h>
#include "inv_mpu_iio.h"
/**
@@ -121,7 +120,6 @@ irqreturn_t inv_mpu6050_read_fifo(int irq, void *p)
struct inv_mpu6050_state *st = iio_priv(indio_dev);
size_t bytes_per_datum;
int result;
- u8 data[INV_MPU6050_OUTPUT_DATA_SIZE];
u16 fifo_count;
s64 timestamp;
int int_status;
@@ -160,11 +158,11 @@ irqreturn_t inv_mpu6050_read_fifo(int irq, void *p)
* read fifo_count register to know how many bytes are inside the FIFO
* right now
*/
- result = regmap_bulk_read(st->map, st->reg->fifo_count_h, data,
- INV_MPU6050_FIFO_COUNT_BYTE);
+ result = regmap_bulk_read(st->map, st->reg->fifo_count_h,
+ st->data, INV_MPU6050_FIFO_COUNT_BYTE);
if (result)
goto end_session;
- fifo_count = get_unaligned_be16(&data[0]);
+ fifo_count = be16_to_cpup((__be16 *)&st->data[0]);
/*
* Handle fifo overflow by resetting fifo.
@@ -182,7 +180,7 @@ irqreturn_t inv_mpu6050_read_fifo(int irq, void *p)
inv_mpu6050_update_period(st, pf->timestamp, nb);
for (i = 0; i < nb; ++i) {
result = regmap_bulk_read(st->map, st->reg->fifo_r_w,
- data, bytes_per_datum);
+ st->data, bytes_per_datum);
if (result)
goto flush_fifo;
/* skip first samples if needed */
@@ -191,7 +189,7 @@ irqreturn_t inv_mpu6050_read_fifo(int irq, void *p)
continue;
}
timestamp = inv_mpu6050_get_timestamp(st);
- iio_push_to_buffers_with_timestamp(indio_dev, data, timestamp);
+ iio_push_to_buffers_with_timestamp(indio_dev, st->data, timestamp);
}
end_session:
--
2.28.0
This is a note to let you know that I've just added the patch titled
iio:adc:ti-adc0832 Fix alignment issue with timestamp
to my staging git tree which can be found at
git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git
in the staging-testing branch.
The patch will show up in the next release of the linux-next tree
(usually sometime within the next 24 hours during the week.)
The patch will be merged to the staging-next branch sometime soon,
after it passes testing, and the merge window is open.
If you have any questions about this process, please let me know.
>From 39e91f3be4cba51c1560bcda3a343ed1f64dc916 Mon Sep 17 00:00:00 2001
From: Jonathan Cameron <Jonathan.Cameron(a)huawei.com>
Date: Wed, 22 Jul 2020 16:51:00 +0100
Subject: iio:adc:ti-adc0832 Fix alignment issue with timestamp
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.
We fix this 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 apart from previous readings.
Note that previously no data could leak 'including' previous readings
but I don't think it is an issue to potentially leak them like
this now does.
In this case the postioning of the timestamp is depends on what
other channels are enabled. As such we cannot use a structure to
make the alignment explicit as it would be missleading by suggesting
only one possible location for the timestamp.
Fixes: 815bbc87462a ("iio: ti-adc0832: add triggered buffer support")
Reported-by: Lars-Peter Clausen <lars(a)metafoo.de>
Signed-off-by: Jonathan Cameron <Jonathan.Cameron(a)huawei.com>
Reviewed-by: Andy Shevchenko <andy.shevchenko(a)gmail.com>
Cc: Akinobu Mita <akinobu.mita(a)gmail.com>
Cc: <Stable(a)vger.kernel.org>
Link: https://lore.kernel.org/r/20200722155103.979802-25-jic23@kernel.org
---
drivers/iio/adc/ti-adc0832.c | 11 ++++++++---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/drivers/iio/adc/ti-adc0832.c b/drivers/iio/adc/ti-adc0832.c
index c7a085dce1f4..0261b3cfc92b 100644
--- a/drivers/iio/adc/ti-adc0832.c
+++ b/drivers/iio/adc/ti-adc0832.c
@@ -29,6 +29,12 @@ struct adc0832 {
struct regulator *reg;
struct mutex lock;
u8 mux_bits;
+ /*
+ * Max size needed: 16x 1 byte ADC data + 8 bytes timestamp
+ * May be shorter if not all channels are enabled subject
+ * to the timestamp remaining 8 byte aligned.
+ */
+ u8 data[24] __aligned(8);
u8 tx_buf[2] ____cacheline_aligned;
u8 rx_buf[2];
@@ -200,7 +206,6 @@ static irqreturn_t adc0832_trigger_handler(int irq, void *p)
struct iio_poll_func *pf = p;
struct iio_dev *indio_dev = pf->indio_dev;
struct adc0832 *adc = iio_priv(indio_dev);
- u8 data[24] = { }; /* 16x 1 byte ADC data + 8 bytes timestamp */
int scan_index;
int i = 0;
@@ -218,10 +223,10 @@ static irqreturn_t adc0832_trigger_handler(int irq, void *p)
goto out;
}
- data[i] = ret;
+ adc->data[i] = ret;
i++;
}
- iio_push_to_buffers_with_timestamp(indio_dev, data,
+ iio_push_to_buffers_with_timestamp(indio_dev, adc->data,
iio_get_time_ns(indio_dev));
out:
mutex_unlock(&adc->lock);
--
2.28.0
This is a note to let you know that I've just added the patch titled
iio:adc:ti-adc12138 Fix alignment issue with timestamp
to my staging git tree which can be found at
git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git
in the staging-testing branch.
The patch will show up in the next release of the linux-next tree
(usually sometime within the next 24 hours during the week.)
The patch will be merged to the staging-next branch sometime soon,
after it passes testing, and the merge window is open.
If you have any questions about this process, please let me know.
>From 293e809b2e8e608b65a949101aaf7c0bd1224247 Mon Sep 17 00:00:00 2001
From: Jonathan Cameron <Jonathan.Cameron(a)huawei.com>
Date: Wed, 22 Jul 2020 16:51:01 +0100
Subject: iio:adc:ti-adc12138 Fix alignment issue with timestamp
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.
We move to a suitable structure in the iio_priv() data with alignment
explicitly requested. This data is allocated with kzalloc so no
data can leak apart from previous readings. Note that previously
no leak at all could occur, but previous readings should never
be a problem.
In this case the timestamp location depends on what other channels
are enabled. As such we can't use a structure without misleading
by suggesting only one possible timestamp location.
Fixes: 50a6edb1b6e0 ("iio: adc: add ADC12130/ADC12132/ADC12138 ADC driver")
Reported-by: Lars-Peter Clausen <lars(a)metafoo.de>
Signed-off-by: Jonathan Cameron <Jonathan.Cameron(a)huawei.com>
Reviewed-by: Andy Shevchenko <andy.shevchenko(a)gmail.com>
Cc: Akinobu Mita <akinobu.mita(a)gmail.com>
Cc: <Stable(a)vger.kernel.org>
Link: https://lore.kernel.org/r/20200722155103.979802-26-jic23@kernel.org
---
drivers/iio/adc/ti-adc12138.c | 13 +++++++++----
1 file changed, 9 insertions(+), 4 deletions(-)
diff --git a/drivers/iio/adc/ti-adc12138.c b/drivers/iio/adc/ti-adc12138.c
index e485719cd2c4..fcd5d39dd03e 100644
--- a/drivers/iio/adc/ti-adc12138.c
+++ b/drivers/iio/adc/ti-adc12138.c
@@ -47,6 +47,12 @@ struct adc12138 {
struct completion complete;
/* The number of cclk periods for the S/H's acquisition time */
unsigned int acquisition_time;
+ /*
+ * Maximum size needed: 16x 2 bytes ADC data + 8 bytes timestamp.
+ * Less may be need if not all channels are enabled, as long as
+ * the 8 byte alignment of the timestamp is maintained.
+ */
+ __be16 data[20] __aligned(8);
u8 tx_buf[2] ____cacheline_aligned;
u8 rx_buf[2];
@@ -329,7 +335,6 @@ static irqreturn_t adc12138_trigger_handler(int irq, void *p)
struct iio_poll_func *pf = p;
struct iio_dev *indio_dev = pf->indio_dev;
struct adc12138 *adc = iio_priv(indio_dev);
- __be16 data[20] = { }; /* 16x 2 bytes ADC data + 8 bytes timestamp */
__be16 trash;
int ret;
int scan_index;
@@ -345,7 +350,7 @@ static irqreturn_t adc12138_trigger_handler(int irq, void *p)
reinit_completion(&adc->complete);
ret = adc12138_start_and_read_conv(adc, scan_chan,
- i ? &data[i - 1] : &trash);
+ i ? &adc->data[i - 1] : &trash);
if (ret) {
dev_warn(&adc->spi->dev,
"failed to start conversion\n");
@@ -362,7 +367,7 @@ static irqreturn_t adc12138_trigger_handler(int irq, void *p)
}
if (i) {
- ret = adc12138_read_conv_data(adc, &data[i - 1]);
+ ret = adc12138_read_conv_data(adc, &adc->data[i - 1]);
if (ret) {
dev_warn(&adc->spi->dev,
"failed to get conversion data\n");
@@ -370,7 +375,7 @@ static irqreturn_t adc12138_trigger_handler(int irq, void *p)
}
}
- iio_push_to_buffers_with_timestamp(indio_dev, data,
+ iio_push_to_buffers_with_timestamp(indio_dev, adc->data,
iio_get_time_ns(indio_dev));
out:
mutex_unlock(&adc->lock);
--
2.28.0
This is a note to let you know that I've just added the patch titled
iio:imu:st_lsm6dsx Fix alignment and data leak issues
to my staging git tree which can be found at
git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git
in the staging-testing branch.
The patch will show up in the next release of the linux-next tree
(usually sometime within the next 24 hours during the week.)
The patch will be merged to the staging-next branch sometime soon,
after it passes testing, and the merge window is open.
If you have any questions about this process, please let me know.
>From c14edb4d0bdc53f969ea84c7f384472c28b1a9f8 Mon Sep 17 00:00:00 2001
From: Jonathan Cameron <Jonathan.Cameron(a)huawei.com>
Date: Wed, 22 Jul 2020 16:50:52 +0100
Subject: iio:imu:st_lsm6dsx Fix alignment and data leak issues
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 an array of suitable structures in the iio_priv() data.
This data is allocated with kzalloc so no data can leak apart from
previous readings.
For the tagged path the data is aligned by using __aligned(8) for
the buffer on the stack.
There has been a lot of churn in this driver, so likely backports
may be needed for stable.
Fixes: 290a6ce11d93 ("iio: imu: add support to lsm6dsx driver")
Reported-by: Lars-Peter Clausen <lars(a)metafoo.de>
Signed-off-by: Jonathan Cameron <Jonathan.Cameron(a)huawei.com>
Reviewed-by: Andy Shevchenko <andy.shevchenko(a)gmail.com>
Cc: Lorenzo Bianconi <lorenzo.bianconi83(a)gmail.com>
Cc: <Stable(a)vger.kernel.org>
Link: https://lore.kernel.org/r/20200722155103.979802-17-jic23@kernel.org
---
drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h | 6 +++
.../iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c | 42 ++++++++++++-------
2 files changed, 32 insertions(+), 16 deletions(-)
diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
index d80ba2e688ed..9275346a9cc1 100644
--- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
+++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
@@ -383,6 +383,7 @@ struct st_lsm6dsx_sensor {
* @iio_devs: Pointers to acc/gyro iio_dev instances.
* @settings: Pointer to the specific sensor settings in use.
* @orientation: sensor chip orientation relative to main hardware.
+ * @scan: Temporary buffers used to align data before iio_push_to_buffers()
*/
struct st_lsm6dsx_hw {
struct device *dev;
@@ -411,6 +412,11 @@ struct st_lsm6dsx_hw {
const struct st_lsm6dsx_settings *settings;
struct iio_mount_matrix orientation;
+ /* Ensure natural alignment of buffer elements */
+ struct {
+ __le16 channels[3];
+ s64 ts __aligned(8);
+ } scan[3];
};
static __maybe_unused const struct iio_event_spec st_lsm6dsx_event = {
diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
index 7de10bd636ea..12ed0a2e55e4 100644
--- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
+++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
@@ -353,9 +353,6 @@ int st_lsm6dsx_read_fifo(struct st_lsm6dsx_hw *hw)
int err, sip, acc_sip, gyro_sip, ts_sip, ext_sip, read_len, offset;
u16 fifo_len, pattern_len = hw->sip * ST_LSM6DSX_SAMPLE_SIZE;
u16 fifo_diff_mask = hw->settings->fifo_ops.fifo_diff.mask;
- u8 gyro_buff[ST_LSM6DSX_IIO_BUFF_SIZE];
- u8 acc_buff[ST_LSM6DSX_IIO_BUFF_SIZE];
- u8 ext_buff[ST_LSM6DSX_IIO_BUFF_SIZE];
bool reset_ts = false;
__le16 fifo_status;
s64 ts = 0;
@@ -416,19 +413,22 @@ int st_lsm6dsx_read_fifo(struct st_lsm6dsx_hw *hw)
while (acc_sip > 0 || gyro_sip > 0 || ext_sip > 0) {
if (gyro_sip > 0 && !(sip % gyro_sensor->decimator)) {
- memcpy(gyro_buff, &hw->buff[offset],
- ST_LSM6DSX_SAMPLE_SIZE);
- offset += ST_LSM6DSX_SAMPLE_SIZE;
+ memcpy(hw->scan[ST_LSM6DSX_ID_GYRO].channels,
+ &hw->buff[offset],
+ sizeof(hw->scan[ST_LSM6DSX_ID_GYRO].channels));
+ offset += sizeof(hw->scan[ST_LSM6DSX_ID_GYRO].channels);
}
if (acc_sip > 0 && !(sip % acc_sensor->decimator)) {
- memcpy(acc_buff, &hw->buff[offset],
- ST_LSM6DSX_SAMPLE_SIZE);
- offset += ST_LSM6DSX_SAMPLE_SIZE;
+ memcpy(hw->scan[ST_LSM6DSX_ID_ACC].channels,
+ &hw->buff[offset],
+ sizeof(hw->scan[ST_LSM6DSX_ID_ACC].channels));
+ offset += sizeof(hw->scan[ST_LSM6DSX_ID_ACC].channels);
}
if (ext_sip > 0 && !(sip % ext_sensor->decimator)) {
- memcpy(ext_buff, &hw->buff[offset],
- ST_LSM6DSX_SAMPLE_SIZE);
- offset += ST_LSM6DSX_SAMPLE_SIZE;
+ memcpy(hw->scan[ST_LSM6DSX_ID_EXT0].channels,
+ &hw->buff[offset],
+ sizeof(hw->scan[ST_LSM6DSX_ID_EXT0].channels));
+ offset += sizeof(hw->scan[ST_LSM6DSX_ID_EXT0].channels);
}
if (ts_sip-- > 0) {
@@ -458,19 +458,22 @@ int st_lsm6dsx_read_fifo(struct st_lsm6dsx_hw *hw)
if (gyro_sip > 0 && !(sip % gyro_sensor->decimator)) {
iio_push_to_buffers_with_timestamp(
hw->iio_devs[ST_LSM6DSX_ID_GYRO],
- gyro_buff, gyro_sensor->ts_ref + ts);
+ &hw->scan[ST_LSM6DSX_ID_GYRO],
+ gyro_sensor->ts_ref + ts);
gyro_sip--;
}
if (acc_sip > 0 && !(sip % acc_sensor->decimator)) {
iio_push_to_buffers_with_timestamp(
hw->iio_devs[ST_LSM6DSX_ID_ACC],
- acc_buff, acc_sensor->ts_ref + ts);
+ &hw->scan[ST_LSM6DSX_ID_ACC],
+ acc_sensor->ts_ref + ts);
acc_sip--;
}
if (ext_sip > 0 && !(sip % ext_sensor->decimator)) {
iio_push_to_buffers_with_timestamp(
hw->iio_devs[ST_LSM6DSX_ID_EXT0],
- ext_buff, ext_sensor->ts_ref + ts);
+ &hw->scan[ST_LSM6DSX_ID_EXT0],
+ ext_sensor->ts_ref + ts);
ext_sip--;
}
sip++;
@@ -555,7 +558,14 @@ int st_lsm6dsx_read_tagged_fifo(struct st_lsm6dsx_hw *hw)
{
u16 pattern_len = hw->sip * ST_LSM6DSX_TAGGED_SAMPLE_SIZE;
u16 fifo_len, fifo_diff_mask;
- u8 iio_buff[ST_LSM6DSX_IIO_BUFF_SIZE], tag;
+ /*
+ * Alignment needed as this can ultimately be passed to a
+ * call to iio_push_to_buffers_with_timestamp() which
+ * must be passed a buffer that is aligned to 8 bytes so
+ * as to allow insertion of a naturally aligned timestamp.
+ */
+ u8 iio_buff[ST_LSM6DSX_IIO_BUFF_SIZE] __aligned(8);
+ u8 tag;
bool reset_ts = false;
int i, err, read_len;
__le16 fifo_status;
--
2.28.0
This is a note to let you know that I've just added the patch titled
iio:light:si1145: Fix timestamp alignment and prevent data leak.
to my staging git tree which can be found at
git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git
in the staging-testing branch.
The patch will show up in the next release of the linux-next tree
(usually sometime within the next 24 hours during the week.)
The patch will be merged to the staging-next branch sometime soon,
after it passes testing, and the merge window is open.
If you have any questions about this process, please let me know.
>From 0456ecf34d466261970e0ff92b2b9c78a4908637 Mon Sep 17 00:00:00 2001
From: Jonathan Cameron <Jonathan.Cameron(a)huawei.com>
Date: Wed, 22 Jul 2020 16:50:44 +0100
Subject: iio:light:si1145: 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 a 24 byte 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 array in the iio_priv() data with alignment
explicitly requested. This data is allocated with kzalloc so no
data can leak appart from previous readings.
Depending on the enabled channels, the location of the timestamp
can be at various aligned offsets through the buffer. As such we
any use of a structure to enforce this alignment would incorrectly
suggest a single location for the timestamp. Comments adjusted to
express this clearly in the code.
Fixes: ac45e57f1590 ("iio: light: Add driver for Silabs si1132, si1141/2/3 and si1145/6/7 ambient light, uv index and proximity sensors")
Reported-by: Lars-Peter Clausen <lars(a)metafoo.de>
Signed-off-by: Jonathan Cameron <Jonathan.Cameron(a)huawei.com>
Reviewed-by: Andy Shevchenko <andy.shevchenko(a)gmail.com>
Cc: Peter Meerwald-Stadler <pmeerw(a)pmeerw.net>
Cc: <Stable(a)vger.kernel.org>
Link: https://lore.kernel.org/r/20200722155103.979802-9-jic23@kernel.org
---
drivers/iio/light/si1145.c | 19 +++++++++++--------
1 file changed, 11 insertions(+), 8 deletions(-)
diff --git a/drivers/iio/light/si1145.c b/drivers/iio/light/si1145.c
index 155faaea8c72..087ef953041d 100644
--- a/drivers/iio/light/si1145.c
+++ b/drivers/iio/light/si1145.c
@@ -168,6 +168,7 @@ struct si1145_part_info {
* @part_info: Part information
* @trig: Pointer to iio trigger
* @meas_rate: Value of MEAS_RATE register. Only set in HW in auto mode
+ * @buffer: Used to pack data read from sensor.
*/
struct si1145_data {
struct i2c_client *client;
@@ -179,6 +180,14 @@ struct si1145_data {
bool autonomous;
struct iio_trigger *trig;
int meas_rate;
+ /*
+ * Ensure timestamp will be naturally aligned if present.
+ * Maximum buffer size (may be only partly used if not all
+ * channels are enabled):
+ * 6*2 bytes channels data + 4 bytes alignment +
+ * 8 bytes timestamp
+ */
+ u8 buffer[24] __aligned(8);
};
/*
@@ -440,12 +449,6 @@ static irqreturn_t si1145_trigger_handler(int irq, void *private)
struct iio_poll_func *pf = private;
struct iio_dev *indio_dev = pf->indio_dev;
struct si1145_data *data = iio_priv(indio_dev);
- /*
- * Maximum buffer size:
- * 6*2 bytes channels data + 4 bytes alignment +
- * 8 bytes timestamp
- */
- u8 buffer[24];
int i, j = 0;
int ret;
u8 irq_status = 0;
@@ -478,7 +481,7 @@ static irqreturn_t si1145_trigger_handler(int irq, void *private)
ret = i2c_smbus_read_i2c_block_data_or_emulated(
data->client, indio_dev->channels[i].address,
- sizeof(u16) * run, &buffer[j]);
+ sizeof(u16) * run, &data->buffer[j]);
if (ret < 0)
goto done;
j += run * sizeof(u16);
@@ -493,7 +496,7 @@ static irqreturn_t si1145_trigger_handler(int irq, void *private)
goto done;
}
- iio_push_to_buffers_with_timestamp(indio_dev, buffer,
+ iio_push_to_buffers_with_timestamp(indio_dev, data->buffer,
iio_get_time_ns(indio_dev));
done:
--
2.28.0
This is a note to let you know that I've just added the patch titled
iio:gyro:itg3200: Fix timestamp alignment and prevent data leak.
to my staging git tree which can be found at
git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git
in the staging-testing branch.
The patch will show up in the next release of the linux-next tree
(usually sometime within the next 24 hours during the week.)
The patch will be merged to the staging-next branch sometime soon,
after it passes testing, and the merge window is open.
If you have any questions about this process, please let me know.
>From 10ab7cfd5522f0041028556dac864a003e158556 Mon Sep 17 00:00:00 2001
From: Jonathan Cameron <Jonathan.Cameron(a)huawei.com>
Date: Wed, 22 Jul 2020 16:50:41 +0100
Subject: iio:gyro:itg3200: 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 a 16 byte array of smaller elements on the stack.
This is fixed by using an explicit c structure. As there are no
holes in the structure, there is no possiblity of data leakage
in this case.
The explicit alignment of ts is not strictly necessary but potentially
makes the code slightly less fragile. It also removes the possibility
of this being cut and paste into another driver where the alignment
isn't already true.
Fixes: 36e0371e7764 ("iio:itg3200: Use iio_push_to_buffers_with_timestamp()")
Reported-by: Lars-Peter Clausen <lars(a)metafoo.de>
Signed-off-by: Jonathan Cameron <Jonathan.Cameron(a)huawei.com>
Reviewed-by: Andy Shevchenko <andy.shevchenko(a)gmail.com>
Cc: <Stable(a)vger.kernel.org>
Link: https://lore.kernel.org/r/20200722155103.979802-6-jic23@kernel.org
---
drivers/iio/gyro/itg3200_buffer.c | 15 +++++++++++----
1 file changed, 11 insertions(+), 4 deletions(-)
diff --git a/drivers/iio/gyro/itg3200_buffer.c b/drivers/iio/gyro/itg3200_buffer.c
index d3fbe9d86467..1c3c1bd53374 100644
--- a/drivers/iio/gyro/itg3200_buffer.c
+++ b/drivers/iio/gyro/itg3200_buffer.c
@@ -46,13 +46,20 @@ static irqreturn_t itg3200_trigger_handler(int irq, void *p)
struct iio_poll_func *pf = p;
struct iio_dev *indio_dev = pf->indio_dev;
struct itg3200 *st = iio_priv(indio_dev);
- __be16 buf[ITG3200_SCAN_ELEMENTS + sizeof(s64)/sizeof(u16)];
-
- int ret = itg3200_read_all_channels(st->i2c, buf);
+ /*
+ * Ensure correct alignment and padding including for the
+ * timestamp that may be inserted.
+ */
+ struct {
+ __be16 buf[ITG3200_SCAN_ELEMENTS];
+ s64 ts __aligned(8);
+ } scan;
+
+ int ret = itg3200_read_all_channels(st->i2c, scan.buf);
if (ret < 0)
goto error_ret;
- iio_push_to_buffers_with_timestamp(indio_dev, buf, pf->timestamp);
+ iio_push_to_buffers_with_timestamp(indio_dev, &scan, pf->timestamp);
iio_trigger_notify_done(indio_dev->trig);
--
2.28.0
This is a note to let you know that I've just added the patch titled
iio:imu:st_lsm6dsx: check st_lsm6dsx_shub_read_output return
to my staging git tree which can be found at
git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git
in the staging-testing branch.
The patch will show up in the next release of the linux-next tree
(usually sometime within the next 24 hours during the week.)
The patch will be merged to the staging-next branch sometime soon,
after it passes testing, and the merge window is open.
If you have any questions about this process, please let me know.
>From f71e41e23e129640f620b65fc362a6da02580310 Mon Sep 17 00:00:00 2001
From: Tom Rix <trix(a)redhat.com>
Date: Sun, 9 Aug 2020 10:55:51 -0700
Subject: iio:imu:st_lsm6dsx: check st_lsm6dsx_shub_read_output return
Potential error return is not checked. This can lead to use
of undefined data.
Detected by clang static analysis.
st_lsm6dsx_shub.c:540:8: warning: Assigned value is garbage or undefined
*val = (s16)le16_to_cpu(*((__le16 *)data));
^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Fixes: c91c1c844ebd ("iio: imu: st_lsm6dsx: add i2c embedded controller support")
Signed-off-by: Tom Rix <trix(a)redhat.com
Cc: <Stable(a)vger.kernel.org>
Reviewed-by: Andy Shevchenko <andy.shevchenko(a)gmail.com>
Link: https://lore.kernel.org/r/20200809175551.6794-1-trix@redhat.com
Signed-off-by: Jonathan Cameron <Jonathan.Cameron(a)huawei.com>
---
drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_shub.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_shub.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_shub.c
index ed83471dc7dd..8c8d8870ca07 100644
--- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_shub.c
+++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_shub.c
@@ -313,6 +313,8 @@ st_lsm6dsx_shub_read(struct st_lsm6dsx_sensor *sensor, u8 addr,
err = st_lsm6dsx_shub_read_output(hw, data,
len & ST_LS6DSX_READ_OP_MASK);
+ if (err < 0)
+ return err;
st_lsm6dsx_shub_master_enable(sensor, false);
--
2.28.0
Dear Sir/Ma
Compliments of the season, I am Tom Egan,loan consultant Skyline
Consult. we actually assist our client to provide financial
facilities for the business and long projects. We liase with our
big investors here in the middle east to extend this facilities
to trusted and verified client,They are ready to finance your
project.
We offer flexible financial packages for various projects by
passing the usual and formal procedures.We are willing to provide
financing for up to 1 Billion USD and above depending on the
nature of the business. We are currently providing Loan funds at
a wonderful rate without any physical collateral.
We would like you to provide us with a comprehensive business
plan for our team of investment expert review.
* Business Start-up
* Business Acquisition
* Business Expansion
* Commercial Real Estate purchase
* Contract Execution, etc
Do reply to if you have need for our investment facility for
further discussion
Best Regards,
Tom Egan
Skyline Consult
Kuwait Tower, No. 1 Kuwait Street,
Bur Dubai,
United Arab Emirates.
Tel/Fax: +971 4 228 2035
Email: tmmegan9(a)gmail.com