linux-kernel - Re: [PATCH] staging: wlan-ng: fix uninitialized variable

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: <c5b92b25-e66c-77b4-3f33-91f7002ef75e@linux.com>
Date:   Thu, 3 Oct 2019 16:39:45 +0300
From:   Denis Efremov <efremov@...ux.com>
To:     Dan Carpenter <dan.carpenter@...cle.com>
Cc:     devel@...verdev.osuosl.org, linux-kernel@...r.kernel.org,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        stable@...r.kernel.org
Subject: Re: [PATCH] staging: wlan-ng: fix uninitialized variable

Hi,

On 10/3/19 2:26 PM, Dan Carpenter wrote:
> On Wed, Oct 02, 2019 at 08:41:03PM +0300, Denis Efremov wrote:
>> The result variable in prism2_connect() can be used uninitialized on path
>> !channel --> ... --> is_wep --> sme->key --> sme->key_idx >= NUM_WEPKEYS.
>> This patch initializes result with 0.
>>
>> Cc: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
>> Cc: stable@...r.kernel.org
>> Signed-off-by: Denis Efremov <efremov@...ux.com>
>> ---
>>  drivers/staging/wlan-ng/cfg80211.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/staging/wlan-ng/cfg80211.c b/drivers/staging/wlan-ng/cfg80211.c
>> index eee1998c4b18..d426905e187e 100644
>> --- a/drivers/staging/wlan-ng/cfg80211.c
>> +++ b/drivers/staging/wlan-ng/cfg80211.c
>> @@ -441,7 +441,7 @@ static int prism2_connect(struct wiphy *wiphy, struct net_device *dev,
>>  	int chan = -1;
>>  	int is_wep = (sme->crypto.cipher_group == WLAN_CIPHER_SUITE_WEP40) ||
>>  	    (sme->crypto.cipher_group == WLAN_CIPHER_SUITE_WEP104);
>> -	int result;
>> +	int result = 0;
>>  	int err = 0;
>>  
> 
> I can't see any reason why we should have both "err" and "result".
> Maybe in olden times "result" used to save positive error codes instead
> of negative error codes but now it's just negatives and zero on success.
> There is no reason for the exit label either, we could just return
> directly.
> 
> So could you redo it and get rid of "result" entirely?  Otherwise it
> just causes more bugs like this.
> 

Yes, of course. I will prepare v2.

Thanks,
Denis