[bug report] wifi: nl80211/mac80211: clarify link ID in control port TX

[Dan Carpenter <dan.carpenter@oracle.com>]

Hello Johannes Berg,

The patch 9dd1953846c7: "wifi: nl80211/mac80211: clarify link ID in
control port TX" from Jul 19, 2022, leads to the following Smatch
static checker warning:

	net/mac80211/tx.c:5902 ieee80211_tx_control_port()
	error: potential NULL/IS_ERR bug 'sta'

net/mac80211/tx.c
    5807 int ieee80211_tx_control_port(struct wiphy *wiphy, struct net_device *dev,
    5808                               const u8 *buf, size_t len,
    5809                               const u8 *dest, __be16 proto, bool unencrypted,
    5810                               int link_id, u64 *cookie)
    5811 {
    5812         struct ieee80211_sub_if_data *sdata = IEEE80211_DEV_TO_SUB_IF(dev);
    5813         struct ieee80211_local *local = sdata->local;
    5814         struct sta_info *sta;
    5815         struct sk_buff *skb;
    5816         struct ethhdr *ehdr;
    5817         u32 ctrl_flags = 0;
    5818         u32 flags = 0;
    5819         int err;
    5820 
    5821         /* Only accept CONTROL_PORT_PROTOCOL configured in CONNECT/ASSOCIATE
    5822          * or Pre-Authentication
    5823          */
    5824         if (proto != sdata->control_port_protocol &&
    5825             proto != cpu_to_be16(ETH_P_PREAUTH))
    5826                 return -EINVAL;
    5827 
    5828         if (proto == sdata->control_port_protocol)
    5829                 ctrl_flags |= IEEE80211_TX_CTRL_PORT_CTRL_PROTO |
    5830                               IEEE80211_TX_CTRL_SKIP_MPATH_LOOKUP;
    5831 
    5832         if (unencrypted)
    5833                 flags |= IEEE80211_TX_INTFL_DONT_ENCRYPT;
    5834 
    5835         if (cookie)
    5836                 ctrl_flags |= IEEE80211_TX_CTL_REQ_TX_STATUS;
    5837 
    5838         flags |= IEEE80211_TX_INTFL_NL80211_FRAME_TX;
    5839 
    5840         skb = dev_alloc_skb(local->hw.extra_tx_headroom +
    5841                             sizeof(struct ethhdr) + len);
    5842         if (!skb)
    5843                 return -ENOMEM;
    5844 
    5845         skb_reserve(skb, local->hw.extra_tx_headroom + sizeof(struct ethhdr));
    5846 
    5847         skb_put_data(skb, buf, len);
    5848 
    5849         ehdr = skb_push(skb, sizeof(struct ethhdr));
    5850         memcpy(ehdr->h_dest, dest, ETH_ALEN);
    5851 
    5852         /* we may override the SA for MLO STA later */
    5853         if (link_id < 0) {
    5854                 ctrl_flags |= u32_encode_bits(IEEE80211_LINK_UNSPECIFIED,
    5855                                               IEEE80211_TX_CTRL_MLO_LINK);
    5856                 memcpy(ehdr->h_source, sdata->vif.addr, ETH_ALEN);
    5857         } else {
    5858                 struct ieee80211_bss_conf *link_conf;
    5859 
    5860                 ctrl_flags |= u32_encode_bits(link_id,
    5861                                               IEEE80211_TX_CTRL_MLO_LINK);
    5862 
    5863                 rcu_read_lock();
    5864                 link_conf = rcu_dereference(sdata->vif.link_conf[link_id]);
    5865                 if (!link_conf) {
    5866                         dev_kfree_skb(skb);
    5867                         rcu_read_unlock();
    5868                         return -ENOLINK;
    5869                 }
    5870                 memcpy(ehdr->h_source, link_conf->addr, ETH_ALEN);
    5871                 rcu_read_unlock();
    5872         }
    5873 
    5874         ehdr->h_proto = proto;
    5875 
    5876         skb->dev = dev;
    5877         skb->protocol = proto;
    5878         skb_reset_network_header(skb);
    5879         skb_reset_mac_header(skb);
    5880 
    5881         /* update QoS header to prioritize control port frames if possible,
    5882          * priorization also happens for control port frames send over
    5883          * AF_PACKET
    5884          */
    5885         rcu_read_lock();
    5886         err = ieee80211_lookup_ra_sta(sdata, skb, &sta);

For sdata->vif.type == NL80211_IFTYPE_MESH_POINT then "sta" can be NULL.
Smatch doesn't know the value of sdata->vif.type at this point, and even
if it did, then it doesn't split the return states up with enough
granularity for that to make a difference.

    5887         if (err) {
    5888                 rcu_read_unlock();
    5889                 return err;
    5890         }
    5891 
    5892         if (!IS_ERR(sta)) {
    5893                 u16 queue = __ieee80211_select_queue(sdata, sta, skb);
    5894 
    5895                 skb_set_queue_mapping(skb, queue);
    5896                 skb_get_hash(skb);
    5897 
    5898                 /*
    5899                  * for MLO STA, the SA should be the AP MLD address, but
    5900                  * the link ID has been selected already
    5901                  */
--> 5902                 if (sta->sta.mlo)

Which Smatch complains about here.

    5903                         memcpy(ehdr->h_source, sdata->vif.addr, ETH_ALEN);
    5904         }
    5905         rcu_read_unlock();
    5906 
    5907         /* mutex lock is only needed for incrementing the cookie counter */
    5908         mutex_lock(&local->mtx);
    5909 
    5910         local_bh_disable();
    5911         __ieee80211_subif_start_xmit(skb, skb->dev, flags, ctrl_flags, cookie);
    5912         local_bh_enable();
    5913 
    5914         mutex_unlock(&local->mtx);
    5915 
    5916         return 0;
    5917 }

regards,
dan carpenter