From: Lorenzo Bianconi <lorenzo@kernel.org>
To: Dan Carpenter <dan.carpenter@linaro.org>
Cc: linux-wireless@vger.kernel.org, linux-mediatek@lists.infradead.org
Subject: Re: [bug report] wifi: mt76: mt7996: Set proper link destination address in mt7996_tx()
Date: Tue, 23 Sep 2025 23:17:10 +0200 [thread overview]
Message-ID: <aNMOVtzQGfPk0A5a@lore-desk> (raw)
In-Reply-To: <aNJTl89jpYob5XaR@stanley.mountain>
[-- Attachment #1: Type: text/plain, Size: 5313 bytes --]
> Hello Lorenzo Bianconi,
Hi Dan,
>
> Commit f940c9b7aef6 ("wifi: mt76: mt7996: Set proper link destination
> address in mt7996_tx()") from Jul 31, 2025 (linux-next), leads to the
> following Smatch static checker warning:
>
> drivers/net/wireless/mediatek/mt76/mt7996/main.c:1344 mt7996_tx()
> error: testing array offset 'link_id' after use.
>
> drivers/net/wireless/mediatek/mt76/mt7996/main.c
> 1288 static void mt7996_tx(struct ieee80211_hw *hw,
> 1289 struct ieee80211_tx_control *control,
> 1290 struct sk_buff *skb)
> 1291 {
> 1292 struct ieee80211_hdr *hdr = (struct ieee80211_hdr *)skb->data;
> 1293 struct mt7996_dev *dev = mt7996_hw_dev(hw);
> 1294 struct ieee80211_sta *sta = control->sta;
> 1295 struct mt7996_sta *msta = sta ? (void *)sta->drv_priv : NULL;
> 1296 struct mt76_phy *mphy = hw->priv;
> 1297 struct ieee80211_tx_info *info = IEEE80211_SKB_CB(skb);
> 1298 struct ieee80211_vif *vif = info->control.vif;
> 1299 struct mt7996_vif *mvif = vif ? (void *)vif->drv_priv : NULL;
> 1300 struct mt76_wcid *wcid = &dev->mt76.global_wcid;
> 1301 u8 link_id = u32_get_bits(info->control.flags,
> 1302 IEEE80211_TX_CTRL_MLO_LINK);
> 1303
> 1304 rcu_read_lock();
> 1305
> 1306 /* Use primary link_id if the value from mac80211 is set to
> 1307 * IEEE80211_LINK_UNSPECIFIED.
> 1308 */
> 1309 if (link_id == IEEE80211_LINK_UNSPECIFIED) {
> 1310 if (msta)
> 1311 link_id = msta->deflink_id;
> 1312 else if (mvif)
> 1313 link_id = mvif->mt76.deflink_id;
>
> Can link_id be IEEE80211_LINK_UNSPECIFIED after this if statement?
>
> 1314 }
> 1315
> 1316 if (vif && ieee80211_vif_is_mld(vif)) {
> 1317 struct ieee80211_bss_conf *link_conf;
> 1318
> 1319 if (msta) {
> 1320 struct ieee80211_link_sta *link_sta;
> 1321
> 1322 link_sta = rcu_dereference(sta->link[link_id]);
>
> Some unchecked uses. IEEE80211_LINK_UNSPECIFIED would be off-by-one.
>
> 1323 if (!link_sta)
> 1324 link_sta = rcu_dereference(sta->link[msta->deflink_id]);
> 1325
> 1326 if (link_sta) {
> 1327 memcpy(hdr->addr1, link_sta->addr, ETH_ALEN);
> 1328 if (ether_addr_equal(sta->addr, hdr->addr3))
> 1329 memcpy(hdr->addr3, link_sta->addr, ETH_ALEN);
> 1330 }
> 1331 }
> 1332
> 1333 link_conf = rcu_dereference(vif->link_conf[link_id]);
>
> Here too.
>
> 1334 if (link_conf) {
> 1335 memcpy(hdr->addr2, link_conf->addr, ETH_ALEN);
> 1336 if (ether_addr_equal(vif->addr, hdr->addr3))
> 1337 memcpy(hdr->addr3, link_conf->addr, ETH_ALEN);
> 1338 }
> 1339 }
> 1340
> 1341 if (mvif) {
> 1342 struct mt76_vif_link *mlink = &mvif->deflink.mt76;
> 1343
> --> 1344 if (link_id < IEEE80211_LINK_UNSPECIFIED)
>
> Is this checker required?
I agree, we can get rid of this condition since if mvif (or msta) is not NULL,
link_id can't be set to IEEE80211_LINK_UNSPECIFIED. I will post a fix for it.
Regards,
Lorenzo
>
> 1345 mlink = rcu_dereference(mvif->mt76.link[link_id]);
> 1346
> 1347 if (mlink->wcid)
> 1348 wcid = mlink->wcid;
> 1349
> 1350 if (mvif->mt76.roc_phy &&
> 1351 (info->flags & IEEE80211_TX_CTL_TX_OFFCHAN)) {
> 1352 mphy = mvif->mt76.roc_phy;
> 1353 if (mphy->roc_link)
> 1354 wcid = mphy->roc_link->wcid;
> 1355 } else {
> 1356 mphy = mt76_vif_link_phy(mlink);
> 1357 }
> 1358 }
> 1359
> 1360 if (!mphy) {
> 1361 ieee80211_free_txskb(hw, skb);
> 1362 goto unlock;
> 1363 }
> 1364
> 1365 if (msta && link_id < IEEE80211_LINK_UNSPECIFIED) {
>
> And this?
>
> 1366 struct mt7996_sta_link *msta_link;
> 1367
> 1368 msta_link = rcu_dereference(msta->link[link_id]);
> 1369 if (msta_link)
> 1370 wcid = &msta_link->wcid;
> 1371 }
> 1372 mt76_tx(mphy, control->sta, wcid, skb);
> 1373 unlock:
> 1374 rcu_read_unlock();
> 1375 }
>
> regards,
> dan carpenter
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
prev parent reply other threads:[~2025-09-23 21:17 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-09-23 8:00 [bug report] wifi: mt76: mt7996: Set proper link destination address in mt7996_tx() Dan Carpenter
2025-09-23 21:17 ` Lorenzo Bianconi [this message]
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=aNMOVtzQGfPk0A5a@lore-desk \
--to=lorenzo@kernel.org \
--cc=dan.carpenter@linaro.org \
--cc=linux-mediatek@lists.infradead.org \
--cc=linux-wireless@vger.kernel.org \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.