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

Message-ID: <1a004693-5dd5-458c-b8ff-b9a978298122@huawei.com>
Date: Fri, 1 Nov 2024 11:43:24 +0800
From: Zicheng Qu <quzicheng@...wei.com>
To: Matti Vaittinen <mazziesaccount@...il.com>, <linux-iio@...r.kernel.org>,
	<linux-kernel@...r.kernel.org>
CC: <tanghui20@...wei.com>, <zhangqiao22@...wei.com>,
	<judy.chenhui@...wei.com>
Subject: Re: [PATCH] iio: Fix uninitialized symbol 'ret'

Hi Matti,

It might be better but I am not pretty sure whether have to have both 
gains and times, so I modified 'ret' in places where issues might arise, 
rather than adding a restriction in iio_gts_sanity_check().

In the corner case, there is a restriction that both num_hwgain and 
num_itime cannot be 0 simultaneously in the iio_gts_sanity_check(). 
However, in the gain_to_scaletables() , if num_itime is 1 and num_hwgain 
is 0, the 'gain_bytes' becomes 0 and 'all_gains' is not null after the 
'for' loop, which causes the subsequent 'while' or 'for' to become 
ineffective, leading to an undefined 'ret' being returned in the 'free_out'.

Yours,

-- Zicheng

On 2024/10/31 15:13, Matti Vaittinen 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>
>
> Yours,
>  -- Matti Vaittinen
>