[PATCH] iio: addac: ad74115: Fix use of uninitialized variable rate

Purva Yeshi posted 1 patch 10 months ago
drivers/iio/addac/ad74115.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
[PATCH] iio: addac: ad74115: Fix use of uninitialized variable rate
Posted by Purva Yeshi 10 months ago
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@gmail.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)
-- 
2.34.1
Re: [PATCH] iio: addac: ad74115: Fix use of uninitialized variable rate
Posted by David Lechner 10 months ago
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@gmail.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.
Re: [PATCH] iio: addac: ad74115: Fix use of uninitialized variable rate
Posted by Nuno Sá 10 months ago
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@gmail.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á 
Re: [PATCH] iio: addac: ad74115: Fix use of uninitialized variable rate
Posted by Purva Yeshi 10 months ago
On 11/04/25 11:19, Nuno Sá wrote:
> 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@gmail.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á

Hi Nuno,

Thank you for your suggestion regarding the paranoid check.

However, ad74115_get_adc_rate is a function, not an array, pointer, or 
vector. Therefore, using ARRAY_SIZE on it results in a compilation error.

I believe the intended check was:

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


This ensures that the index i does not exceed the bounds of the 
ad74115_adc_conv_rate_tbl array, preventing potential out-of-bounds access.

This check prevents potential out-of-bounds access, it does not address 
the Smatch warning about the uninitialized variable 'rate'. Smatch may 
still flag 'rate' as potentially uninitialized if it cannot determine 
that ad74115_get_adc_rate() always initializes it before use.

Best regards,
Purva
Re: [PATCH] iio: addac: ad74115: Fix use of uninitialized variable rate
Posted by Nuno Sá 10 months ago
On Fri, 2025-04-11 at 14:39 +0530, Purva Yeshi wrote:
> On 11/04/25 11:19, Nuno Sá wrote:
> > 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@gmail.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á
> 
> Hi Nuno,
> 
> Thank you for your suggestion regarding the paranoid check.
> 
> However, ad74115_get_adc_rate is a function, not an array, pointer, or 
> vector. Therefore, using ARRAY_SIZE on it results in a compilation error.
> 
> I believe the intended check was:
> 
> if (i >= ARRAY_SIZE(ad74115_adc_conv_rate_tbl))
>      return -EIO;
> 

Oh yes, bad copy-paste...

> 
> This ensures that the index i does not exceed the bounds of the 
> ad74115_adc_conv_rate_tbl array, preventing potential out-of-bounds access.
> 
> This check prevents potential out-of-bounds access, it does not address 
> the Smatch warning about the uninitialized variable 'rate'. Smatch may 
> still flag 'rate' as potentially uninitialized if it cannot determine 
> that ad74115_get_adc_rate() always initializes it before use.
> 

Well, as said, this is a false positive...

- Nuno Sá
Re: [PATCH] iio: addac: ad74115: Fix use of uninitialized variable rate
Posted by Purva Yeshi 9 months, 3 weeks ago
On 11/04/25 17:47, Nuno Sá wrote:
> On Fri, 2025-04-11 at 14:39 +0530, Purva Yeshi wrote:
>> On 11/04/25 11:19, Nuno Sá wrote:
>>> 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@gmail.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á
>>
>> Hi Nuno,
>>
>> Thank you for your suggestion regarding the paranoid check.
>>
>> However, ad74115_get_adc_rate is a function, not an array, pointer, or
>> vector. Therefore, using ARRAY_SIZE on it results in a compilation error.
>>
>> I believe the intended check was:
>>
>> if (i >= ARRAY_SIZE(ad74115_adc_conv_rate_tbl))
>>       return -EIO;
>>
> 
> Oh yes, bad copy-paste...
> 
>>
>> This ensures that the index i does not exceed the bounds of the
>> ad74115_adc_conv_rate_tbl array, preventing potential out-of-bounds access.
>>
>> This check prevents potential out-of-bounds access, it does not address
>> the Smatch warning about the uninitialized variable 'rate'. Smatch may
>> still flag 'rate' as potentially uninitialized if it cannot determine
>> that ad74115_get_adc_rate() always initializes it before use.
>>
> 
> Well, as said, this is a false positive...
> 
> - Nuno Sá
> 

Hi Nuno,

Thank you for the review. I'll drop the patch.

Best regards,
Purva

Re: [PATCH] iio: addac: ad74115: Fix use of uninitialized variable rate
Posted by Purva Yeshi 10 months ago
On 10/04/25 20:21, 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@gmail.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.

Hi,

Thank you for the review and explanation.

Understood — since there's a risk of misuse later (like divide-by-zero), 
it's best to leave it as is.

Best regards,
Purva

> 
>