linux-staging.lists.linux.dev archive mirror
 help / color / mirror / Atom feed
From: Fabio Aiuto <fabioaiuto83@gmail.com>
To: Colin Ian King <colin.king@canonical.com>
Cc: Quytelda Kahja <quytelda@tamalin.org>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	"linux-staging@lists.linux.dev" <linux-staging@lists.linux.dev>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: staging: rtl8723bs mask values in wpa_set_auth_algs
Date: Thu, 15 Jul 2021 12:08:47 +0200	[thread overview]
Message-ID: <20210715100846.GA2271@agape.jhs> (raw)
In-Reply-To: <619462ca-e652-30f5-6ffa-3213175d8bbc@canonical.com>

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 <quytelda@tamalin.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

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

  reply	other threads:[~2021-07-15 10:08 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-09 14:53 staging: rtl8723bs mask values in wpa_set_auth_algs Colin Ian King
2021-07-15 10:08 ` Fabio Aiuto [this message]
2021-07-15 14:57 ` [PATCH] staging: rtl8723bs: fix wpa_set_auth_algs() function Fabio Aiuto

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210715100846.GA2271@agape.jhs \
    --to=fabioaiuto83@gmail.com \
    --cc=colin.king@canonical.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-staging@lists.linux.dev \
    --cc=quytelda@tamalin.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).