2021-02-11 13:49:41

by Dan Carpenter

[permalink] [raw]
Subject: [bug report] mt76: mt76_connac: create mcu library

Hello Lorenzo Bianconi,

The patch d0e274af2f2e: "mt76: mt76_connac: create mcu library" from
Jan 26, 2021, leads to the following static checker warning:

drivers/net/wireless/mediatek/mt76/mt76_connac_mcu.c:837 mt76_connac_mcu_add_sta_cmd()
error: 'wtbl_hdr' dereferencing possible ERR_PTR()

drivers/net/wireless/mediatek/mt76/mt76_connac_mcu.c
810 int mt76_connac_mcu_add_sta_cmd(struct mt76_phy *phy,
811 struct ieee80211_vif *vif,
812 struct ieee80211_sta *sta,
813 struct mt76_wcid *wcid,
814 bool enable, int cmd)
815 {
816 struct mt76_vif *mvif = (struct mt76_vif *)vif->drv_priv;
817 struct mt76_dev *dev = phy->dev;
818 struct wtbl_req_hdr *wtbl_hdr;
819 struct tlv *sta_wtbl;
820 struct sk_buff *skb;
821
822 skb = mt76_connac_mcu_alloc_sta_req(dev, mvif, wcid);
823 if (IS_ERR(skb))
824 return PTR_ERR(skb);
825
826 mt76_connac_mcu_sta_basic_tlv(skb, vif, sta, enable);
827 if (enable && sta)
828 mt76_connac_mcu_sta_tlv(phy, skb, sta, vif);
829
830 sta_wtbl = mt76_connac_mcu_add_tlv(skb, STA_REC_WTBL,
831 sizeof(struct tlv));
832
833 wtbl_hdr = mt76_connac_mcu_alloc_wtbl_req(dev, wcid,
834 WTBL_RESET_AND_SET,
835 sta_wtbl, &skb);

if (IS_ERR(wtbl_hdr)) {
do some cleanup?
return PTR_ERR(wtbl_hdr);
}

836 if (enable) {
837 mt76_connac_mcu_wtbl_generic_tlv(dev, skb, vif, sta, sta_wtbl,
838 wtbl_hdr);
839 if (sta)
840 mt76_connac_mcu_wtbl_ht_tlv(dev, skb, sta, sta_wtbl,
841 wtbl_hdr);
842 }
843
844 return mt76_mcu_skb_send_msg(dev, skb, cmd, true);
845 }

regards,
dan carpenter


2021-02-11 15:31:47

by Lorenzo Bianconi

[permalink] [raw]
Subject: Re: [bug report] mt76: mt76_connac: create mcu library

> Hello Lorenzo Bianconi,

Hi Dan,

>
> The patch d0e274af2f2e: "mt76: mt76_connac: create mcu library" from
> Jan 26, 2021, leads to the following static checker warning:
>
> drivers/net/wireless/mediatek/mt76/mt76_connac_mcu.c:837 mt76_connac_mcu_add_sta_cmd()
> error: 'wtbl_hdr' dereferencing possible ERR_PTR()
>
> drivers/net/wireless/mediatek/mt76/mt76_connac_mcu.c
> 810 int mt76_connac_mcu_add_sta_cmd(struct mt76_phy *phy,
> 811 struct ieee80211_vif *vif,
> 812 struct ieee80211_sta *sta,
> 813 struct mt76_wcid *wcid,
> 814 bool enable, int cmd)
> 815 {
> 816 struct mt76_vif *mvif = (struct mt76_vif *)vif->drv_priv;
> 817 struct mt76_dev *dev = phy->dev;
> 818 struct wtbl_req_hdr *wtbl_hdr;
> 819 struct tlv *sta_wtbl;
> 820 struct sk_buff *skb;
> 821
> 822 skb = mt76_connac_mcu_alloc_sta_req(dev, mvif, wcid);
> 823 if (IS_ERR(skb))
> 824 return PTR_ERR(skb);
> 825
> 826 mt76_connac_mcu_sta_basic_tlv(skb, vif, sta, enable);
> 827 if (enable && sta)
> 828 mt76_connac_mcu_sta_tlv(phy, skb, sta, vif);
> 829
> 830 sta_wtbl = mt76_connac_mcu_add_tlv(skb, STA_REC_WTBL,
> 831 sizeof(struct tlv));
> 832
> 833 wtbl_hdr = mt76_connac_mcu_alloc_wtbl_req(dev, wcid,
> 834 WTBL_RESET_AND_SET,
> 835 sta_wtbl, &skb);
>
> if (IS_ERR(wtbl_hdr)) {
> do some cleanup?
> return PTR_ERR(wtbl_hdr);
> }

I think this is a false positive since reviewing
mt76_connac_mcu_alloc_wtbl_req(), it can return ERR_PTR(-ENOMEM) only if nskb
is NULL and it is not the case.

Regards,
Lorenzo

>
> 836 if (enable) {
> 837 mt76_connac_mcu_wtbl_generic_tlv(dev, skb, vif, sta, sta_wtbl,
> 838 wtbl_hdr);
> 839 if (sta)
> 840 mt76_connac_mcu_wtbl_ht_tlv(dev, skb, sta, sta_wtbl,
> 841 wtbl_hdr);
> 842 }
> 843
> 844 return mt76_mcu_skb_send_msg(dev, skb, cmd, true);
> 845 }
>
> regards,
> dan carpenter


Attachments:
(No filename) (2.53 kB)
signature.asc (235.00 B)
Download all attachments