[bug report] mt76: adjust wcid size to support new 802.11ax generation

* [bug report] mt76: adjust wcid size to support new 802.11ax generation
@ 2020-05-20 12:08 ` Dan Carpenter
  0 siblings, 0 replies; 2+ messages in thread
From: Dan Carpenter @ 2020-05-20 12:08 UTC (permalink / raw)
  To: ryder.lee; +Cc: Ryder Lee, linux-wireless, linux-mediatek

Hello Ryder Lee,

The patch 49e649c3e0a6: "mt76: adjust wcid size to support new
802.11ax generation" from Apr 25, 2020, leads to the following static
checker warning:

    drivers/net/wireless/mediatek/mt76/mt76x02.h:249 mt76x02_rx_get_sta()
    warn: impossible condition '(idx >= (2304 / 8 + (0))) => (0-255 >= 288)'

    drivers/net/wireless/mediatek/mt76/mt7603/mac.c:476 mt7603_rx_get_wcid()
    warn: impossible condition '(idx >= (2304 / 8 + (0))) => (0-255 >= 288)'

    drivers/net/wireless/mediatek/mt76/mt7603/mac.c:1241 mt7603_mac_add_txs()
    warn: impossible condition '(wcidx >= (2304 / 8 + (0))) => (0-255 >= 288)'

    drivers/net/wireless/mediatek/mt76/mt7615/mac.c:64 mt7615_rx_get_wcid()
    warn: impossible condition '(idx >= (2304 / 8 + (0))) => (0-255 >= 288)'

    drivers/net/wireless/mediatek/mt76/mt7615/mac.c:1305 mt7615_mac_add_txs()
    warn: impossible condition '(wcidx >= (2304 / 8 + (0))) => (0-255 >= 288)'

    drivers/net/wireless/mediatek/mt76/mt76x02_mac.c:564 mt76x02_send_tx_status()
    warn: always true condition '(stat->wcid < (2304 / 8 + (0))) => (0-255 < 288)'

vers/net/wireless/mediatek/mt76/mt76x02.h
   244  static inline struct mt76x02_sta *
   245  mt76x02_rx_get_sta(struct mt76_dev *dev, u8 idx)
   246  {
   247          struct mt76_wcid *wcid;
   248  
   249          if (idx >= ARRAY_SIZE(dev->wcid))

The dev->wcid[] array used to have 128 elements but now it has 288 so
a u8 idx isn't large enough.

   250                  return NULL;
   251  
   252          wcid = rcu_dereference(dev->wcid[idx]);
   253          if (!wcid)
   254                  return NULL;
   255  
   256          return container_of(wcid, struct mt76x02_sta, wcid);
   257  }
...
   470  static struct mt76_wcid *
   471  mt7603_rx_get_wcid(struct mt7603_dev *dev, u8 idx, bool unicast)
   472  {
   473          struct mt7603_sta *sta;
   474          struct mt76_wcid *wcid;
   475  
   476          if (idx >= ARRAY_SIZE(dev->mt76.wcid))
                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Same.

   477                  return NULL;
   478  
   479          wcid = rcu_dereference(dev->mt76.wcid[idx]);
   480          if (unicast || !wcid)
   481                  return wcid;
   482  
   483          if (!wcid->sta)
   484                  return NULL;
   485  
   486          sta = container_of(wcid, struct mt7603_sta, wcid);
   487          if (!sta->vif)
   488                  return NULL;
   489  
   490          return &sta->vif->sta.wcid;
   491  }
...
  1236          wcidx = FIELD_GET(MT_TXS3_WCID, txs);
                                  ^^^^^^^^^^^^
This is bits 24-31 so it may need to be adjusted.

  1237  
  1238          if (pid == MT_PACKET_ID_NO_ACK)
  1239                  return;
  1240  
  1241          if (wcidx >= ARRAY_SIZE(dev->mt76.wcid))
                    ^^^^^
u8 type.

  1242                  return;
  1243  
  1244          rcu_read_lock();

drivers/net/wireless/mediatek/mt76/mt7615/mac.c
    58  static struct mt76_wcid *mt7615_rx_get_wcid(struct mt7615_dev *dev,
    59                                              u8 idx, bool unicast)
    60  {
    61          struct mt7615_sta *sta;
    62          struct mt76_wcid *wcid;
    63  
    64          if (idx >= ARRAY_SIZE(dev->mt76.wcid))
                    ^^^
It's the same thing.  The type and the mask used (MT_RXD2_NORMAL_WLAN_IDX)
make this condition impossible.

    65                  return NULL;
    66  

See drivers/net/wireless/mediatek/mt76/mt7615/mac.c:1305 mt7615_mac_add_txs() warn: impossible condition '(wcidx >= (2304 / 8 + (0))) => (0-255 >= 288)'

drivers/net/wireless/mediatek/mt76/mt76x02_mac.c
   551          struct mt76x02_sta *msta = NULL;
   552          struct mt76_dev *mdev = &dev->mt76;
   553          struct sk_buff_head list;
   554          u32 duration = 0;
   555          u8 cur_pktid;
   556          u32 ac = 0;
   557          int len = 0;
   558  
   559          if (stat->pktid == MT_PACKET_ID_NO_ACK)
   560                  return;
   561  
   562          rcu_read_lock();
   563  
   564          if (stat->wcid < ARRAY_SIZE(dev->mt76.wcid))
                    ^^^^^^^^^^
This is a u8 as well.

   565                  wcid = rcu_dereference(dev->mt76.wcid[stat->wcid]);
   566  
   567          if (wcid && wcid->sta) {
   568                  void *priv;
   569  
   570                  priv = msta = container_of(wcid, struct mt76x02_sta, wcid);
   571                  status.sta = container_of(priv, struct ieee80211_sta,
   572                                            drv_priv);
   573          }
...
   796  
   797          wcid = FIELD_GET(MT_RXWI_CTL_WCID, ctl);
                ^^^^             ^^^^^^^^^^^^^^^^
Here "wcid" is a u8 and MT_RXWI_CTL_WCID is bits 0-7 so both of these
may need to be adjusted.

   798          sta = mt76x02_rx_get_sta(&dev->mt76, wcid);
   799          status->wcid = mt76x02_rx_get_sta_wcid(sta, unicast);
   800  

regards,
dan carpenter

^ permalink raw reply	[flat|nested] 2+ messages in thread