On Mon, 2024-10-28 at 20:50 +0000, Jonathan Cameron wrote:
> On Mon, 28 Oct 2024 14:23:57 +0000
> Zicheng Qu <quzicheng@xxxxxxxxxx> wrote:
>
> > In the ad7923_update_scan_mode() , the variable len may exceed the length
> > of the st->tx_buf array, leading to an array overflow issue. The final
> > value of len depends on active_scan_mask (an unsigned long) and
> > num_channels-1 (an integer), with an upper limit of num_channels-1. In
> > the ad7923_probe() function, when assigning to indio_dev->num_channels,
> > its size is not checked. Therefore, in ad7923_update_scan_mode(), since
> > active_scan_mask is an unsigned long and num_channels has no set upper
> > limit, an overflow might occur.
> >
> > Fixes: 0eac259db28f ("IIO ADC support for AD7923")
> > Cc: <stable@xxxxxxxxxxxxxxx>
> > Signed-off-by: Zicheng Qu <quzicheng@xxxxxxxxxx>
> Thanks.
> This looks to be a valid bug but a wrong fix. Fairly sure the number of
> channels
> supported has changed at somepoint (probably with addition of more parts)
> and the size of tx has not increased to match.
>
> Nuno, could you take a look?
Hi Jonathan,
Yes, the fix seems to be the wrong one (and incomplete). In
commit 851644a60d20 ("iio: adc: ad7923: Add support for the
ad7908/ad7918/ad7928")
devices with 8 channels were added but the buffers not updated. Then, you
actually partially fixed the problem in
commit 01fcf129f61b ("iio: adc: ad7923: Fix undersized rx buffer.") but only for
the rx buffer.
So to me this is the right fix (if nothing else missed):
diff --git a/drivers/iio/adc/ad7923.c b/drivers/iio/adc/ad7923.c
index 09680015a7ab..acc44cb34f82 100644
--- a/drivers/iio/adc/ad7923.c
+++ b/drivers/iio/adc/ad7923.c
@@ -48,7 +48,7 @@
struct ad7923_state {
struct spi_device *spi;
- struct spi_transfer ring_xfer[5];
+ struct spi_transfer ring_xfer[9];
struct spi_transfer scan_single_xfer[2];
struct spi_message ring_msg;
struct spi_message scan_single_msg;
@@ -64,7 +64,7 @@ struct ad7923_state {
* Length = 8 channels + 4 extra for 8 byte timestamp
*/
__be16 rx_buf[12] __aligned(IIO_DMA_MINALIGN);
- __be16 tx_buf[4];
+ __be16 tx_buf[8];
};
- Nuno Sá
