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

[Jonathan Cameron]

On Sun, 23 Aug 2026 13:29:32 -0500
David Lechner <dlechner@xxxxxxxxxxxx> wrote:

> On 8/22/26 11:52 PM, Salah Triki wrote:
> > ad4030_set_avg_frame_len() computes avg_log2 = ilog2(avg_val) before
> > validating avg_val, and the subsequent range check only rejects
> > negative values or values above the maximum supported OSR. It does
> > not reject avg_val == 0, nor values that are not exact powers of 2.
> > 
> > - avg_val == 0 passes the check (0 is not < 0 and not > max), so
> >   ilog2(0) is called with an undefined/garbage result.
> > 
> > - Non-power-of-2 values (e.g. avg_val == 3) also pass the check and
> >   silently get rounded down by ilog2() to the nearest lower power of
> >   2, so userspace can write a value to the oversampling_ratio sysfs
> >   attribute that does not match what actually gets programmed into
> >   hardware, without any error being reported.
> > 
> > Only powers of 2 in [1, 65536] are valid OSR values, as listed in
> > ad4030_average_modes[]. Validate avg_val fully before computing its
> > log2, using is_power_of_2() and requiring avg_val > 0.  
> 
> There is no rule in IIO that says we can't round values when writing
> attributes. Users are expected to read the attribute after writing to
> see what actually took effect. So this patch doesn't seem justified.
> 
> > 
> > This issue was identified with assistance from Claude AI and manually
> > verified against the code.
> > 
> > Fixes: 949abd1ca5a4 ("iio: adc: ad4030: add averaging support")  
> 
> Actually, this could break existing users that depend on the current
> behavior. It certainly doesn't fix anything.
The zero check may make sense even if the rest do not.

Jonathan

> 
> > Signed-off-by: Salah Triki <salah.triki@xxxxxxxxx>
> > ---
> > 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(-)
> >