Skip to contentSkip to conversation

iwlwifi: split the drivers for agn and legacy devices 3945/4965

DISCUSSION Dan Carpenter 1 participant 1 message Feb 26, 2015

lore
Share & tools
lore
#0 lore raw

Hello Wey-Yi Guy,

The patch be663ab67077: "iwlwifi: split the drivers for agn and legacy devices 3945/4965" from Feb 21, 2011, leads to the following static checker warning:

	drivers/net/wireless/iwlegacy/4965-mac.c:6390 il4965_init_drv()
	error: potential NULL dereference 'il->ieee_rates'.

Also:

	drivers/net/wireless/iwlegacy/3945-mac.c:3541 il3945_init_drv()
	error: potential NULL dereference 'il->ieee_rates'.

The il_init_geos() function is the only place where we set "->ieee_rates" to non-NULL. There are two callers and they both rely on il_init_geos() to set it to non-NULL otherwise they crash. However, there is a return in il_init_geos() which does not set ->ieee_rates so if we ever hit that path then the kernel will Oops. This makes the static checkers complain.

drivers/net/wireless/iwlegacy/common.c

  3431  int
  3432  il_init_geos(struct il_priv *il)
  3433  {
  3434          struct il_channel_info *ch;
  3435          struct ieee80211_supported_band *sband;
  3436          struct ieee80211_channel *channels;
  3437          struct ieee80211_channel *geo_ch;
  3438          struct ieee80211_rate *rates;
  3439          int i = 0;
  3440          s8 max_tx_power = 0;
  3441  
  3442          if (il->bands[IEEE80211_BAND_2GHZ].n_bitrates ||
  3443              il->bands[IEEE80211_BAND_5GHZ].n_bitrates) {
  3444                  D_INFO("Geography modes already initialized.\n");
  3445                  set_bit(S_GEO_CONFIGURED, &il->status);
  3446                  return 0;
                        ^^^^^^^^

We have not set ->ieee_rates here. If we ever hit this return then the kernel will Oops.

3447 }
3448

Here is one caller, the other is the same.

drivers/net/wireless/iwlegacy/4965-mac.c
6362 il->ieee_channels = NULL;

  6363          il->ieee_rates = NULL;
                ^^^^^^^^^^^^^^^^^^^^^

We set il->ieee_rates to NULL.

6364 il->band = IEEE80211_BAND_2GHZ;
6365

  6366          il->iw_mode = NL80211_IFTYPE_STATION;
  6367          il->current_ht_config.smps = IEEE80211_SMPS_STATIC;
  6368          il->missed_beacon_threshold = IL_MISSED_BEACON_THRESHOLD_DEF;
  6369  
  6370          /* initialize force reset */
  6371          il->force_reset.reset_duration = IL_DELAY_NEXT_FORCE_FW_RELOAD;
  6372  
  6373          /* Choose which receivers/antennas to use */
  6374          if (il->ops->set_rxon_chain)
  6375                  il->ops->set_rxon_chain(il);
  6376  
  6377          il_init_scan_params(il);
  6378  
  6379          ret = il_init_channel_map(il);
  6380          if (ret) {
  6381                  IL_ERR("initializing regulatory failed: %d\n", ret);
  6382                  goto err;
  6383          }
  6384  
  6385          ret = il_init_geos(il);
                      ^^^^^^^^^^^^^^^^

Assume we hit the problematic return.

6386 if (ret) {
6387 IL_ERR("initializing geos failed: %d\n", ret);
6388 goto err_free_channel_map;
6389 }

  6390          il4965_init_hw_rates(il, il->ieee_rates);
                                         ^^^^^^^^^^^^^^

This will lead to a NULL dereference inside the il4965_init_hw_rates() function.

6391
6392 return 0;

regards,
dan carpenter