[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <169271132742.680890.9212359181887346936.kvalo@kernel.org>
Date: Tue, 22 Aug 2023 13:35:29 +0000 (UTC)
From: Kalle Valo <kvalo@...nel.org>
To: Fedor Pchelkin <pchelkin@...ras.ru>
Cc: Toke Høiland-Jørgensen <toke@...e.dk>,
Fedor Pchelkin <pchelkin@...ras.ru>,
"David S. Miller" <davem@...emloft.net>,
Eric Dumazet <edumazet@...gle.com>,
Jakub Kicinski <kuba@...nel.org>,
Paolo Abeni <pabeni@...hat.com>,
Senthil Balasubramanian <senthilkumar@...eros.com>,
"John W. Linville" <linville@...driver.com>,
Vasanthakumar Thiagarajan <vasanth@...eros.com>,
Sujith <Sujith.Manoharan@...eros.com>,
linux-wireless@...r.kernel.org, netdev@...r.kernel.org,
linux-kernel@...r.kernel.org,
Alexey Khoroshilov <khoroshilov@...ras.ru>,
lvc-project@...uxtesting.org,
syzbot+f2cb6e0ffdb961921e4d@...kaller.appspotmail.com,
Hillf Danton <hdanton@...a.com>
Subject: Re: [PATCH v3 1/2] wifi: ath9k: fix races between ath9k_wmi_cmd and
ath9k_wmi_ctrl_rx
Fedor Pchelkin <pchelkin@...ras.ru> wrote:
> 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@...ras.ru>
> Acked-by: Toke Høiland-Jørgensen <toke@...e.dk>
> Signed-off-by: Kalle Valo <quic_kvalo@...cinc.com>
2 patches applied to ath-next branch of ath.git, thanks.
b674fb513e2e wifi: ath9k: fix races between ath9k_wmi_cmd and ath9k_wmi_ctrl_rx
454994cfa9e4 wifi: ath9k: protect WMI command response buffer replacement with a lock
--
https://patchwork.kernel.org/project/linux-wireless/patch/20230425192607.18015-1-pchelkin@ispras.ru/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
Powered by blists - more mailing lists