linux-kernel - Re: [PATCH] iio: Fix uninitialized symbol 'ret'

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: <d13887a9-a321-4329-82e5-34afd33e0300@gmail.com>
Date: Sat, 2 Nov 2024 11:46:14 +0200
From: Matti Vaittinen <mazziesaccount@...il.com>
To: Jonathan Cameron <jic23@...nel.org>
Cc: Zicheng Qu <quzicheng@...wei.com>, lars@...afoo.de,
 linux-iio@...r.kernel.org, linux-kernel@...r.kernel.org,
 tanghui20@...wei.com, zhangqiao22@...wei.com, judy.chenhui@...wei.com
Subject: Re: [PATCH] iio: Fix uninitialized symbol 'ret'

On 31/10/2024 23:47, Jonathan Cameron wrote:
> On Thu, 31 Oct 2024 09:13:16 +0200
> Matti Vaittinen <mazziesaccount@...il.com> wrote:
> 
>> Hi Zicheng,
>>
>> Thanks for the patch.
>>
>> On 31/10/2024 03:45, Zicheng Qu wrote:
>>> Initialize the variable ret at the time of declaration to prevent it from
>>> being returned without a defined value. Fixes smatch warning:
>>> drivers/iio/industrialio-gts-helper.c:256 gain_to_scaletables() error:
>>> uninitialized symbol 'ret'.
>>>
>>> Cc: stable@...r.kernel.org # v6.6+
>>> Fixes: 38416c28e168 ("iio: light: Add gain-time-scale helpers")
>>> Signed-off-by: Zicheng Qu <quzicheng@...wei.com>
>>> ---
>>>    drivers/iio/industrialio-gts-helper.c | 2 +-
>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/iio/industrialio-gts-helper.c b/drivers/iio/industrialio-gts-helper.c
>>> index 59d7615c0f56..c5dc5b51693d 100644
>>> --- a/drivers/iio/industrialio-gts-helper.c
>>> +++ b/drivers/iio/industrialio-gts-helper.c
>>> @@ -167,7 +167,7 @@ static int iio_gts_gain_cmp(const void *a, const void *b)
>>>    
>>>    static int gain_to_scaletables(struct iio_gts *gts, int **gains, int **scales)
>>>    {
>>> -	int ret, i, j, new_idx, time_idx;
>>> +	int i, j, new_idx, time_idx, ret = 0;
>>>    	int *all_gains;
>>>    	size_t gain_bytes;
>>>      
>>
>> So, if I read it right, this handles a (corner) case where there is no
>> times given. I am not sure how well such use has been considered because
>> the point of GTS is helping out with cases where the gain and
>> integration time both impact to scale.
>>
>> How do you see the benefits of the gts if there is no such shared impact
>> to scale? Sure the gts could still provide the 'standard table format'
>> to present the gains (or times), and conversions from the register
>> values to gains (or times), and perhaps the available scale table(s) -
>> but I suppose it also brings a lot of unused code and some
>> initialization overhead. (I have a vague feeling this was discussed with
>> Jonathan during the reviews).
>>
>> Reason I am asking these questions is that I wonder if the usage should
>> be limited to cases where we have both gains and times? We could check
>> this in the iio_gts_sanity_check(). (And, I am actually a bit surprized
>> this check was not implemented).
>>
>> Well, initialization fixes a potential bug here and does not really cost
>> much - so big thanks to you :)
>>
>> Reviewed-by: Matti Vaittinen <mazziesaccount@...il.com>
> Indeed I'm not convinced this is a a bug that can be hit, but it is
> obviously good hardening so applied to the fixes togreg branch of iio.git.
> Note I'd like a follow up to use __free() + early returns in this function.
> Will reduce complexity and that last line will become a return 0;

I suppose it is time for me to adapt to use the __cleanup based 
helpers... I'll add this to my TODO-list :)

Yours,
	-- Matti