[PATCH v3] iio: adc: ad4030: fix invalid oversampling_ratio validation

[PATCH v3] iio: adc: ad4030: fix invalid oversampling_ratio validation

[Salah Triki]

ad4030_set_avg_frame_len() computes avg_log2 = ilog2(avg_val) before
validating avg_val. The existing check (avg_val < 0) allows avg_val == 0
to pass through, resulting in ilog2(0) being called with undefined/garbage
behavior.

Reject non-positive values (avg_val <= 0) before computing log2.

This issue was identified with assistance from Claude AI and manually
verified against the code.

Fixes: 949abd1ca5a4 ("iio: adc: ad4030: add averaging support")
Signed-off-by: Salah Triki <salah.triki@gmail.com>
---
Changes in v3:
   - Dropped the !is_power_of_2() check to preserve standard IIO attribute
   rounding behavior, per feedback from David Lechner and Jonathan Cameron.

Changes since v1:
   - Added note stating the issue was identified with assistance from
   Claude AI and verified manually.
   - Removed initialization of avg_log2 at declaration.

 drivers/iio/adc/ad4030.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/iio/adc/ad4030.c b/drivers/iio/adc/ad4030.c
index 9c5f19321e3b..5a5e69dfb5d4 100644
--- a/drivers/iio/adc/ad4030.c
+++ b/drivers/iio/adc/ad4030.c
@@ -746,14 +746,16 @@ static int ad4030_set_chan_calibbias(struct iio_dev *indio_dev,
 static int ad4030_set_avg_frame_len(struct iio_dev *dev, int avg_val)
 {
 	struct ad4030_state *st = iio_priv(dev);
-	unsigned int avg_log2 = ilog2(avg_val);
+	unsigned int avg_log2;
 	unsigned int last_avg_idx = ARRAY_SIZE(ad4030_average_modes) - 1;
 	int freq_hz;
 	int ret;
 
-	if (avg_val < 0 || avg_val > ad4030_average_modes[last_avg_idx])
+	if (avg_val <= 0 || avg_val > ad4030_average_modes[last_avg_idx])
 		return -EINVAL;
 
+	avg_log2 = ilog2(avg_val);
+
 	if (st->offload_trigger) {
 		/*
 		 * The sample averaging and sampling frequency configurations
-- 
2.43.0

Re: [PATCH v3] iio: adc: ad4030: fix invalid oversampling_ratio validation

[Andy Shevchenko]

On Mon, Aug 24, 2026 at 06:10:26AM +0100, Salah Triki wrote:
> ad4030_set_avg_frame_len() computes avg_log2 = ilog2(avg_val) before
> validating avg_val. The existing check (avg_val < 0) allows avg_val == 0

Try to write the sentences straight to the point using plain English,
Avoid using C language as substitute of plain English.

> to pass through, resulting in ilog2(0) being called with undefined/garbage
> behavior.
> 
> Reject non-positive values (avg_val <= 0) before computing log2.
> 
> This issue was identified with assistance from Claude AI and manually
> verified against the code.

> Changes since v1:
>    - Added note stating the issue was identified with assistance from
>    Claude AI and verified manually.

Assisted-by?

...

>  static int ad4030_set_avg_frame_len(struct iio_dev *dev, int avg_val)
>  {
>  	struct ad4030_state *st = iio_priv(dev);
> -	unsigned int avg_log2 = ilog2(avg_val);
> +	unsigned int avg_log2;

Have you read my comments to the previous version?

>  	unsigned int last_avg_idx = ARRAY_SIZE(ad4030_average_modes) - 1;
>  	int freq_hz;
>  	int ret;
>  
> -	if (avg_val < 0 || avg_val > ad4030_average_modes[last_avg_idx])
> +	if (avg_val <= 0 || avg_val > ad4030_average_modes[last_avg_idx])
>  		return -EINVAL;

Ditto.


-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3] iio: adc: ad4030: fix invalid oversampling_ratio validation

[Salah Triki]

Hi Andy,

Sorry for the confusion - my previous v3 email crossed paths with yours.
Regarding is_power_of_2(avg_val): following feedback from David and 
Jonathan, we agreed to keep only the check for non-positive values
(avg_val <= 0) to avoid the invalid ilog2(0) call. Checking for non-powers
of two was dropped to preserve standard IIO attribute rounding behavior.

I will sent v4 addressing all your formatting and style requirements.

Thanks for your patience and thorough review.

Best regards,
Salah Triki

On Mon, Aug 24, 2026 at 05:54:31PM +0300, Andy Shevchenko wrote:
> On Mon, Aug 24, 2026 at 06:10:26AM +0100, Salah Triki wrote:
> > ad4030_set_avg_frame_len() computes avg_log2 = ilog2(avg_val) before
> > validating avg_val. The existing check (avg_val < 0) allows avg_val == 0
> 
> Try to write the sentences straight to the point using plain English,
> Avoid using C language as substitute of plain English.
> 
> > to pass through, resulting in ilog2(0) being called with undefined/garbage
> > behavior.
> > 
> > Reject non-positive values (avg_val <= 0) before computing log2.
> > 
> > This issue was identified with assistance from Claude AI and manually
> > verified against the code.
> 
> > Changes since v1:
> >    - Added note stating the issue was identified with assistance from
> >    Claude AI and verified manually.
> 
> Assisted-by?
> 
> ...
> 
> >  static int ad4030_set_avg_frame_len(struct iio_dev *dev, int avg_val)
> >  {
> >  	struct ad4030_state *st = iio_priv(dev);
> > -	unsigned int avg_log2 = ilog2(avg_val);
> > +	unsigned int avg_log2;
> 
> Have you read my comments to the previous version?
> 
> >  	unsigned int last_avg_idx = ARRAY_SIZE(ad4030_average_modes) - 1;
> >  	int freq_hz;
> >  	int ret;
> >  
> > -	if (avg_val < 0 || avg_val > ad4030_average_modes[last_avg_idx])
> > +	if (avg_val <= 0 || avg_val > ad4030_average_modes[last_avg_idx])
> >  		return -EINVAL;
> 
> Ditto.
> 
> 
> -- 
> With Best Regards,
> Andy Shevchenko
> 
>