Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Reply-To: efremov@linux.com
Subject: Re: [PATCH] staging: wlan-ng: fix uninitialized variable
To:     Dan Carpenter <dan.carpenter@oracle.com>
Cc:     devel@driverdev.osuosl.org, linux-kernel@vger.kernel.org,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        stable@vger.kernel.org
References: <20191002174103.1274-1-efremov@linux.com>
 <20191003112649.GR22609@kadam>
From:   Denis Efremov <efremov@linux.com>
Message-ID: <c5b92b25-e66c-77b4-3f33-91f7002ef75e@linux.com>
Date:   Thu, 3 Oct 2019 16:39:45 +0300
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101
 Thunderbird/68.1.0
MIME-Version: 1.0
In-Reply-To: <20191003112649.GR22609@kadam>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

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@linuxfoundation.org>
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Denis Efremov <efremov@linux.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