mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: David Lechner <dlechner@baylibre.com>
To: "Salah Triki" <salah.triki@gmail.com>,
	"Michael Hennerich" <michael.hennerich@analog.com>,
	"Nuno Sá" <nuno.sa@analog.com>,
	"Jonathan Cameron" <jic23@kernel.org>,
	"Andy Shevchenko" <andy@kernel.org>,
	linux-iio@vger.kernel.org, linux@analog.com,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2] iio: adc: ad4030: fix invalid oversampling_ratio validation
Date: Sun, 23 Aug 2026 13:29:32 -0500	[thread overview]
Message-ID: <0b03fc0f-8032-47b2-ac97-f2d93c1c5bdf@baylibre.com> (raw)
In-Reply-To: <20260823045205.25554-1-salah.triki@gmail.com>

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.

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

  reply	other threads:[~2026-08-23 18:29 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-23  4:52 Salah Triki
2026-08-23 18:29 ` David Lechner [this message]
2026-08-23 21:31   ` Jonathan Cameron
2026-08-24  9:04 ` Andy Shevchenko

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=0b03fc0f-8032-47b2-ac97-f2d93c1c5bdf@baylibre.com \
    --to=dlechner@baylibre.com \
    --cc=andy@kernel.org \
    --cc=jic23@kernel.org \
    --cc=linux-iio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@analog.com \
    --cc=michael.hennerich@analog.com \
    --cc=nuno.sa@analog.com \
    --cc=salah.triki@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®