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

[Andy Shevchenko <andriy.shevchenko@intel.com>]

On Sun, Aug 23, 2026 at 05:52:05AM +0100, 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.

No need to repeat in the commit message what we can see in the code.
Use plain English to write the problem statement, the solution approach
and what might happen if patch is not applied.

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

Assisted-by?

> Fixes: 949abd1ca5a4 ("iio: adc: ad4030: add averaging support")
> Signed-off-by: Salah Triki <salah.triki@gmail.com>

...

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

Reorder (only the line you touched) to follow the reversed xmas tree ordering.

...

> -	if (avg_val < 0 || avg_val > ad4030_average_modes[last_avg_idx])
> +	if (avg_val <= 0 || avg_val > ad4030_average_modes[last_avg_idx] || !is_power_of_2(avg_val))
>  		return -EINVAL;

Split it, the

	if (avg_val == 0 || !is_power_of_2(avg_val))
		return -EINVAL;

is idiomatic as the 0-check required for is_power_of_2(). Also it puts the line
in the limits.

-- 
With Best Regards,
Andy Shevchenko