linux-kernel - staging: rtl8723bs mask values in wpa_set_auth

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

Message-ID: <619462ca-e652-30f5-6ffa-3213175d8bbc@canonical.com>
Date:   Fri, 9 Jul 2021 15:53:39 +0100
From:   Colin Ian King <colin.king@...onical.com>
To:     Quytelda Kahja <quytelda@...alin.org>
Cc:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        "linux-staging@...ts.linux.dev" <linux-staging@...ts.linux.dev>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: 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 <quytelda@...alin.org>
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