staging: rtl8723bs mask values in wpa_set_auth_algs

Hi,

Static analysis with Coverity has found an issue introduced by the following commit:

commit 5befa937e8daaebcde81b9423eb93f3ff2e918f7

Author: Quytelda Kahja <[email protected]>
Date:   Tue Mar 27 01:41:02 2018 -0700

staging: rtl8723bs: Fix IEEE80211 authentication algorithm constants.

The analysis is as follows:

347 static int wpa_set_auth_algs(struct net_device *dev, u32 value) 348 {
349 struct adapter *padapter = rtw_netdev_priv(dev);
350 int ret = 0;
351
352 if ((value & WLAN_AUTH_SHARED_KEY) && (value & WLAN_AUTH_OPEN)) {

Logically dead code (DEADCODE)

353 padapter->securitypriv.ndisencryptstatus =
Ndis802_11Encryption1Enabled;
354 padapter->securitypriv.ndisauthtype =
Ndis802_11AuthModeAutoSwitch;
355 padapter->securitypriv.dot11AuthAlgrthm =
dot11AuthAlgrthm_Auto;
356 } else if (value & WLAN_AUTH_SHARED_KEY) {
357 padapter->securitypriv.ndisencryptstatus =
Ndis802_11Encryption1Enabled;
358
359 padapter->securitypriv.ndisauthtype =
Ndis802_11AuthModeShared;
360 padapter->securitypriv.dot11AuthAlgrthm =
dot11AuthAlgrthm_Shared;
dead_error_condition: The condition value & 0U cannot be true.
361 } else if (value & WLAN_AUTH_OPEN) {
362 /* padapter->securitypriv.ndisencryptstatus =
Ndis802_11EncryptionDisabled; */

Logically dead code (DEADCODE)

363 if (padapter->securitypriv.ndisauthtype <
Ndis802_11AuthModeWPAPSK) {
364 padapter->securitypriv.ndisauthtype =
Ndis802_11AuthModeOpen;
365 padapter->securitypriv.dot11AuthAlgrthm =
dot11AuthAlgrthm_Open;
366 }
367 } else {
368 ret = -EINVAL;
369 }
370
371 return ret;
372
373 }

The deadcode warnings occur because the mask value of WLAN_AUTH_OPEN is defined in /include/linux/ieee80211.h as:

#define WLAN_AUTO_OPEN 0

and so any value masked with 0 is 0 and hence that part of the if statement is always false.

The original code was using the mask values:

AUTH_ALG_SHARED_KEY and also AUTH_ALG_OPEN_SYSTEM that are defined as:

#define AUTH_ALG_OPEN_SYSTEM 0x1
#define AUTH_ALG_SHARED_KEY 0x2

So this appears to be a regression. I'm not sure the best approach for a suitable fix to use the intended macros in ieee80211.h

Colin

Hello Colin,

I think that the correct macros are:

/* IW_AUTH_80211_AUTH_ALG values (bit field) */
#define IW_AUTH_ALG_OPEN_SYSTEM 0x00000001
#define IW_AUTH_ALG_SHARED_KEY 0x00000002
#define IW_AUTH_ALG_LEAP 0x00000004

defined in include/uapi/linux/wireless.h

thank you,

fabio

fix authentication algorithm constants. wpa_set_auth_algs() function contains some conditional statements masking the checked value with the wrong constants. This produces some unintentional dead code. Mask the value with the right macros.

Tested-on: Lenovo Ideapad MiiX 300-10IBY

---
 drivers/staging/rtl8723bs/os_dep/ioctl_linux.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

index f95000df8942..965558516cbd 100644
@@ -349,16 +349,16 @@ static int wpa_set_auth_algs(struct net_device *dev, u32 value)
 	struct adapter *padapter = rtw_netdev_priv(dev);
 	int ret = 0;
 
-	if ((value & WLAN_AUTH_SHARED_KEY) && (value & WLAN_AUTH_OPEN)) {
+	if ((value & IW_AUTH_ALG_SHARED_KEY) && (value & IW_AUTH_ALG_OPEN_SYSTEM)) {
 		padapter->securitypriv.ndisencryptstatus = Ndis802_11Encryption1Enabled;
 		padapter->securitypriv.ndisauthtype = Ndis802_11AuthModeAutoSwitch;
 		padapter->securitypriv.dot11AuthAlgrthm = dot11AuthAlgrthm_Auto;
-	} else if (value & WLAN_AUTH_SHARED_KEY)	{
+	} else if (value & IW_AUTH_ALG_SHARED_KEY)	{
 		padapter->securitypriv.ndisencryptstatus = Ndis802_11Encryption1Enabled;
 
 		padapter->securitypriv.ndisauthtype = Ndis802_11AuthModeShared;
 		padapter->securitypriv.dot11AuthAlgrthm = dot11AuthAlgrthm_Shared;
-	} else if (value & WLAN_AUTH_OPEN) {
+	} else if (value & IW_AUTH_ALG_OPEN_SYSTEM) {
 		/* padapter->securitypriv.ndisencryptstatus = Ndis802_11EncryptionDisabled; */
 		if (padapter->securitypriv.ndisauthtype < Ndis802_11AuthModeWPAPSK) {
 			padapter->securitypriv.ndisauthtype = Ndis802_11AuthModeOpen;

2.20.1