Re: drivers/net/wireless/mediatek/mt76/mt76x02_util.c:475 mt76x02_set_key() warn: variable dereferenced before check 'key' (see line 415)

From: Lorenzo Bianconi
Date: Tue Jun 06 2023 - 12:14:25 EST

> Adding linux-wireless, top posting so that the whole report is included.
>
> Dan Carpenter <dan.carpenter@xxxxxxxxxx> 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@xxxxxxxxx/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@xxxxxxxxx>
> > | Reported-by: Dan Carpenter <dan.carpenter@xxxxxxxxxx>
> > | Closes: https://lore.kernel.org/r/202306060332.WbIToDHL-lkp@xxxxxxxxx/
> >
> > 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@xxxxxxxxxx/

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: signature.asc
Description: PGP signature