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

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

[Salah Triki]

In ad4030_set_avg_frame_len(), the logarithm is calculated before input
validation. Passing zero or negative values leads to an undefined result
from ilog2().

Validate that the input is strictly positive prior to computing its
logarithm to ensure only valid values are processed.

Fixes: 949abd1ca5a4 ("iio: adc: ad4030: add averaging support")
Assisted-by: Claude:claude-sonnet-5
Signed-off-by: Salah Triki <salah.triki@gmail.com>
---
Changes in v4:
  - Reordered local variables to enforce strict reversed Christmas tree layout
  (per Andy Shevchenko).
  - Rephrased commit log in plain English without C-specific terms
  (per Andy Shevchenko).
  - Converted AI assistance note into a formal Assisted-by tag
  (per Andy Shevchenko).

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 in v2:
  - 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..17b0ca7f1cce 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 last_avg_idx = ARRAY_SIZE(ad4030_average_modes) - 1;
+	unsigned int avg_log2;
 	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 v4] iio: adc: ad4030: fix invalid oversampling_ratio validation

[Andy Shevchenko]

On Tue, Sep 01, 2026 at 11:42:43AM +0100, Salah Triki wrote:
> In ad4030_set_avg_frame_len(), the logarithm is calculated before input
> validation. Passing zero or negative values leads to an undefined result
> from ilog2().
> 
> Validate that the input is strictly positive prior to computing its
> logarithm to ensure only valid values are processed.

...

> -	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;

I still find '<= 0' confusing. Why not check against 1?
And I would rather see an additional comment.

What about

	/* Reject unsupported modes */
	if (avg_val > ad4030_average_modes[last_avg_idx])
		return -EINVAL;

	/* Avoid invalid values for logarithm since it's undefined */
	if (avg_val < 1)
// In this case, when condition is split, if (avg_val <= 0) is also possible
		return -EINVAL;

> +	avg_log2 = ilog2(avg_val);

-- 
With Best Regards,
Andy Shevchenko

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

[Nuno Sá]

On Tue, Sep 01, 2026 at 02:08:44PM +0300, Andy Shevchenko wrote:
> On Tue, Sep 01, 2026 at 11:42:43AM +0100, Salah Triki wrote:
> > In ad4030_set_avg_frame_len(), the logarithm is calculated before input
> > validation. Passing zero or negative values leads to an undefined result
> > from ilog2().
> > 
> > Validate that the input is strictly positive prior to computing its
> > logarithm to ensure only valid values are processed.
> 
> ...
> 
> > -	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;
> 
> I still find '<= 0' confusing. Why not check against 1?
> And I would rather see an additional comment.

I realize here it does not matter much but I tend to agree that the correct
check is against 1

> 
> What about
> 
> 	/* Reject unsupported modes */
> 	if (avg_val > ad4030_average_modes[last_avg_idx])
> 		return -EINVAL;
> 
> 	/* Avoid invalid values for logarithm since it's undefined */
> 	if (avg_val < 1)
> // In this case, when condition is split, if (avg_val <= 0) is also possible
> 		return -EINVAL;
> 

Also tend to agree with the above but no strong feeling.

- Nuno Sá

> > +	avg_log2 = ilog2(avg_val);
> 
> -- 
> With Best Regards,
> Andy Shevchenko
> 
>