From: Kalle Valo <kvalo@kernel.org>
To: Dan Carpenter <dan.carpenter@linaro.org>
Cc: oe-kbuild@lists.linux.dev, Felix Fietkau <nbd@nbd.name>,
lkp@intel.com, oe-kbuild-all@lists.linux.dev,
linux-kernel@vger.kernel.org, linux-wireless@vger.kernel.org
Subject: Re: drivers/net/wireless/mediatek/mt76/mt76x02_util.c:475 mt76x02_set_key() warn: variable dereferenced before check 'key' (see line 415)
Date: Thu, 12 Oct 2023 13:27:59 +0300 [thread overview]
Message-ID: <871qe03zkg.fsf@kernel.org> (raw)
In-Reply-To: <77fee326-963a-40eb-80c9-2788a9ff9c22@kadam.mountain> (Dan Carpenter's message of "Thu, 12 Oct 2023 09:40:39 +0300")
(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
next parent reply other threads:[~2023-10-12 10:28 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <77fee326-963a-40eb-80c9-2788a9ff9c22@kadam.mountain>
2023-10-12 10:27 ` Kalle Valo [this message]
[not found] <2f121202-5846-44a9-8b83-e2ba1fa671d0@kadam.mountain>
2023-06-06 13:42 ` drivers/net/wireless/mediatek/mt76/mt76x02_util.c:475 mt76x02_set_key() warn: variable dereferenced before check 'key' (see line 415) Kalle Valo
2023-06-06 16:13 ` Lorenzo Bianconi
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=871qe03zkg.fsf@kernel.org \
--to=kvalo@kernel.org \
--cc=dan.carpenter@linaro.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-wireless@vger.kernel.org \
--cc=lkp@intel.com \
--cc=nbd@nbd.name \
--cc=oe-kbuild-all@lists.linux.dev \
--cc=oe-kbuild@lists.linux.dev \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).