Bug 212993

Summary: A possible divide by zero in mwifiex_set_ibss_params
Product: Networking Reporter: YiyuanGUO (yguoaz)
Component: WirelessAssignee: networking_wireless (networking_wireless)
Status: RESOLVED INVALID    
Severity: normal CC: akarwar, briannorris, ganapathi.bhat, huxinming820
Priority: P1    
Hardware: All   
OS: Linux   
Kernel Version: 5.12.2 Subsystem:
Regression: No Bisected commit-id:

Description YiyuanGUO 2021-05-08 11:01:34 UTC
In the file drivers/net/wireless/marvell/mwifiex/cfg80211.c, the function mwifiex_set_ibss_params has the following code (link to the code location: https://github.com/torvalds/linux/blob/dd860052c99b1e088352bdd4fb7aef46f8d2ef47/drivers/net/wireless/marvell/mwifiex/cfg80211.c#L2453):

static int mwifiex_set_ibss_params(struct mwifiex_private *priv,
				   struct cfg80211_ibss_params *params) {
  ...
  int index = 0;
  ...
  if (params->chandef.chan->band == NL80211_BAND_2GHZ) {
    if (!params->basic_rates) {
      ...
    } else {
      for (i = 0; i < mwifiex_band_2ghz.n_bitrates; i++) {
        if (mwifiex_rates[i].bitrate == 60) {
	  index = 1 << i;
	  break;
	}
      }

      if (params->basic_rates < index) {
        ...
      } else {
        ...
	if (params->basic_rates % index)
	...				
      }
    }
  }
}

If the variable index is never altered inside the for loop (i.e., no bitrare equals to 60), then it will remain 0 until evaluating params->basic_rates % index, leading to a divide by zero problem.
Comment 1 Brian Norris 2021-06-10 23:53:31 UTC
> If the variable index is never altered inside the for loop (i.e., no bitrare
> equals to 60)

Well, there's your kicker -- this loop uses a constant array (mwifiex_rates[]) which *does* have an entry with a bitrate of 60. So this divide by zero won't happen.

This isn't exactly the most beautiful code, but bugzilla probably isn't the place for code review.

Closing this as INVALID.