| Message ID | 20211215232321.2069314-1-keescook@chromium.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | iio: addac: Do not reference negative array offsets | expand |
> -----Original Message----- > From: Kees Cook <keescook@chromium.org> > Sent: Thursday, December 16, 2021 1:23 AM > To: Lars-Peter Clausen <lars@metafoo.de> > Cc: Kees Cook <keescook@chromium.org>; Hennerich, Michael > <Michael.Hennerich@analog.com>; Tanislav, Cosmin > <Cosmin.Tanislav@analog.com>; Jonathan Cameron <jic23@kernel.org>; > Linus Walleij <linus.walleij@linaro.org>; linux-kernel@vger.kernel.org; linux- > iio@vger.kernel.org; linux-hardening@vger.kernel.org > Subject: [PATCH] iio: addac: Do not reference negative array offsets > > [External] > > Instead of aiming rx_buf at an invalid array-boundary-crossing location, > just skip the first assignment. Seen when building with -Warray-bounds: > > drivers/iio/addac/ad74413r.c: In function 'ad74413r_update_scan_mode': > drivers/iio/addac/ad74413r.c:843:22: warning: array subscript -4 is below > array bounds of 'u8[16]' { aka 'unsigned char[16]'} [-Warray-bounds] > 843 | u8 *rx_buf = &st->adc_samples_buf.rx_buf[-1 * > AD74413R_FRAME_SIZE]; > | > ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > drivers/iio/addac/ad74413r.c:84:20: note: while referencing 'rx_buf' > 84 | u8 rx_buf[AD74413R_FRAME_SIZE * > AD74413R_CHANNEL_MAX]; > | ^~~~~~ > > Fixes: fea251b6a5db ("iio: addac: add AD74413R driver") > Signed-off-by: Kees Cook <keescook@chromium.org> > --- > drivers/iio/addac/ad74413r.c | 10 ++++++---- > 1 file changed, 6 insertions(+), 4 deletions(-) > > diff --git a/drivers/iio/addac/ad74413r.c b/drivers/iio/addac/ad74413r.c > index cbd9aa9b399a..b0a6d8ee5133 100644 > --- a/drivers/iio/addac/ad74413r.c > +++ b/drivers/iio/addac/ad74413r.c > @@ -840,7 +840,7 @@ static int ad74413r_update_scan_mode(struct iio_dev > *indio_dev, > { > struct ad74413r_state *st = iio_priv(indio_dev); > struct spi_transfer *xfer = st->adc_samples_xfer; > - u8 *rx_buf = &st->adc_samples_buf.rx_buf[-1 * > AD74413R_FRAME_SIZE]; > + u8 *rx_buf = st->adc_samples_buf.rx_buf; > u8 *tx_buf = st->adc_samples_tx_buf; > unsigned int channel; > int ret; > @@ -877,9 +877,8 @@ static int ad74413r_update_scan_mode(struct iio_dev > *indio_dev, > if (ret) > goto out; > > - st->adc_active_channels++; > > - if (xfer == st->adc_samples_xfer) > + if (xfer == st->adc_samples_xfer || st->adc_active_channels > == 0) You can probably keep only one of the checks. Both xfer and adc_active_channels will be incremented after your changes anyway. > xfer->rx_buf = NULL; > else > xfer->rx_buf = rx_buf; > @@ -896,7 +895,10 @@ static int ad74413r_update_scan_mode(struct > iio_dev *indio_dev, > > xfer++; > tx_buf += AD74413R_FRAME_SIZE; > - rx_buf += AD74413R_FRAME_SIZE; > + if (st->adc_active_channels) > + rx_buf += AD74413R_FRAME_SIZE; > + > + st->adc_active_channels++; > } > > xfer->rx_buf = rx_buf; > -- > 2.30.2
diff --git a/drivers/iio/addac/ad74413r.c b/drivers/iio/addac/ad74413r.c index cbd9aa9b399a..b0a6d8ee5133 100644 --- a/drivers/iio/addac/ad74413r.c +++ b/drivers/iio/addac/ad74413r.c @@ -840,7 +840,7 @@ static int ad74413r_update_scan_mode(struct iio_dev *indio_dev, { struct ad74413r_state *st = iio_priv(indio_dev); struct spi_transfer *xfer = st->adc_samples_xfer; - u8 *rx_buf = &st->adc_samples_buf.rx_buf[-1 * AD74413R_FRAME_SIZE]; + u8 *rx_buf = st->adc_samples_buf.rx_buf; u8 *tx_buf = st->adc_samples_tx_buf; unsigned int channel; int ret; @@ -877,9 +877,8 @@ static int ad74413r_update_scan_mode(struct iio_dev *indio_dev, if (ret) goto out; - st->adc_active_channels++; - if (xfer == st->adc_samples_xfer) + if (xfer == st->adc_samples_xfer || st->adc_active_channels == 0) xfer->rx_buf = NULL; else xfer->rx_buf = rx_buf; @@ -896,7 +895,10 @@ static int ad74413r_update_scan_mode(struct iio_dev *indio_dev, xfer++; tx_buf += AD74413R_FRAME_SIZE; - rx_buf += AD74413R_FRAME_SIZE; + if (st->adc_active_channels) + rx_buf += AD74413R_FRAME_SIZE; + + st->adc_active_channels++; } xfer->rx_buf = rx_buf;
Instead of aiming rx_buf at an invalid array-boundary-crossing location, just skip the first assignment. Seen when building with -Warray-bounds: drivers/iio/addac/ad74413r.c: In function 'ad74413r_update_scan_mode': drivers/iio/addac/ad74413r.c:843:22: warning: array subscript -4 is below array bounds of 'u8[16]' { aka 'unsigned char[16]'} [-Warray-bounds] 843 | u8 *rx_buf = &st->adc_samples_buf.rx_buf[-1 * AD74413R_FRAME_SIZE]; | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ drivers/iio/addac/ad74413r.c:84:20: note: while referencing 'rx_buf' 84 | u8 rx_buf[AD74413R_FRAME_SIZE * AD74413R_CHANNEL_MAX]; | ^~~~~~ Fixes: fea251b6a5db ("iio: addac: add AD74413R driver") Signed-off-by: Kees Cook <keescook@chromium.org> --- drivers/iio/addac/ad74413r.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-)