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
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.
Fixes: 5befa937e8da ("staging: rtl8723bs: Fix IEEE80211 authentication algorithm constants.")
Reported-by: Colin Ian King <[email protected]>
Tested-on: Lenovo Ideapad MiiX 300-10IBY
Signed-off-by: Fabio Aiuto <[email protected]>
---
drivers/staging/rtl8723bs/os_dep/ioctl_linux.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/staging/rtl8723bs/os_dep/ioctl_linux.c b/drivers/staging/rtl8723bs/os_dep/ioctl_linux.c
index f95000df8942..965558516cbd 100644
--- a/drivers/staging/rtl8723bs/os_dep/ioctl_linux.c
+++ b/drivers/staging/rtl8723bs/os_dep/ioctl_linux.c
@@ -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
Hello Colin,
On Fri, Jul 09, 2021 at 03:53:39PM +0100, Colin Ian King wrote:
> 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
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
>
> 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
>
>
>
>
thank you,
fabio