[bug report] wifi: mt76: connac: rework connac helpers

Dan Carpenter dan.carpenter at linaro.org
Mon Jan 20 01:26:56 PST 2025


Hello Shayne Chen,

Commit a0facfc80ec1 ("wifi: mt76: connac: rework connac helpers")
from Jan 2, 2025 (linux-next), leads to the following Smatch static
checker warning:

	drivers/net/wireless/mediatek/mt76/mt76_connac_mac.c:588 mt76_connac2_mac_write_txwi()
	error: we previously assumed 'vif' could be null (see line 510)

drivers/net/wireless/mediatek/mt76/mt76_connac_mac.c
   492  void mt76_connac2_mac_write_txwi(struct mt76_dev *dev, __le32 *txwi,
   493                                   struct sk_buff *skb, struct mt76_wcid *wcid,
   494                                   struct ieee80211_key_conf *key, int pid,
   495                                   enum mt76_txq_id qid, u32 changed)
   496  {
   497          struct ieee80211_tx_info *info = IEEE80211_SKB_CB(skb);
   498          u8 phy_idx = (info->hw_queue & MT_TX_HW_QUEUE_PHY) >> 2;
   499          struct ieee80211_vif *vif = info->control.vif;
   500          struct mt76_phy *mphy = &dev->phy;
   501          u8 p_fmt, q_idx, omac_idx = 0, wmm_idx = 0, band_idx = 0;
   502          u32 val, sz_txd = mt76_is_mmio(dev) ? MT_TXD_SIZE : MT_SDIO_TXD_SIZE;
   503          bool is_8023 = info->flags & IEEE80211_TX_CTL_HW_80211_ENCAP;
   504          bool beacon = !!(changed & (BSS_CHANGED_BEACON |
   505                                      BSS_CHANGED_BEACON_ENABLED));
   506          bool inband_disc = !!(changed & (BSS_CHANGED_UNSOL_BCAST_PROBE_RESP |
   507                                           BSS_CHANGED_FILS_DISCOVERY));
   508          bool amsdu_en = wcid->amsdu;
   509  
   510          if (vif) {
                    ^^^
This code assumes the "vif" can be NULL.

   511                  struct mt76_vif_link *mvif = (struct mt76_vif_link *)vif->drv_priv;
   512  
   513                  omac_idx = mvif->omac_idx;
   514                  wmm_idx = mvif->wmm_idx;
   515                  band_idx = mvif->band_idx;
   516          }
   517  
   518          if (phy_idx && dev->phys[MT_BAND1])
   519                  mphy = dev->phys[MT_BAND1];
   520  
   521          if (inband_disc) {
   522                  p_fmt = MT_TX_TYPE_FW;
   523                  q_idx = MT_LMAC_ALTX0;
   524          } else if (beacon) {
   525                  p_fmt = MT_TX_TYPE_FW;
   526                  q_idx = MT_LMAC_BCN0;
   527          } else if (qid >= MT_TXQ_PSD) {
   528                  p_fmt = mt76_is_mmio(dev) ? MT_TX_TYPE_CT : MT_TX_TYPE_SF;
   529                  q_idx = MT_LMAC_ALTX0;
   530          } else {
   531                  p_fmt = mt76_is_mmio(dev) ? MT_TX_TYPE_CT : MT_TX_TYPE_SF;
   532                  q_idx = wmm_idx * MT76_CONNAC_MAX_WMM_SETS +
   533                          mt76_connac_lmac_mapping(skb_get_queue_mapping(skb));
   534  
   535                  /* mt7915 WA only counts WED path */
   536                  if (is_mt7915(dev) && mtk_wed_device_active(&dev->mmio.wed))
   537                          wcid->stats.tx_packets++;
   538          }
   539  
   540          val = FIELD_PREP(MT_TXD0_TX_BYTES, skb->len + sz_txd) |
   541                FIELD_PREP(MT_TXD0_PKT_FMT, p_fmt) |
   542                FIELD_PREP(MT_TXD0_Q_IDX, q_idx);
   543          txwi[0] = cpu_to_le32(val);
   544  
   545          val = MT_TXD1_LONG_FORMAT |
   546                FIELD_PREP(MT_TXD1_WLAN_IDX, wcid->idx) |
   547                FIELD_PREP(MT_TXD1_OWN_MAC, omac_idx);
   548          if (!is_mt7921(dev))
   549                  val |= MT_TXD1_VTA;
   550          if (phy_idx || band_idx)
   551                  val |= MT_TXD1_TGID;
   552  
   553          txwi[1] = cpu_to_le32(val);
   554          txwi[2] = 0;
   555  
   556          val = FIELD_PREP(MT_TXD3_REM_TX_COUNT, 15);
   557          if (!is_mt7921(dev))
   558                  val |= MT_TXD3_SW_POWER_MGMT;
   559          if (key)
   560                  val |= MT_TXD3_PROTECT_FRAME;
   561          if (info->flags & IEEE80211_TX_CTL_NO_ACK)
   562                  val |= MT_TXD3_NO_ACK;
   563  
   564          txwi[3] = cpu_to_le32(val);
   565          txwi[4] = 0;
   566  
   567          val = FIELD_PREP(MT_TXD5_PID, pid);
   568          if (pid >= MT_PACKET_ID_FIRST) {
   569                  val |= MT_TXD5_TX_STATUS_HOST;
   570                  amsdu_en = 0;
   571          }
   572  
   573          txwi[5] = cpu_to_le32(val);
   574          txwi[6] = 0;
   575          txwi[7] = amsdu_en ? cpu_to_le32(MT_TXD7_HW_AMSDU) : 0;
   576  
   577          if (is_8023)
   578                  mt76_connac2_mac_write_txwi_8023(txwi, skb, wcid);
   579          else
   580                  mt76_connac2_mac_write_txwi_80211(dev, txwi, skb, key);
   581  
   582          if (txwi[2] & cpu_to_le32(MT_TXD2_FIX_RATE)) {
   583                  /* Fixed rata is available just for 802.11 txd */
   584                  struct ieee80211_hdr *hdr = (struct ieee80211_hdr *)skb->data;
   585                  bool multicast = ieee80211_is_data(hdr->frame_control) &&
   586                                   is_multicast_ether_addr(hdr->addr1);
   587                  u16 rate = mt76_connac2_mac_tx_rate_val(mphy, &vif->bss_conf, beacon,
                                                                      ^^^^^^^^^^^^^^
This code dereferences vif without checking.  Technically the
mt76_connac2_mac_tx_rate_val() has a NULL check but it doesn't work
because the address of &vif->bss_conf can't be NULL since it's in the
middle of the struct.

   588                                                          multicast);
   589                  u32 val = MT_TXD6_FIXED_BW;
   590  
   591                  /* hardware won't add HTC for mgmt/ctrl frame */
   592                  txwi[2] |= cpu_to_le32(MT_TXD2_HTC_VLD);
   593  
   594                  val |= FIELD_PREP(MT_TXD6_TX_RATE, rate);
   595                  txwi[6] |= cpu_to_le32(val);
   596                  txwi[3] |= cpu_to_le32(MT_TXD3_BA_DISABLE);
   597  
   598                  if (!is_mt7921(dev)) {
   599                          u8 spe_idx = mt76_connac_spe_idx(mphy->antenna_mask);
   600  
   601                          if (!spe_idx)
   602                                  spe_idx = 24 + phy_idx;
   603                          txwi[7] |= cpu_to_le32(FIELD_PREP(MT_TXD7_SPE_IDX, spe_idx));
   604                  }
   605  
   606                  txwi[7] &= ~cpu_to_le32(MT_TXD7_HW_AMSDU);
   607          }
   608  }


regards,
dan carpenter



More information about the Linux-mediatek mailing list