Re: [PATCH] ad7923: fix array out of bounds in ad7923_update_scan_mode()

[Jonathan Cameron <jic23@kernel.org>]

On Mon, 28 Oct 2024 14:23:57 +0000
Zicheng Qu <quzicheng@huawei.com> 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@vger.kernel.org>
> Signed-off-by: Zicheng Qu <quzicheng@huawei.com>
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?

Jonathan


> ---
>  drivers/iio/adc/ad7923.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/iio/adc/ad7923.c b/drivers/iio/adc/ad7923.c
> index 09680015a7ab..82b341709a64 100644
> --- a/drivers/iio/adc/ad7923.c
> +++ b/drivers/iio/adc/ad7923.c
> @@ -170,6 +170,8 @@ static int ad7923_update_scan_mode(struct iio_dev *indio_dev,
>  	 * skip that one.
>  	 */
>  	for_each_set_bit(i, active_scan_mask, indio_dev->num_channels - 1) {
> +		if (len >= 4)
> +			return -EINVAL;
>  		cmd = AD7923_WRITE_CR | AD7923_CHANNEL_WRITE(i) |
>  			AD7923_SEQUENCE_WRITE(AD7923_SEQUENCE_OFF) |
>  			st->settings;