linux-wireless.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* Re: drivers/net/wireless/mediatek/mt76/mt76x02_util.c:475 mt76x02_set_key() warn: variable dereferenced before check 'key' (see line 415)
       [not found] <2f121202-5846-44a9-8b83-e2ba1fa671d0@kadam.mountain>
@ 2023-06-06 13:42 ` Kalle Valo
  2023-06-06 16:13   ` Lorenzo Bianconi
  0 siblings, 1 reply; 3+ messages in thread
From: Kalle Valo @ 2023-06-06 13:42 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: oe-kbuild, Felix Fietkau, lkp, oe-kbuild-all, linux-kernel,
	linux-wireless

Adding linux-wireless, top posting so that the whole report is included.

Dan Carpenter <dan.carpenter@linaro.org> writes:

> tree:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
> head:   f8dba31b0a826e691949cd4fdfa5c30defaac8c5
> commit: e6db67fa871dee37d22701daba806bfcd4d9df49 wifi: mt76: ignore
> key disable commands
> config: riscv-randconfig-m031-20230605
> (https://download.01.org/0day-ci/archive/20230606/202306060332.WbIToDHL-lkp@intel.com/config)
> compiler: riscv64-linux-gcc (GCC) 12.3.0
>
> If you fix the issue, kindly add following tag where applicable
> | Reported-by: kernel test robot <lkp@intel.com>
> | Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
> | Closes: https://lore.kernel.org/r/202306060332.WbIToDHL-lkp@intel.com/
>
> smatch warnings:
> drivers/net/wireless/mediatek/mt76/mt76x02_util.c:475
> mt76x02_set_key() warn: variable dereferenced before check 'key' (see
> line 415)
>
> vim +/key +475 drivers/net/wireless/mediatek/mt76/mt76x02_util.c
>
> 60c26859e863c1 Stanislaw Gruszka 2018-09-04 407 int
> mt76x02_set_key(struct ieee80211_hw *hw, enum set_key_cmd cmd,
> 60c26859e863c1 Stanislaw Gruszka 2018-09-04 408 struct ieee80211_vif
> *vif, struct ieee80211_sta *sta,
> 60c26859e863c1 Stanislaw Gruszka 2018-09-04 409 struct
> ieee80211_key_conf *key)
> 60c26859e863c1 Stanislaw Gruszka 2018-09-04  410  {
> d87cf75f111183 Lorenzo Bianconi 2018-10-07 411 struct mt76x02_dev *dev
> = hw->priv;
> 60c26859e863c1 Stanislaw Gruszka 2018-09-04 412 struct mt76x02_vif
> *mvif = (struct mt76x02_vif *)vif->drv_priv;
> 60c26859e863c1 Stanislaw Gruszka 2018-09-04  413  	struct mt76x02_sta *msta;
> 60c26859e863c1 Stanislaw Gruszka 2018-09-04  414  	struct mt76_wcid *wcid;
> 60c26859e863c1 Stanislaw Gruszka 2018-09-04 @415  	int idx = key->keyidx;
>
> "key" is dereferenced here
>
> 60c26859e863c1 Stanislaw Gruszka 2018-09-04  416  	int ret;
> 60c26859e863c1 Stanislaw Gruszka 2018-09-04  417  
> 60c26859e863c1 Stanislaw Gruszka 2018-09-04 418 /* fall back to sw
> encryption for unsupported ciphers */
> 60c26859e863c1 Stanislaw Gruszka 2018-09-04  419  	switch (key->cipher) {
> 60c26859e863c1 Stanislaw Gruszka 2018-09-04  420  	case WLAN_CIPHER_SUITE_WEP40:
> 60c26859e863c1 Stanislaw Gruszka 2018-09-04  421  	case WLAN_CIPHER_SUITE_WEP104:
> 60c26859e863c1 Stanislaw Gruszka 2018-09-04  422  	case WLAN_CIPHER_SUITE_TKIP:
> 60c26859e863c1 Stanislaw Gruszka 2018-09-04  423  	case WLAN_CIPHER_SUITE_CCMP:
> 60c26859e863c1 Stanislaw Gruszka 2018-09-04  424  		break;
> 60c26859e863c1 Stanislaw Gruszka 2018-09-04  425  	default:
> 60c26859e863c1 Stanislaw Gruszka 2018-09-04  426  		return -EOPNOTSUPP;
> 60c26859e863c1 Stanislaw Gruszka 2018-09-04  427  	}
> 60c26859e863c1 Stanislaw Gruszka 2018-09-04  428  
> 60c26859e863c1 Stanislaw Gruszka 2018-09-04  429  	/*
> 60c26859e863c1 Stanislaw Gruszka 2018-09-04 430 * The hardware does
> not support per-STA RX GTK, fall back
> 60c26859e863c1 Stanislaw Gruszka 2018-09-04  431  	 * to software mode for these.
> 60c26859e863c1 Stanislaw Gruszka 2018-09-04  432  	 */
> 60c26859e863c1 Stanislaw Gruszka 2018-09-04 433 if ((vif->type ==
> NL80211_IFTYPE_ADHOC ||
> 60c26859e863c1 Stanislaw Gruszka 2018-09-04 434 vif->type ==
> NL80211_IFTYPE_MESH_POINT) &&
> 60c26859e863c1 Stanislaw Gruszka 2018-09-04 435 (key->cipher ==
> WLAN_CIPHER_SUITE_TKIP ||
> 60c26859e863c1 Stanislaw Gruszka 2018-09-04 436 key->cipher ==
> WLAN_CIPHER_SUITE_CCMP) &&
> 60c26859e863c1 Stanislaw Gruszka 2018-09-04 437 !(key->flags &
> IEEE80211_KEY_FLAG_PAIRWISE))
> 60c26859e863c1 Stanislaw Gruszka 2018-09-04  438  		return -EOPNOTSUPP;
> 60c26859e863c1 Stanislaw Gruszka 2018-09-04  439  
> b98558e2529986 Stanislaw Gruszka 2019-03-19  440  	/*
> b98558e2529986 Stanislaw Gruszka 2019-03-19 441 * In USB AP mode,
> broadcast/multicast frames are setup in beacon
> b98558e2529986 Stanislaw Gruszka 2019-03-19 442 * data registers and
> sent via HW beacons engine, they require to
> b98558e2529986 Stanislaw Gruszka 2019-03-19  443  	 * be already encrypted.
> b98558e2529986 Stanislaw Gruszka 2019-03-19  444  	 */
> 61c51a74a4e586 Lorenzo Bianconi  2019-10-29  445  	if (mt76_is_usb(&dev->mt76) &&
> b98558e2529986 Stanislaw Gruszka 2019-03-19 446 vif->type ==
> NL80211_IFTYPE_AP &&
> b98558e2529986 Stanislaw Gruszka 2019-03-19 447 !(key->flags &
> IEEE80211_KEY_FLAG_PAIRWISE))
> b98558e2529986 Stanislaw Gruszka 2019-03-19  448  		return -EOPNOTSUPP;
> b98558e2529986 Stanislaw Gruszka 2019-03-19  449  
> 4b36cc6b390f18 David Bauer 2021-02-07 450 /* MT76x0 GTK offloading
> does not work with more than one VIF */
> 4b36cc6b390f18 David Bauer 2021-02-07 451 if (is_mt76x0(dev) &&
> !(key->flags & IEEE80211_KEY_FLAG_PAIRWISE))
> 4b36cc6b390f18 David Bauer       2021-02-07  452  		return -EOPNOTSUPP;
> 4b36cc6b390f18 David Bauer       2021-02-07  453  
> 60c26859e863c1 Stanislaw Gruszka 2018-09-04 454 msta = sta ? (struct
> mt76x02_sta *)sta->drv_priv : NULL;
> 60c26859e863c1 Stanislaw Gruszka 2018-09-04 455 wcid = msta ?
> &msta->wcid : &mvif->group_wcid;
> 60c26859e863c1 Stanislaw Gruszka 2018-09-04  456  
> e6db67fa871dee Felix Fietkau     2023-03-30  457  	if (cmd != SET_KEY) {
> 60c26859e863c1 Stanislaw Gruszka 2018-09-04 458 if (idx ==
> wcid->hw_key_idx) {
> 60c26859e863c1 Stanislaw Gruszka 2018-09-04 459 wcid->hw_key_idx = -1;
> f2f6a47b504b8f Felix Fietkau 2019-01-25 460 wcid->sw_iv = false;
> 60c26859e863c1 Stanislaw Gruszka 2018-09-04  461  		}
> 60c26859e863c1 Stanislaw Gruszka 2018-09-04  462  
> e6db67fa871dee Felix Fietkau     2023-03-30  463  		return 0;
> e6db67fa871dee Felix Fietkau     2023-03-30  464  	}
> e6db67fa871dee Felix Fietkau     2023-03-30  465  
> e6db67fa871dee Felix Fietkau     2023-03-30  466  	key->hw_key_idx = wcid->idx;
> e6db67fa871dee Felix Fietkau     2023-03-30  467  	wcid->hw_key_idx = idx;
> e6db67fa871dee Felix Fietkau 2023-03-30 468 if (key->flags &
> IEEE80211_KEY_FLAG_RX_MGMT) {
> e6db67fa871dee Felix Fietkau 2023-03-30 469 key->flags |=
> IEEE80211_KEY_FLAG_SW_MGMT_TX;
> e6db67fa871dee Felix Fietkau     2023-03-30  470  		wcid->sw_iv = true;
> 60c26859e863c1 Stanislaw Gruszka 2018-09-04  471  	}
> d87cf75f111183 Lorenzo Bianconi 2018-10-07 472
> mt76_wcid_key_setup(&dev->mt76, wcid, key);
> 60c26859e863c1 Stanislaw Gruszka 2018-09-04  473  
> 60c26859e863c1 Stanislaw Gruszka 2018-09-04  474  	if (!msta) {
> 60c26859e863c1 Stanislaw Gruszka 2018-09-04 @475 if (key ||
> wcid->hw_key_idx == idx) {
>
> This NULL check is too late.
>
> 8d66af49a3db9a Lorenzo Bianconi 2018-10-07 476 ret =
> mt76x02_mac_wcid_set_key(dev, wcid->idx, key);
> 60c26859e863c1 Stanislaw Gruszka 2018-09-04  477  			if (ret)
> 60c26859e863c1 Stanislaw Gruszka 2018-09-04 478 return ret;
> 60c26859e863c1 Stanislaw Gruszka 2018-09-04  479  		}
> 60c26859e863c1 Stanislaw Gruszka 2018-09-04  480  
> 8d66af49a3db9a Lorenzo Bianconi 2018-10-07 481 return
> mt76x02_mac_shared_key_setup(dev, mvif->idx, idx, key);
> 60c26859e863c1 Stanislaw Gruszka 2018-09-04  482  	}
> 60c26859e863c1 Stanislaw Gruszka 2018-09-04  483  
> 8d66af49a3db9a Lorenzo Bianconi 2018-10-07 484 return
> mt76x02_mac_wcid_set_key(dev, msta->wcid.idx, key);
> 60c26859e863c1 Stanislaw Gruszka 2018-09-04  485  }

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: drivers/net/wireless/mediatek/mt76/mt76x02_util.c:475 mt76x02_set_key() warn: variable dereferenced before check 'key' (see line 415)
  2023-06-06 13:42 ` Kalle Valo
@ 2023-06-06 16:13   ` Lorenzo Bianconi
  0 siblings, 0 replies; 3+ messages in thread
From: Lorenzo Bianconi @ 2023-06-06 16:13 UTC (permalink / raw)
  To: Kalle Valo
  Cc: Dan Carpenter, oe-kbuild, Felix Fietkau, lkp, oe-kbuild-all,
	linux-kernel, linux-wireless

[-- Attachment #1: Type: text/plain, Size: 8012 bytes --]

> Adding linux-wireless, top posting so that the whole report is included.
> 
> Dan Carpenter <dan.carpenter@linaro.org> writes:
> 
> > tree:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
> > head:   f8dba31b0a826e691949cd4fdfa5c30defaac8c5
> > commit: e6db67fa871dee37d22701daba806bfcd4d9df49 wifi: mt76: ignore
> > key disable commands
> > config: riscv-randconfig-m031-20230605
> > (https://download.01.org/0day-ci/archive/20230606/202306060332.WbIToDHL-lkp@intel.com/config)
> > compiler: riscv64-linux-gcc (GCC) 12.3.0
> >
> > If you fix the issue, kindly add following tag where applicable
> > | Reported-by: kernel test robot <lkp@intel.com>
> > | Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
> > | Closes: https://lore.kernel.org/r/202306060332.WbIToDHL-lkp@intel.com/
> >
> > smatch warnings:
> > drivers/net/wireless/mediatek/mt76/mt76x02_util.c:475
> > mt76x02_set_key() warn: variable dereferenced before check 'key' (see
> > line 415)
> >
> > vim +/key +475 drivers/net/wireless/mediatek/mt76/mt76x02_util.c
> >
> > 60c26859e863c1 Stanislaw Gruszka 2018-09-04 407 int
> > mt76x02_set_key(struct ieee80211_hw *hw, enum set_key_cmd cmd,
> > 60c26859e863c1 Stanislaw Gruszka 2018-09-04 408 struct ieee80211_vif
> > *vif, struct ieee80211_sta *sta,
> > 60c26859e863c1 Stanislaw Gruszka 2018-09-04 409 struct
> > ieee80211_key_conf *key)
> > 60c26859e863c1 Stanislaw Gruszka 2018-09-04  410  {
> > d87cf75f111183 Lorenzo Bianconi 2018-10-07 411 struct mt76x02_dev *dev
> > = hw->priv;
> > 60c26859e863c1 Stanislaw Gruszka 2018-09-04 412 struct mt76x02_vif
> > *mvif = (struct mt76x02_vif *)vif->drv_priv;
> > 60c26859e863c1 Stanislaw Gruszka 2018-09-04  413  	struct mt76x02_sta *msta;
> > 60c26859e863c1 Stanislaw Gruszka 2018-09-04  414  	struct mt76_wcid *wcid;
> > 60c26859e863c1 Stanislaw Gruszka 2018-09-04 @415  	int idx = key->keyidx;
> >
> > "key" is dereferenced here

I think I have already fixed it here:

https://patchwork.kernel.org/project/linux-wireless/patch/d560f1a16c9024b6e4029bd0baf53384f4552f5a.1683020788.git.lorenzo@kernel.org/

Regards,
Lorenzo

> >
> > 60c26859e863c1 Stanislaw Gruszka 2018-09-04  416  	int ret;
> > 60c26859e863c1 Stanislaw Gruszka 2018-09-04  417  
> > 60c26859e863c1 Stanislaw Gruszka 2018-09-04 418 /* fall back to sw
> > encryption for unsupported ciphers */
> > 60c26859e863c1 Stanislaw Gruszka 2018-09-04  419  	switch (key->cipher) {
> > 60c26859e863c1 Stanislaw Gruszka 2018-09-04  420  	case WLAN_CIPHER_SUITE_WEP40:
> > 60c26859e863c1 Stanislaw Gruszka 2018-09-04  421  	case WLAN_CIPHER_SUITE_WEP104:
> > 60c26859e863c1 Stanislaw Gruszka 2018-09-04  422  	case WLAN_CIPHER_SUITE_TKIP:
> > 60c26859e863c1 Stanislaw Gruszka 2018-09-04  423  	case WLAN_CIPHER_SUITE_CCMP:
> > 60c26859e863c1 Stanislaw Gruszka 2018-09-04  424  		break;
> > 60c26859e863c1 Stanislaw Gruszka 2018-09-04  425  	default:
> > 60c26859e863c1 Stanislaw Gruszka 2018-09-04  426  		return -EOPNOTSUPP;
> > 60c26859e863c1 Stanislaw Gruszka 2018-09-04  427  	}
> > 60c26859e863c1 Stanislaw Gruszka 2018-09-04  428  
> > 60c26859e863c1 Stanislaw Gruszka 2018-09-04  429  	/*
> > 60c26859e863c1 Stanislaw Gruszka 2018-09-04 430 * The hardware does
> > not support per-STA RX GTK, fall back
> > 60c26859e863c1 Stanislaw Gruszka 2018-09-04  431  	 * to software mode for these.
> > 60c26859e863c1 Stanislaw Gruszka 2018-09-04  432  	 */
> > 60c26859e863c1 Stanislaw Gruszka 2018-09-04 433 if ((vif->type ==
> > NL80211_IFTYPE_ADHOC ||
> > 60c26859e863c1 Stanislaw Gruszka 2018-09-04 434 vif->type ==
> > NL80211_IFTYPE_MESH_POINT) &&
> > 60c26859e863c1 Stanislaw Gruszka 2018-09-04 435 (key->cipher ==
> > WLAN_CIPHER_SUITE_TKIP ||
> > 60c26859e863c1 Stanislaw Gruszka 2018-09-04 436 key->cipher ==
> > WLAN_CIPHER_SUITE_CCMP) &&
> > 60c26859e863c1 Stanislaw Gruszka 2018-09-04 437 !(key->flags &
> > IEEE80211_KEY_FLAG_PAIRWISE))
> > 60c26859e863c1 Stanislaw Gruszka 2018-09-04  438  		return -EOPNOTSUPP;
> > 60c26859e863c1 Stanislaw Gruszka 2018-09-04  439  
> > b98558e2529986 Stanislaw Gruszka 2019-03-19  440  	/*
> > b98558e2529986 Stanislaw Gruszka 2019-03-19 441 * In USB AP mode,
> > broadcast/multicast frames are setup in beacon
> > b98558e2529986 Stanislaw Gruszka 2019-03-19 442 * data registers and
> > sent via HW beacons engine, they require to
> > b98558e2529986 Stanislaw Gruszka 2019-03-19  443  	 * be already encrypted.
> > b98558e2529986 Stanislaw Gruszka 2019-03-19  444  	 */
> > 61c51a74a4e586 Lorenzo Bianconi  2019-10-29  445  	if (mt76_is_usb(&dev->mt76) &&
> > b98558e2529986 Stanislaw Gruszka 2019-03-19 446 vif->type ==
> > NL80211_IFTYPE_AP &&
> > b98558e2529986 Stanislaw Gruszka 2019-03-19 447 !(key->flags &
> > IEEE80211_KEY_FLAG_PAIRWISE))
> > b98558e2529986 Stanislaw Gruszka 2019-03-19  448  		return -EOPNOTSUPP;
> > b98558e2529986 Stanislaw Gruszka 2019-03-19  449  
> > 4b36cc6b390f18 David Bauer 2021-02-07 450 /* MT76x0 GTK offloading
> > does not work with more than one VIF */
> > 4b36cc6b390f18 David Bauer 2021-02-07 451 if (is_mt76x0(dev) &&
> > !(key->flags & IEEE80211_KEY_FLAG_PAIRWISE))
> > 4b36cc6b390f18 David Bauer       2021-02-07  452  		return -EOPNOTSUPP;
> > 4b36cc6b390f18 David Bauer       2021-02-07  453  
> > 60c26859e863c1 Stanislaw Gruszka 2018-09-04 454 msta = sta ? (struct
> > mt76x02_sta *)sta->drv_priv : NULL;
> > 60c26859e863c1 Stanislaw Gruszka 2018-09-04 455 wcid = msta ?
> > &msta->wcid : &mvif->group_wcid;
> > 60c26859e863c1 Stanislaw Gruszka 2018-09-04  456  
> > e6db67fa871dee Felix Fietkau     2023-03-30  457  	if (cmd != SET_KEY) {
> > 60c26859e863c1 Stanislaw Gruszka 2018-09-04 458 if (idx ==
> > wcid->hw_key_idx) {
> > 60c26859e863c1 Stanislaw Gruszka 2018-09-04 459 wcid->hw_key_idx = -1;
> > f2f6a47b504b8f Felix Fietkau 2019-01-25 460 wcid->sw_iv = false;
> > 60c26859e863c1 Stanislaw Gruszka 2018-09-04  461  		}
> > 60c26859e863c1 Stanislaw Gruszka 2018-09-04  462  
> > e6db67fa871dee Felix Fietkau     2023-03-30  463  		return 0;
> > e6db67fa871dee Felix Fietkau     2023-03-30  464  	}
> > e6db67fa871dee Felix Fietkau     2023-03-30  465  
> > e6db67fa871dee Felix Fietkau     2023-03-30  466  	key->hw_key_idx = wcid->idx;
> > e6db67fa871dee Felix Fietkau     2023-03-30  467  	wcid->hw_key_idx = idx;
> > e6db67fa871dee Felix Fietkau 2023-03-30 468 if (key->flags &
> > IEEE80211_KEY_FLAG_RX_MGMT) {
> > e6db67fa871dee Felix Fietkau 2023-03-30 469 key->flags |=
> > IEEE80211_KEY_FLAG_SW_MGMT_TX;
> > e6db67fa871dee Felix Fietkau     2023-03-30  470  		wcid->sw_iv = true;
> > 60c26859e863c1 Stanislaw Gruszka 2018-09-04  471  	}
> > d87cf75f111183 Lorenzo Bianconi 2018-10-07 472
> > mt76_wcid_key_setup(&dev->mt76, wcid, key);
> > 60c26859e863c1 Stanislaw Gruszka 2018-09-04  473  
> > 60c26859e863c1 Stanislaw Gruszka 2018-09-04  474  	if (!msta) {
> > 60c26859e863c1 Stanislaw Gruszka 2018-09-04 @475 if (key ||
> > wcid->hw_key_idx == idx) {
> >
> > This NULL check is too late.
> >
> > 8d66af49a3db9a Lorenzo Bianconi 2018-10-07 476 ret =
> > mt76x02_mac_wcid_set_key(dev, wcid->idx, key);
> > 60c26859e863c1 Stanislaw Gruszka 2018-09-04  477  			if (ret)
> > 60c26859e863c1 Stanislaw Gruszka 2018-09-04 478 return ret;
> > 60c26859e863c1 Stanislaw Gruszka 2018-09-04  479  		}
> > 60c26859e863c1 Stanislaw Gruszka 2018-09-04  480  
> > 8d66af49a3db9a Lorenzo Bianconi 2018-10-07 481 return
> > mt76x02_mac_shared_key_setup(dev, mvif->idx, idx, key);
> > 60c26859e863c1 Stanislaw Gruszka 2018-09-04  482  	}
> > 60c26859e863c1 Stanislaw Gruszka 2018-09-04  483  
> > 8d66af49a3db9a Lorenzo Bianconi 2018-10-07 484 return
> > mt76x02_mac_wcid_set_key(dev, msta->wcid.idx, key);
> > 60c26859e863c1 Stanislaw Gruszka 2018-09-04  485  }
> 
> -- 
> https://patchwork.kernel.org/project/linux-wireless/list/
> 
> https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: drivers/net/wireless/mediatek/mt76/mt76x02_util.c:475 mt76x02_set_key() warn: variable dereferenced before check 'key' (see line 415)
       [not found] <77fee326-963a-40eb-80c9-2788a9ff9c22@kadam.mountain>
@ 2023-10-12 10:27 ` Kalle Valo
  0 siblings, 0 replies; 3+ messages in thread
From: Kalle Valo @ 2023-10-12 10:27 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: oe-kbuild, Felix Fietkau, lkp, oe-kbuild-all, linux-kernel,
	linux-wireless

(Adding linux-wireless, full report below.)

Dan Carpenter <dan.carpenter@linaro.org> writes:

> tree:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
> head:   401644852d0b2a278811de38081be23f74b5bb04
> commit: e6db67fa871dee37d22701daba806bfcd4d9df49 wifi: mt76: ignore key disable commands
> config: i386-randconfig-141-20231011 (https://download.01.org/0day-ci/archive/20231012/202310121455.LwR349tb-lkp@intel.com/config)
> compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
> reproduce: (https://download.01.org/0day-ci/archive/20231012/202310121455.LwR349tb-lkp@intel.com/reproduce)
>
> If you fix the issue in a separate patch/commit (i.e. not just a new version of
> the same patch/commit), kindly add following tags
> | Reported-by: kernel test robot <lkp@intel.com>
> | Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
> | Closes: https://lore.kernel.org/r/202310121455.LwR349tb-lkp@intel.com/
>
> smatch warnings:
> drivers/net/wireless/mediatek/mt76/mt76x02_util.c:475 mt76x02_set_key() warn: variable dereferenced before check 'key' (see line 415)
>
> vim +/key +475 drivers/net/wireless/mediatek/mt76/mt76x02_util.c
>
> 60c26859e863c1 Stanislaw Gruszka 2018-09-04  407  int mt76x02_set_key(struct ieee80211_hw *hw, enum set_key_cmd cmd,
> 60c26859e863c1 Stanislaw Gruszka 2018-09-04  408  		    struct ieee80211_vif *vif, struct ieee80211_sta *sta,
> 60c26859e863c1 Stanislaw Gruszka 2018-09-04  409  		    struct ieee80211_key_conf *key)
> 60c26859e863c1 Stanislaw Gruszka 2018-09-04  410  {
> d87cf75f111183 Lorenzo Bianconi  2018-10-07  411  	struct mt76x02_dev *dev = hw->priv;
> 60c26859e863c1 Stanislaw Gruszka 2018-09-04  412  	struct mt76x02_vif *mvif = (struct mt76x02_vif *)vif->drv_priv;
> 60c26859e863c1 Stanislaw Gruszka 2018-09-04  413  	struct mt76x02_sta *msta;
> 60c26859e863c1 Stanislaw Gruszka 2018-09-04  414  	struct mt76_wcid *wcid;
> 60c26859e863c1 Stanislaw Gruszka 2018-09-04 @415  	int idx = key->keyidx;
>                                                                   ^^^^^^^^^^^
> Dereference
>
> 60c26859e863c1 Stanislaw Gruszka 2018-09-04  416  	int ret;
> 60c26859e863c1 Stanislaw Gruszka 2018-09-04  417  
> 60c26859e863c1 Stanislaw Gruszka 2018-09-04  418  	/* fall back to sw encryption for unsupported ciphers */
> 60c26859e863c1 Stanislaw Gruszka 2018-09-04  419  	switch (key->cipher) {
> 60c26859e863c1 Stanislaw Gruszka 2018-09-04  420  	case WLAN_CIPHER_SUITE_WEP40:
> 60c26859e863c1 Stanislaw Gruszka 2018-09-04  421  	case WLAN_CIPHER_SUITE_WEP104:
> 60c26859e863c1 Stanislaw Gruszka 2018-09-04  422  	case WLAN_CIPHER_SUITE_TKIP:
> 60c26859e863c1 Stanislaw Gruszka 2018-09-04  423  	case WLAN_CIPHER_SUITE_CCMP:
> 60c26859e863c1 Stanislaw Gruszka 2018-09-04  424  		break;
> 60c26859e863c1 Stanislaw Gruszka 2018-09-04  425  	default:
> 60c26859e863c1 Stanislaw Gruszka 2018-09-04  426  		return -EOPNOTSUPP;
> 60c26859e863c1 Stanislaw Gruszka 2018-09-04  427  	}
> 60c26859e863c1 Stanislaw Gruszka 2018-09-04  428  
> 60c26859e863c1 Stanislaw Gruszka 2018-09-04  429  	/*
> 60c26859e863c1 Stanislaw Gruszka 2018-09-04  430  	 * The hardware does not support per-STA RX GTK, fall back
> 60c26859e863c1 Stanislaw Gruszka 2018-09-04  431  	 * to software mode for these.
> 60c26859e863c1 Stanislaw Gruszka 2018-09-04  432  	 */
> 60c26859e863c1 Stanislaw Gruszka 2018-09-04  433  	if ((vif->type == NL80211_IFTYPE_ADHOC ||
> 60c26859e863c1 Stanislaw Gruszka 2018-09-04  434  	     vif->type == NL80211_IFTYPE_MESH_POINT) &&
> 60c26859e863c1 Stanislaw Gruszka 2018-09-04  435  	    (key->cipher == WLAN_CIPHER_SUITE_TKIP ||
> 60c26859e863c1 Stanislaw Gruszka 2018-09-04  436  	     key->cipher == WLAN_CIPHER_SUITE_CCMP) &&
> 60c26859e863c1 Stanislaw Gruszka 2018-09-04  437  	    !(key->flags & IEEE80211_KEY_FLAG_PAIRWISE))
> 60c26859e863c1 Stanislaw Gruszka 2018-09-04  438  		return -EOPNOTSUPP;
> 60c26859e863c1 Stanislaw Gruszka 2018-09-04  439  
> b98558e2529986 Stanislaw Gruszka 2019-03-19  440  	/*
> b98558e2529986 Stanislaw Gruszka 2019-03-19  441  	 * In USB AP mode, broadcast/multicast frames are setup in beacon
> b98558e2529986 Stanislaw Gruszka 2019-03-19  442  	 * data registers and sent via HW beacons engine, they require to
> b98558e2529986 Stanislaw Gruszka 2019-03-19  443  	 * be already encrypted.
> b98558e2529986 Stanislaw Gruszka 2019-03-19  444  	 */
> 61c51a74a4e586 Lorenzo Bianconi  2019-10-29  445  	if (mt76_is_usb(&dev->mt76) &&
> b98558e2529986 Stanislaw Gruszka 2019-03-19  446  	    vif->type == NL80211_IFTYPE_AP &&
> b98558e2529986 Stanislaw Gruszka 2019-03-19  447  	    !(key->flags & IEEE80211_KEY_FLAG_PAIRWISE))
> b98558e2529986 Stanislaw Gruszka 2019-03-19  448  		return -EOPNOTSUPP;
> b98558e2529986 Stanislaw Gruszka 2019-03-19  449  
> 4b36cc6b390f18 David Bauer       2021-02-07  450  	/* MT76x0 GTK offloading does not work with more than one VIF */
> 4b36cc6b390f18 David Bauer       2021-02-07  451  	if (is_mt76x0(dev) && !(key->flags & IEEE80211_KEY_FLAG_PAIRWISE))
> 4b36cc6b390f18 David Bauer       2021-02-07  452  		return -EOPNOTSUPP;
> 4b36cc6b390f18 David Bauer       2021-02-07  453  
> 60c26859e863c1 Stanislaw Gruszka 2018-09-04  454  	msta = sta ? (struct mt76x02_sta *)sta->drv_priv : NULL;
> 60c26859e863c1 Stanislaw Gruszka 2018-09-04  455  	wcid = msta ? &msta->wcid : &mvif->group_wcid;
> 60c26859e863c1 Stanislaw Gruszka 2018-09-04  456  
> e6db67fa871dee Felix Fietkau     2023-03-30  457  	if (cmd != SET_KEY) {
> 60c26859e863c1 Stanislaw Gruszka 2018-09-04  458  		if (idx == wcid->hw_key_idx) {
> 60c26859e863c1 Stanislaw Gruszka 2018-09-04  459  			wcid->hw_key_idx = -1;
> f2f6a47b504b8f Felix Fietkau     2019-01-25  460  			wcid->sw_iv = false;
> 60c26859e863c1 Stanislaw Gruszka 2018-09-04  461  		}
> 60c26859e863c1 Stanislaw Gruszka 2018-09-04  462  
> e6db67fa871dee Felix Fietkau     2023-03-30  463  		return 0;
> e6db67fa871dee Felix Fietkau     2023-03-30  464  	}
> e6db67fa871dee Felix Fietkau     2023-03-30  465  
> e6db67fa871dee Felix Fietkau     2023-03-30  466  	key->hw_key_idx = wcid->idx;
> e6db67fa871dee Felix Fietkau     2023-03-30  467  	wcid->hw_key_idx = idx;
> e6db67fa871dee Felix Fietkau     2023-03-30  468  	if (key->flags & IEEE80211_KEY_FLAG_RX_MGMT) {
> e6db67fa871dee Felix Fietkau     2023-03-30  469  		key->flags |= IEEE80211_KEY_FLAG_SW_MGMT_TX;
> e6db67fa871dee Felix Fietkau     2023-03-30  470  		wcid->sw_iv = true;
> 60c26859e863c1 Stanislaw Gruszka 2018-09-04  471  	}
> d87cf75f111183 Lorenzo Bianconi  2018-10-07  472  	mt76_wcid_key_setup(&dev->mt76, wcid, key);
> 60c26859e863c1 Stanislaw Gruszka 2018-09-04  473  
> 60c26859e863c1 Stanislaw Gruszka 2018-09-04  474  	if (!msta) {
> 60c26859e863c1 Stanislaw Gruszka 2018-09-04 @475  		if (key || wcid->hw_key_idx == idx) {
>                                                                     ^^^
> Check for NULL.  I think "key" can't be NULL so this check is always
> true.  The check can be removed and the code pulled in an tab.
>
> 8d66af49a3db9a Lorenzo Bianconi  2018-10-07  476  			ret = mt76x02_mac_wcid_set_key(dev, wcid->idx, key);
> 60c26859e863c1 Stanislaw Gruszka 2018-09-04  477  			if (ret)
> 60c26859e863c1 Stanislaw Gruszka 2018-09-04  478  				return ret;
> 60c26859e863c1 Stanislaw Gruszka 2018-09-04  479  		}
> 60c26859e863c1 Stanislaw Gruszka 2018-09-04  480  
> 8d66af49a3db9a Lorenzo Bianconi  2018-10-07  481  		return mt76x02_mac_shared_key_setup(dev, mvif->idx, idx, key);
> 60c26859e863c1 Stanislaw Gruszka 2018-09-04  482  	}
> 60c26859e863c1 Stanislaw Gruszka 2018-09-04  483  
> 8d66af49a3db9a Lorenzo Bianconi  2018-10-07  484  	return mt76x02_mac_wcid_set_key(dev, msta->wcid.idx, key);
> 60c26859e863c1 Stanislaw Gruszka 2018-09-04  485  }

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2023-10-12 10:28 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <77fee326-963a-40eb-80c9-2788a9ff9c22@kadam.mountain>
2023-10-12 10:27 ` drivers/net/wireless/mediatek/mt76/mt76x02_util.c:475 mt76x02_set_key() warn: variable dereferenced before check 'key' (see line 415) Kalle Valo
     [not found] <2f121202-5846-44a9-8b83-e2ba1fa671d0@kadam.mountain>
2023-06-06 13:42 ` Kalle Valo
2023-06-06 16:13   ` Lorenzo Bianconi

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).