[bug report] wifi: mt76: mt7996: Rely on mt7996_sta_link in sta_add/sta_remove callbacks

[bug report] wifi: mt76: mt7996: Rely on mt7996_sta_link in sta_add/sta_remove callbacks

[Dan Carpenter]

Hello Lorenzo Bianconi,

Commit dd82a9e02c05 ("wifi: mt76: mt7996: Rely on mt7996_sta_link in
sta_add/sta_remove callbacks") from Mar 11, 2025 (linux-next), leads
to the following Smatch static checker warning:

	drivers/net/wireless/mediatek/mt76/mt7996/main.c:1023 mt7996_mac_sta_add_links()
	error: uninitialized symbol 'err'.

drivers/net/wireless/mediatek/mt76/mt7996/main.c
    984 static int
    985 mt7996_mac_sta_add_links(struct mt7996_dev *dev, struct ieee80211_vif *vif,
    986                          struct ieee80211_sta *sta, unsigned long new_links)
    987 {
    988         struct mt7996_sta *msta = (struct mt7996_sta *)sta->drv_priv;
    989         unsigned int link_id;
    990         int err;
    991 
    992         for_each_set_bit(link_id, &new_links, IEEE80211_MLD_MAX_NUM_LINKS) {
    993                 struct ieee80211_bss_conf *link_conf;
    994                 struct ieee80211_link_sta *link_sta;
    995                 struct mt7996_vif_link *link;
    996 
    997                 if (rcu_access_pointer(msta->link[link_id]))
    998                         continue;

What about if there are no new links or we hit the continue on every
iteration.

    999 
    1000                 link_conf = link_conf_dereference_protected(vif, link_id);
    1001                 if (!link_conf)
    1002                         goto error_unlink;

error code?

    1003 
    1004                 link = mt7996_vif_link(dev, vif, link_id);
    1005                 if (!link)
    1006                         goto error_unlink;

error code?

    1007 
    1008                 link_sta = link_sta_dereference_protected(sta, link_id);
    1009                 if (!link_sta)
    1010                         goto error_unlink;

error code?

    1011 
    1012                 err = mt7996_mac_sta_init_link(dev, link_conf, link_sta, link,
    1013                                                link_id);
    1014                 if (err)
    1015                         goto error_unlink;
    1016         }
    1017 
    1018         return 0;
    1019 
    1020 error_unlink:
    1021         mt7996_mac_sta_remove_links(dev, sta, new_links);
    1022 
--> 1023         return err;
    1024 }

regards,
dan carpenter

Re: [bug report] wifi: mt76: mt7996: Rely on mt7996_sta_link in sta_add/sta_remove callbacks

[Lorenzo Bianconi]

[-- Attachment #1: Type: text/plain, Size: 2627 bytes --]

> Hello Lorenzo Bianconi,

Hi Dan,

this issue is already fixed by the following patch:
https://lore.kernel.org/linux-wireless/20250320201914.48159-1-qasdev00@gmail.com/T/#u

Regards,
Lorenzo

> 
> Commit dd82a9e02c05 ("wifi: mt76: mt7996: Rely on mt7996_sta_link in
> sta_add/sta_remove callbacks") from Mar 11, 2025 (linux-next), leads
> to the following Smatch static checker warning:
> 
> 	drivers/net/wireless/mediatek/mt76/mt7996/main.c:1023 mt7996_mac_sta_add_links()
> 	error: uninitialized symbol 'err'.
> 
> drivers/net/wireless/mediatek/mt76/mt7996/main.c
>     984 static int
>     985 mt7996_mac_sta_add_links(struct mt7996_dev *dev, struct ieee80211_vif *vif,
>     986                          struct ieee80211_sta *sta, unsigned long new_links)
>     987 {
>     988         struct mt7996_sta *msta = (struct mt7996_sta *)sta->drv_priv;
>     989         unsigned int link_id;
>     990         int err;
>     991 
>     992         for_each_set_bit(link_id, &new_links, IEEE80211_MLD_MAX_NUM_LINKS) {
>     993                 struct ieee80211_bss_conf *link_conf;
>     994                 struct ieee80211_link_sta *link_sta;
>     995                 struct mt7996_vif_link *link;
>     996 
>     997                 if (rcu_access_pointer(msta->link[link_id]))
>     998                         continue;
> 
> What about if there are no new links or we hit the continue on every
> iteration.
> 
>     999 
>     1000                 link_conf = link_conf_dereference_protected(vif, link_id);
>     1001                 if (!link_conf)
>     1002                         goto error_unlink;
> 
> error code?
> 
>     1003 
>     1004                 link = mt7996_vif_link(dev, vif, link_id);
>     1005                 if (!link)
>     1006                         goto error_unlink;
> 
> error code?
> 
>     1007 
>     1008                 link_sta = link_sta_dereference_protected(sta, link_id);
>     1009                 if (!link_sta)
>     1010                         goto error_unlink;
> 
> error code?
> 
>     1011 
>     1012                 err = mt7996_mac_sta_init_link(dev, link_conf, link_sta, link,
>     1013                                                link_id);
>     1014                 if (err)
>     1015                         goto error_unlink;
>     1016         }
>     1017 
>     1018         return 0;
>     1019 
>     1020 error_unlink:
>     1021         mt7996_mac_sta_remove_links(dev, sta, new_links);
>     1022 
> --> 1023         return err;
>     1024 }
> 
> regards,
> dan carpenter

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]