wifi: ath9k: delay all of ath9k_wmi_event_tasklet() until init is complete
authorToke Høiland-Jørgensen <toke@redhat.com>
Fri, 26 Jan 2024 14:02:17 +0000 (15:02 +0100)
committerKalle Valo <quic_kvalo@quicinc.com>
Fri, 2 Feb 2024 11:39:58 +0000 (13:39 +0200)
The ath9k_wmi_event_tasklet() used in ath9k_htc assumes that all the data
structures have been fully initialised by the time it runs. However, because of
the order in which things are initialised, this is not guaranteed to be the
case, because the device is exposed to the USB subsystem before the ath9k driver
initialisation is completed.

We already committed a partial fix for this in commit:
8b3046abc99e ("ath9k_htc: fix NULL pointer dereference at ath9k_htc_tx_get_packet()")

However, that commit only aborted the WMI_TXSTATUS_EVENTID command in the event
tasklet, pairing it with an "initialisation complete" bit in the TX struct. It
seems syzbot managed to trigger the race for one of the other commands as well,
so let's just move the existing synchronisation bit to cover the whole
tasklet (setting it at the end of ath9k_htc_probe_device() instead of inside
ath9k_tx_init()).

Link: https://lore.kernel.org/r/ed1d2c66-1193-4c81-9542-d514c29ba8b8.bugreport@ubisectech.com
Fixes: 8b3046abc99e ("ath9k_htc: fix NULL pointer dereference at ath9k_htc_tx_get_packet()")
Reported-by: Ubisectech Sirius <bugreport@ubisectech.com>
Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com>
Signed-off-by: Kalle Valo <quic_kvalo@quicinc.com>
Link: https://msgid.link/20240126140218.1033443-1-toke@toke.dk
drivers/net/wireless/ath/ath9k/htc.h
drivers/net/wireless/ath/ath9k/htc_drv_init.c
drivers/net/wireless/ath/ath9k/htc_drv_txrx.c
drivers/net/wireless/ath/ath9k/wmi.c

index 237f4ec2cffd7b0c53f8d1b253fe81072cf1ceab..6c33e898b30006820f3b7993715a3eac0d9486a0 100644 (file)
@@ -306,7 +306,6 @@ struct ath9k_htc_tx {
        DECLARE_BITMAP(tx_slot, MAX_TX_BUF_NUM);
        struct timer_list cleanup_timer;
        spinlock_t tx_lock;
-       bool initialized;
 };
 
 struct ath9k_htc_tx_ctl {
@@ -515,6 +514,7 @@ struct ath9k_htc_priv {
        unsigned long ps_usecount;
        bool ps_enabled;
        bool ps_idle;
+       bool initialized;
 
 #ifdef CONFIG_MAC80211_LEDS
        enum led_brightness brightness;
index 0aa5bdeb44a1b0bf605ddb1d67e46dd7e9f4302d..3633f9eb2c559a70d1c9229a0a6befc5fbce43a0 100644 (file)
@@ -966,6 +966,10 @@ int ath9k_htc_probe_device(struct htc_target *htc_handle, struct device *dev,
 
        htc_handle->drv_priv = priv;
 
+       /* Allow ath9k_wmi_event_tasklet() to operate. */
+       smp_wmb();
+       priv->initialized = true;
+
        return 0;
 
 err_init:
index efcaeccb055aa521414e99b630dd485159555efb..ce9c04e418b8d7d8cfc23e9e5d40968022d73daa 100644 (file)
@@ -815,10 +815,6 @@ int ath9k_tx_init(struct ath9k_htc_priv *priv)
        skb_queue_head_init(&priv->tx.data_vo_queue);
        skb_queue_head_init(&priv->tx.tx_failed);
 
-       /* Allow ath9k_wmi_event_tasklet(WMI_TXSTATUS_EVENTID) to operate. */
-       smp_wmb();
-       priv->tx.initialized = true;
-
        return 0;
 }
 
index 1476b42b52a9156f40aac8cf262f71a2aa6c4beb..805ad31edba2ba3aeaad619467ab93733bd84e4e 100644 (file)
@@ -155,6 +155,12 @@ void ath9k_wmi_event_tasklet(struct tasklet_struct *t)
                }
                spin_unlock_irqrestore(&wmi->wmi_lock, flags);
 
+               /* Check if ath9k_htc_probe_device() completed. */
+               if (!data_race(priv->initialized)) {
+                       kfree_skb(skb);
+                       continue;
+               }
+
                hdr = (struct wmi_cmd_hdr *) skb->data;
                cmd_id = be16_to_cpu(hdr->command_id);
                wmi_event = skb_pull(skb, sizeof(struct wmi_cmd_hdr));
@@ -169,10 +175,6 @@ void ath9k_wmi_event_tasklet(struct tasklet_struct *t)
                                             &wmi->drv_priv->fatal_work);
                        break;
                case WMI_TXSTATUS_EVENTID:
-                       /* Check if ath9k_tx_init() completed. */
-                       if (!data_race(priv->tx.initialized))
-                               break;
-
                        spin_lock_bh(&priv->tx.tx_lock);
                        if (priv->tx.flags & ATH9K_HTC_OP_TX_DRAIN) {
                                spin_unlock_bh(&priv->tx.tx_lock);