lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <10a9dd5cdf55b6a9845fb9543cdef5f2251ffa6a.camel@gmail.com>
Date: Fri, 11 Apr 2025 06:49:24 +0100
From: Nuno Sá <noname.nuno@...il.com>
To: David Lechner <dlechner@...libre.com>, Purva Yeshi
 <purvayeshi550@...il.com>,  cosmin.tanislav@...log.com, lars@...afoo.de,
 Michael.Hennerich@...log.com,  jic23@...nel.org
Cc: linux-iio@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] iio: addac: ad74115: Fix use of uninitialized variable
 rate

On Thu, 2025-04-10 at 09:51 -0500, David Lechner wrote:
> On 4/9/25 3:29 PM, Purva Yeshi wrote:
> > Fix Smatch-detected error:
> > drivers/iio/addac/ad74115.c:823 _ad74115_get_adc_code() error:
> > uninitialized symbol 'rate'.
> > 
> > The variable rate was declared but not given any value before being used
> > in a division. If the code reached that point without setting rate, it
> > would cause unpredictable behavior.
> > 
> > Declare and initialize 'rate' to zero inside the 'else' block where it is
> > used. This ensures 'rate' is always initialized before being passed to
> > DIV_ROUND_CLOSEST.
> > 
> > Signed-off-by: Purva Yeshi <purvayeshi550@...il.com>
> > ---
> >  drivers/iio/addac/ad74115.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/iio/addac/ad74115.c b/drivers/iio/addac/ad74115.c
> > index a7e480f2472d..26770c68e5fa 100644
> > --- a/drivers/iio/addac/ad74115.c
> > +++ b/drivers/iio/addac/ad74115.c
> > @@ -814,7 +814,7 @@ static int _ad74115_get_adc_code(struct ad74115_state *st,
> >  			return -ETIMEDOUT;
> >  	} else {
> >  		unsigned int regval, wait_time;
> > -		int rate;
> > +		int rate = 0;
> >  
> >  		ret = ad74115_get_adc_rate(st, channel, &rate);
> >  		if (ret < 0)
> 
> I don't see how rate could be used uninitialized since we are
> returning the error if ad74115_get_adc_rate() fails.
> 
> Also, initializing to 0 would then cause a divide by 0 error
> if that value was actually used later in the code.
> 

Agreed... A better check could actually be (in ad74115_get_adc_rate()):


if (i >= ARRAY_SIZE(ad74115_get_adc_rate))
    return -EIO;

Kind of a paranoid check but just making sure a faulty chip does not lead to an out
of bounds access.

- Nuno Sá 

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ