linux-kernel - Re: [PATCH] iio: addac: ad74115: Fix use of uninitialized variable rate

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] [day] [month] [year] [list]

Message-ID: <383030aa-2312-4f11-ba80-c8dd54fc9010@gmail.com>
Date: Thu, 17 Apr 2025 11:13:19 +0530
From: Purva Yeshi <purvayeshi550@...il.com>
To: Nuno Sá <noname.nuno@...il.com>,
 David Lechner <dlechner@...libre.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 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@...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á
>>
>> 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