wifi: mt76: mt7996: Set proper link destination address in mt7996_tx()
Hello Lorenzo Bianconi,
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?
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
Quoted 1 line
Hello Lorenzo Bianconi,
Hi Dan,
Quoted 74 lines
Commit
f940c9b7aef6("wifi: mt76: mt7996: Set proper link destination address inmt7996_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.c1288 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_idbeIEEE80211_LINK_UNSPECIFIEDafter 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_UNSPECIFIEDwould 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
Quoted 3 lines
--> 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
Quoted 38 lines
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