From: Zong-Zhe Yang kevin_yang@realtek.com
[ Upstream commit bc2a5a12fa6259e190c7edb03e63b28ab480101b ]
Logically before a waiting side which has already timed out turns the atomic status back to idle, a completing side could still pass atomic condition and call complete. It will make the following H2C commands, waiting C2H events, get a completion unexpectedly early. Hence, renew a completion for each H2C command waiting a C2H event.
Signed-off-by: Zong-Zhe Yang kevin_yang@realtek.com Signed-off-by: Ping-Ke Shih pkshih@realtek.com Link: https://patch.msgid.link/20250915065343.39023-1-pkshih@realtek.com Signed-off-by: Sasha Levin sashal@kernel.org ---
LLM Generated explanations, may be completely bogus:
YES **Why Backport** - A timed-out wait leaves the shared completion object signaled, so the very next H2C command returns before firmware really responds and reuses stale `wait->data`, breaking consumers like `rtw89_fw_h2c_mcc_req_tsf` that immediately copy that buffer (`drivers/net/wireless/realtek/rtw89/fw.c:7484`). - The fix adds `struct rtw89_wait_response` and an RCU-protected pointer in `rtw89_wait_info`, giving each wait its own completion/data storage and preventing reuse of a fulfilled completion (`drivers/net/wireless/realtek/rtw89/core.h:4014`). - `rtw89_wait_for_cond` now allocates, initializes, and frees that response around every wait, so late C2H replies can only complete the instance that created them and never affect a later command (`drivers/net/wireless/realtek/rtw89/core.c:4463`). - The completion path dereferences the current response under RCU before signalling, guaranteeing that firmware acks only wake the matching waiter and that late packets after a timeout are safely ignored (`drivers/net/wireless/realtek/rtw89/core.c:4503`, `drivers/net/wireless/realtek/rtw89/mac.c:4493`). - Adding `lockdep_assert_wiphy` documents the required serialization, and the new `kzalloc`/`kfree_rcu` pair is tiny and self-contained, making regression risk low compared to the hard failures this race causes in power-save, WoW, and MCC command flows (`drivers/net/wireless/realtek/rtw89/fw.c:7304`).
Next steps: 1) Consider also pulling `a27136f1050a6` (“open C2H event waiting window first...”) which complements this area but fixes a different race.
drivers/net/wireless/realtek/rtw89/core.c | 49 ++++++++++++++++++++--- drivers/net/wireless/realtek/rtw89/core.h | 10 ++++- drivers/net/wireless/realtek/rtw89/fw.c | 2 + 3 files changed, 53 insertions(+), 8 deletions(-)
diff --git a/drivers/net/wireless/realtek/rtw89/core.c b/drivers/net/wireless/realtek/rtw89/core.c index 2cebea10cb99b..9896c4ab7146b 100644 --- a/drivers/net/wireless/realtek/rtw89/core.c +++ b/drivers/net/wireless/realtek/rtw89/core.c @@ -4860,37 +4860,74 @@ void rtw89_core_csa_beacon_work(struct wiphy *wiphy, struct wiphy_work *work)
int rtw89_wait_for_cond(struct rtw89_wait_info *wait, unsigned int cond) { - struct completion *cmpl = &wait->completion; + struct rtw89_wait_response *prep; unsigned long time_left; unsigned int cur; + int err = 0;
cur = atomic_cmpxchg(&wait->cond, RTW89_WAIT_COND_IDLE, cond); if (cur != RTW89_WAIT_COND_IDLE) return -EBUSY;
- time_left = wait_for_completion_timeout(cmpl, RTW89_WAIT_FOR_COND_TIMEOUT); + prep = kzalloc(sizeof(*prep), GFP_KERNEL); + if (!prep) { + err = -ENOMEM; + goto reset; + } + + init_completion(&prep->completion); + + rcu_assign_pointer(wait->resp, prep); + + time_left = wait_for_completion_timeout(&prep->completion, + RTW89_WAIT_FOR_COND_TIMEOUT); if (time_left == 0) { - atomic_set(&wait->cond, RTW89_WAIT_COND_IDLE); - return -ETIMEDOUT; + err = -ETIMEDOUT; + goto cleanup; }
+ wait->data = prep->data; + +cleanup: + rcu_assign_pointer(wait->resp, NULL); + kfree_rcu(prep, rcu_head); + +reset: + atomic_set(&wait->cond, RTW89_WAIT_COND_IDLE); + + if (err) + return err; + if (wait->data.err) return -EFAULT;
return 0; }
+static void rtw89_complete_cond_resp(struct rtw89_wait_response *resp, + const struct rtw89_completion_data *data) +{ + resp->data = *data; + complete(&resp->completion); +} + void rtw89_complete_cond(struct rtw89_wait_info *wait, unsigned int cond, const struct rtw89_completion_data *data) { + struct rtw89_wait_response *resp; unsigned int cur;
+ guard(rcu)(); + + resp = rcu_dereference(wait->resp); + if (!resp) + return; + cur = atomic_cmpxchg(&wait->cond, cond, RTW89_WAIT_COND_IDLE); if (cur != cond) return;
- wait->data = *data; - complete(&wait->completion); + rtw89_complete_cond_resp(resp, data); }
void rtw89_core_ntfy_btc_event(struct rtw89_dev *rtwdev, enum rtw89_btc_hmsg event) diff --git a/drivers/net/wireless/realtek/rtw89/core.h b/drivers/net/wireless/realtek/rtw89/core.h index 2de9505c48ffc..460453e63f844 100644 --- a/drivers/net/wireless/realtek/rtw89/core.h +++ b/drivers/net/wireless/realtek/rtw89/core.h @@ -4545,17 +4545,23 @@ struct rtw89_completion_data { u8 buf[RTW89_COMPLETION_BUF_SIZE]; };
+struct rtw89_wait_response { + struct rcu_head rcu_head; + struct completion completion; + struct rtw89_completion_data data; +}; + struct rtw89_wait_info { atomic_t cond; - struct completion completion; struct rtw89_completion_data data; + struct rtw89_wait_response __rcu *resp; };
#define RTW89_WAIT_FOR_COND_TIMEOUT msecs_to_jiffies(100)
static inline void rtw89_init_wait(struct rtw89_wait_info *wait) { - init_completion(&wait->completion); + rcu_assign_pointer(wait->resp, NULL); atomic_set(&wait->cond, RTW89_WAIT_COND_IDLE); }
diff --git a/drivers/net/wireless/realtek/rtw89/fw.c b/drivers/net/wireless/realtek/rtw89/fw.c index e6f8fab799fc1..7a5d616f7a9b8 100644 --- a/drivers/net/wireless/realtek/rtw89/fw.c +++ b/drivers/net/wireless/realtek/rtw89/fw.c @@ -8679,6 +8679,8 @@ static int rtw89_h2c_tx_and_wait(struct rtw89_dev *rtwdev, struct sk_buff *skb, { int ret;
+ lockdep_assert_wiphy(rtwdev->hw->wiphy); + ret = rtw89_h2c_tx(rtwdev, skb, false); if (ret) { rtw89_err(rtwdev, "failed to send h2c\n");