linux-kernel - Re: [PATCH] ad7923: fix array out of bounds in ad7923_update_scan

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20241028205004.2298af74@jic23-huawei>
Date: Mon, 28 Oct 2024 20:50:14 +0000
From: Jonathan Cameron <jic23@...nel.org>
To: Zicheng Qu <quzicheng@...wei.com>
Cc: <lars@...afoo.de>, <Michael.Hennerich@...log.com>,
 <patrick.vasseur@....fr>, <christophe.leroy@....fr>,
 <linux-iio@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
 <tanghui20@...wei.com>, <zhangqiao22@...wei.com>,
 <judy.chenhui@...wei.com>, Nuno Sá <noname.nuno@...il.com>,
 Nuno Sá <nuno.sa@...log.com>
Subject: Re: [PATCH] ad7923: fix array out of bounds in
 ad7923_update_scan_mode()

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