Patch "wifi: ath9k: fix races between ath9k_wmi_cmd and ath9k_wmi_ctrl_rx" has been added to the 5.4-stable tree

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



This is a note to let you know that I've just added the patch titled

    wifi: ath9k: fix races between ath9k_wmi_cmd and ath9k_wmi_ctrl_rx

to the 5.4-stable tree which can be found at:
    http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary

The filename of the patch is:
     wifi-ath9k-fix-races-between-ath9k_wmi_cmd-and-ath9k.patch
and it can be found in the queue-5.4 subdirectory.

If you, or anyone else, feels it should not be added to the stable tree,
please let <stable@xxxxxxxxxxxxxxx> know about it.



commit 008080fd9467d071c00c383b723e98cb3b89825b
Author: Fedor Pchelkin <pchelkin@xxxxxxxxx>
Date:   Tue Apr 25 22:26:06 2023 +0300

    wifi: ath9k: fix races between ath9k_wmi_cmd and ath9k_wmi_ctrl_rx
    
    [ Upstream commit b674fb513e2e7a514fcde287c0f73915d393fdb6 ]
    
    Currently, the synchronization between ath9k_wmi_cmd() and
    ath9k_wmi_ctrl_rx() is exposed to a race condition which, although being
    rather unlikely, can lead to invalid behaviour of ath9k_wmi_cmd().
    
    Consider the following scenario:
    
    CPU0                                    CPU1
    
    ath9k_wmi_cmd(...)
      mutex_lock(&wmi->op_mutex)
      ath9k_wmi_cmd_issue(...)
      wait_for_completion_timeout(...)
      ---
      timeout
      ---
                                            /* the callback is being processed
                                             * before last_seq_id became zero
                                             */
                                            ath9k_wmi_ctrl_rx(...)
                                              spin_lock_irqsave(...)
                                              /* wmi->last_seq_id check here
                                               * doesn't detect timeout yet
                                               */
                                              spin_unlock_irqrestore(...)
      /* last_seq_id is zeroed to
       * indicate there was a timeout
       */
      wmi->last_seq_id = 0
      mutex_unlock(&wmi->op_mutex)
      return -ETIMEDOUT
    
    ath9k_wmi_cmd(...)
      mutex_lock(&wmi->op_mutex)
      /* the buffer is replaced with
       * another one
       */
      wmi->cmd_rsp_buf = rsp_buf
      wmi->cmd_rsp_len = rsp_len
      ath9k_wmi_cmd_issue(...)
        spin_lock_irqsave(...)
        spin_unlock_irqrestore(...)
      wait_for_completion_timeout(...)
                                            /* the continuation of the
                                             * callback left after the first
                                             * ath9k_wmi_cmd call
                                             */
                                              ath9k_wmi_rsp_callback(...)
                                                /* copying data designated
                                                 * to already timeouted
                                                 * WMI command into an
                                                 * inappropriate wmi_cmd_buf
                                                 */
                                                memcpy(...)
                                                complete(&wmi->cmd_wait)
      /* awakened by the bogus callback
       * => invalid return result
       */
      mutex_unlock(&wmi->op_mutex)
      return 0
    
    To fix this, update last_seq_id on timeout path inside ath9k_wmi_cmd()
    under the wmi_lock. Move ath9k_wmi_rsp_callback() under wmi_lock inside
    ath9k_wmi_ctrl_rx() so that the wmi->cmd_wait can be completed only for
    initially designated wmi_cmd call, otherwise the path would be rejected
    with last_seq_id check.
    
    Found by Linux Verification Center (linuxtesting.org) with Syzkaller.
    
    Fixes: fb9987d0f748 ("ath9k_htc: Support for AR9271 chipset.")
    Signed-off-by: Fedor Pchelkin <pchelkin@xxxxxxxxx>
    Acked-by: Toke Høiland-Jørgensen <toke@xxxxxxx>
    Signed-off-by: Kalle Valo <quic_kvalo@xxxxxxxxxxx>
    Link: https://lore.kernel.org/r/20230425192607.18015-1-pchelkin@xxxxxxxxx
    Signed-off-by: Sasha Levin <sashal@xxxxxxxxxx>

diff --git a/drivers/net/wireless/ath/ath9k/wmi.c b/drivers/net/wireless/ath/ath9k/wmi.c
index ef861b19fd477..96482ad23145c 100644
--- a/drivers/net/wireless/ath/ath9k/wmi.c
+++ b/drivers/net/wireless/ath/ath9k/wmi.c
@@ -239,10 +239,10 @@ static void ath9k_wmi_ctrl_rx(void *priv, struct sk_buff *skb,
 		spin_unlock_irqrestore(&wmi->wmi_lock, flags);
 		goto free_skb;
 	}
-	spin_unlock_irqrestore(&wmi->wmi_lock, flags);
 
 	/* WMI command response */
 	ath9k_wmi_rsp_callback(wmi, skb);
+	spin_unlock_irqrestore(&wmi->wmi_lock, flags);
 
 free_skb:
 	kfree_skb(skb);
@@ -305,8 +305,8 @@ int ath9k_wmi_cmd(struct wmi *wmi, enum wmi_cmd_id cmd_id,
 	struct ath_common *common = ath9k_hw_common(ah);
 	u16 headroom = sizeof(struct htc_frame_hdr) +
 		       sizeof(struct wmi_cmd_hdr);
+	unsigned long time_left, flags;
 	struct sk_buff *skb;
-	unsigned long time_left;
 	int ret = 0;
 
 	if (ah->ah_flags & AH_UNPLUGGED)
@@ -342,7 +342,9 @@ int ath9k_wmi_cmd(struct wmi *wmi, enum wmi_cmd_id cmd_id,
 	if (!time_left) {
 		ath_dbg(common, WMI, "Timeout waiting for WMI command: %s\n",
 			wmi_cmd_to_name(cmd_id));
+		spin_lock_irqsave(&wmi->wmi_lock, flags);
 		wmi->last_seq_id = 0;
+		spin_unlock_irqrestore(&wmi->wmi_lock, flags);
 		mutex_unlock(&wmi->op_mutex);
 		return -ETIMEDOUT;
 	}



[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux