wifi: mwifiex: drop BUG_ON from TX paths
authorDmitry Antipov <dmantipov@yandex.ru>
Wed, 2 Aug 2023 16:07:19 +0000 (19:07 +0300)
committerKalle Valo <kvalo@kernel.org>
Mon, 21 Aug 2023 15:56:27 +0000 (18:56 +0300)
In 'mwifiex_process_sta_txpd()' and 'mwifiex_process_uap_txpd()',
replace 'BUG_ON()' with runtime check, and move all these checks
to 'mwifiex_process_tx()'. This way, both callees may be converted
to 'void', and the caller may be simplified as well.

Suggested-by: Brian Norris <briannorris@chromium.org>
Signed-off-by: Dmitry Antipov <dmantipov@yandex.ru>
Acked-by: Brian Norris <briannorris@chromium.org>
Signed-off-by: Kalle Valo <kvalo@kernel.org>
Link: https://lore.kernel.org/r/20230802160726.85545-5-dmantipov@yandex.ru
drivers/net/wireless/marvell/mwifiex/main.h
drivers/net/wireless/marvell/mwifiex/sta_tx.c
drivers/net/wireless/marvell/mwifiex/txrx.c
drivers/net/wireless/marvell/mwifiex/uap_txrx.c

index 7421e9b..97e7c83 100644 (file)
@@ -1144,8 +1144,10 @@ int mwifiex_process_uap_event(struct mwifiex_private *);
 void mwifiex_delete_all_station_list(struct mwifiex_private *priv);
 void mwifiex_wmm_del_peer_ra_list(struct mwifiex_private *priv,
                                  const u8 *ra_addr);
-void *mwifiex_process_sta_txpd(struct mwifiex_private *, struct sk_buff *skb);
-void *mwifiex_process_uap_txpd(struct mwifiex_private *, struct sk_buff *skb);
+void mwifiex_process_sta_txpd(struct mwifiex_private *priv,
+                             struct sk_buff *skb);
+void mwifiex_process_uap_txpd(struct mwifiex_private *priv,
+                             struct sk_buff *skb);
 int mwifiex_sta_init_cmd(struct mwifiex_private *, u8 first_sta, bool init);
 int mwifiex_cmd_802_11_scan(struct host_cmd_ds_command *cmd,
                            struct mwifiex_scan_cmd_config *scan_cfg);
index d27b6e6..70c2790 100644 (file)
@@ -29,8 +29,8 @@
  *      - Priority specific Tx control
  *      - Flags
  */
-void *mwifiex_process_sta_txpd(struct mwifiex_private *priv,
-                               struct sk_buff *skb)
+void mwifiex_process_sta_txpd(struct mwifiex_private *priv,
+                             struct sk_buff *skb)
 {
        struct mwifiex_adapter *adapter = priv->adapter;
        struct txpd *local_tx_pd;
@@ -39,14 +39,6 @@ void *mwifiex_process_sta_txpd(struct mwifiex_private *priv,
        u16 pkt_type, pkt_offset;
        int hroom = adapter->intf_hdr_len;
 
-       if (!skb->len) {
-               mwifiex_dbg(adapter, ERROR,
-                           "Tx: bad packet length: %d\n", skb->len);
-               return skb->data;
-       }
-
-       BUG_ON(skb_headroom(skb) < MWIFIEX_MIN_DATA_HEADER_LEN);
-
        pkt_type = mwifiex_is_skb_mgmt_frame(skb) ? PKT_TYPE_MGMT : 0;
 
        pad = ((uintptr_t)skb->data - (sizeof(*local_tx_pd) + hroom)) &
@@ -108,8 +100,6 @@ void *mwifiex_process_sta_txpd(struct mwifiex_private *priv,
        if (!local_tx_pd->tx_control)
                /* TxCtrl set by user or default */
                local_tx_pd->tx_control = cpu_to_le32(priv->pkt_tx_ctrl);
-
-       return skb->data;
 }
 
 /*
index 54c2046..bd91678 100644 (file)
@@ -72,13 +72,18 @@ EXPORT_SYMBOL_GPL(mwifiex_handle_rx_packet);
 int mwifiex_process_tx(struct mwifiex_private *priv, struct sk_buff *skb,
                       struct mwifiex_tx_param *tx_param)
 {
-       int hroom, ret = -1;
+       int hroom, ret;
        struct mwifiex_adapter *adapter = priv->adapter;
-       u8 *head_ptr;
        struct txpd *local_tx_pd = NULL;
        struct mwifiex_sta_node *dest_node;
        struct ethhdr *hdr = (void *)skb->data;
 
+       if (unlikely(!skb->len ||
+                    skb_headroom(skb) < MWIFIEX_MIN_DATA_HEADER_LEN)) {
+               ret = -EINVAL;
+               goto out;
+       }
+
        hroom = adapter->intf_hdr_len;
 
        if (priv->bss_role == MWIFIEX_BSS_ROLE_UAP) {
@@ -88,33 +93,31 @@ int mwifiex_process_tx(struct mwifiex_private *priv, struct sk_buff *skb,
                        dest_node->stats.tx_packets++;
                }
 
-               head_ptr = mwifiex_process_uap_txpd(priv, skb);
+               mwifiex_process_uap_txpd(priv, skb);
        } else {
-               head_ptr = mwifiex_process_sta_txpd(priv, skb);
+               mwifiex_process_sta_txpd(priv, skb);
        }
 
-       if ((adapter->data_sent || adapter->tx_lock_flag) && head_ptr) {
+       if (adapter->data_sent || adapter->tx_lock_flag) {
                skb_queue_tail(&adapter->tx_data_q, skb);
                atomic_inc(&adapter->tx_queued);
                return 0;
        }
 
-       if (head_ptr) {
-               if (GET_BSS_ROLE(priv) == MWIFIEX_BSS_ROLE_STA)
-                       local_tx_pd = (struct txpd *)(head_ptr + hroom);
-               if (adapter->iface_type == MWIFIEX_USB) {
-                       ret = adapter->if_ops.host_to_card(adapter,
-                                                          priv->usb_port,
-                                                          skb, tx_param);
-               } else {
-                       ret = adapter->if_ops.host_to_card(adapter,
-                                                          MWIFIEX_TYPE_DATA,
-                                                          skb, tx_param);
-               }
+       if (GET_BSS_ROLE(priv) == MWIFIEX_BSS_ROLE_STA)
+               local_tx_pd = (struct txpd *)(skb->data + hroom);
+       if (adapter->iface_type == MWIFIEX_USB) {
+               ret = adapter->if_ops.host_to_card(adapter,
+                                                  priv->usb_port,
+                                                  skb, tx_param);
+       } else {
+               ret = adapter->if_ops.host_to_card(adapter,
+                                                  MWIFIEX_TYPE_DATA,
+                                                  skb, tx_param);
        }
        mwifiex_dbg_dump(adapter, DAT_D, "tx pkt:", skb->data,
                         min_t(size_t, skb->len, DEBUG_DUMP_DATA_MAX_LEN));
-
+out:
        switch (ret) {
        case -ENOSR:
                mwifiex_dbg(adapter, DATA, "data: -ENOSR is returned\n");
@@ -137,6 +140,11 @@ int mwifiex_process_tx(struct mwifiex_private *priv, struct sk_buff *skb,
                break;
        case -EINPROGRESS:
                break;
+       case -EINVAL:
+               mwifiex_dbg(adapter, ERROR,
+                           "malformed skb (length: %u, headroom: %u)\n",
+                           skb->len, skb_headroom(skb));
+               fallthrough;
        case 0:
                mwifiex_write_data_complete(adapter, skb, 0, ret);
                break;
index 360d36c..caff442 100644 (file)
@@ -461,8 +461,8 @@ int mwifiex_process_uap_rx_packet(struct mwifiex_private *priv,
  *      - Priority specific Tx control
  *      - Flags
  */
-void *mwifiex_process_uap_txpd(struct mwifiex_private *priv,
-                              struct sk_buff *skb)
+void mwifiex_process_uap_txpd(struct mwifiex_private *priv,
+                             struct sk_buff *skb)
 {
        struct mwifiex_adapter *adapter = priv->adapter;
        struct uap_txpd *txpd;
@@ -471,14 +471,6 @@ void *mwifiex_process_uap_txpd(struct mwifiex_private *priv,
        u16 pkt_type, pkt_offset;
        int hroom = adapter->intf_hdr_len;
 
-       if (!skb->len) {
-               mwifiex_dbg(adapter, ERROR,
-                           "Tx: bad packet length: %d\n", skb->len);
-               return skb->data;
-       }
-
-       BUG_ON(skb_headroom(skb) < MWIFIEX_MIN_DATA_HEADER_LEN);
-
        pkt_type = mwifiex_is_skb_mgmt_frame(skb) ? PKT_TYPE_MGMT : 0;
 
        pad = ((uintptr_t)skb->data - (sizeof(*txpd) + hroom)) &
@@ -526,6 +518,4 @@ void *mwifiex_process_uap_txpd(struct mwifiex_private *priv,
        if (!txpd->tx_control)
                /* TxCtrl set by user or default */
                txpd->tx_control = cpu_to_le32(priv->pkt_tx_ctrl);
-
-       return skb->data;
 }