[bug report] wifi: mac80211: store BSS param change count from assoc response

[bug report] wifi: mac80211: store BSS param change count from assoc response

[Dan Carpenter]

[ Unpublished Smatch stuff. -dan ]

Hello Johannes Berg,

The patch 5c1f97537bfb: "wifi: mac80211: store BSS param change count
from assoc response" from Jun 19, 2023, leads to the following Smatch
static checker warning:

	include/linux/ieee80211.h:4924 ieee80211_mle_basic_sta_prof_size_ok()
	warn: duplicate check 'control & 1024' (previous on line 4923)

include/linux/ieee80211.h
    4901 static inline bool ieee80211_mle_basic_sta_prof_size_ok(const u8 *data,
    4902                                                         size_t len)
    4903 {
    4904         const struct ieee80211_mle_per_sta_profile *prof = (const void *)data;
    4905         u16 control;
    4906         u8 fixed = sizeof(*prof);
    4907         u8 info_len = 1;
    4908 
    4909         if (len < fixed)
    4910                 return false;
    4911 
    4912         control = le16_to_cpu(prof->control);
    4913 
    4914         if (control & IEEE80211_MLE_STA_CONTROL_STA_MAC_ADDR_PRESENT)
    4915                 info_len += 6;
    4916         if (control & IEEE80211_MLE_STA_CONTROL_BEACON_INT_PRESENT)
    4917                 info_len += 2;
    4918         if (control & IEEE80211_MLE_STA_CONTROL_TSF_OFFS_PRESENT)
    4919                 info_len += 8;
    4920         if (control & IEEE80211_MLE_STA_CONTROL_DTIM_INFO_PRESENT)
    4921                 info_len += 2;
    4922         if (control & IEEE80211_MLE_STA_CONTROL_COMPLETE_PROFILE &&
                                                                          ^^
Should this be ||?

    4923             control & IEEE80211_MLE_STA_CONTROL_NSTR_BITMAP_SIZE) {
    4924                 if (control & IEEE80211_MLE_STA_CONTROL_NSTR_BITMAP_SIZE)

This is always true because it's checked on the previous line.

    4925                         info_len += 2;
    4926                 else
    4927                         info_len += 1;
    4928         }
    4929         if (control & IEEE80211_MLE_STA_CONTROL_BSS_PARAM_CHANGE_CNT_PRESENT)
    4930                 info_len += 1;
    4931 
    4932         return prof->sta_info_len >= info_len &&
    4933                fixed + prof->sta_info_len <= len;
    4934 }

regards,
dan carpenter

Re: [bug report] wifi: mac80211: store BSS param change count from assoc response

[Johannes Berg]

On Thu, 2023-06-22 at 17:47 +0300, Dan Carpenter wrote:
> 
>     4922         if (control & IEEE80211_MLE_STA_CONTROL_COMPLETE_PROFILE &&
>                                                                           ^^
> Should this be ||?
> 
>     4923             control & IEEE80211_MLE_STA_CONTROL_NSTR_BITMAP_SIZE) {
>     4924                 if (control & IEEE80211_MLE_STA_CONTROL_NSTR_BITMAP_SIZE)
> 
> This is always true because it's checked on the previous line.
> 

Oops. There are actually two places doing this, I think I copy/pasted
it.

But the second part of the if statement should simply be removed. The
COMPLETE_PROFILE bit indicates whether or not the NSTR bitmap is
present, and the NSTR_BITMAP_SIZE bit indicates how it's encoded (8 or
16 bits).

Thanks!

johannes

Re: [bug report] wifi: mac80211: store BSS param change count from assoc response

[Johannes Berg]

On Thu, 2023-06-22 at 19:43 +0200, Johannes Berg wrote:
> On Thu, 2023-06-22 at 17:47 +0300, Dan Carpenter wrote:
> > 
> >     4922         if (control & IEEE80211_MLE_STA_CONTROL_COMPLETE_PROFILE &&
> >                                                                           ^^
> > Should this be ||?
> > 
> >     4923             control & IEEE80211_MLE_STA_CONTROL_NSTR_BITMAP_SIZE) {
> >     4924                 if (control & IEEE80211_MLE_STA_CONTROL_NSTR_BITMAP_SIZE)
> > 
> > This is always true because it's checked on the previous line.
> > 
> 
> Oops. There are actually two places doing this, I think I copy/pasted
> it.
> 
> But the second part of the if statement should simply be removed.

Actually that's wrong, it should check the present bit :) Anyway, fixed,
thanks.

johannes