[bug report] wifi: mt76: mt7996: Set proper link destination address in mt7996_tx()

[Lorenzo Bianconi]

> 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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 228 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-mediatek/attachments/20250923/b2777062/attachment.sig>