Skip to content

Commit 3e31a6b

Browse files
pchelkin91Ping-Ke Shih
authored andcommitted
wifi: rtw89: fix use-after-free in rtw89_core_tx_kick_off_and_wait()
There is a bug observed when rtw89_core_tx_kick_off_and_wait() tries to access already freed skb_data: BUG: KFENCE: use-after-free write in rtw89_core_tx_kick_off_and_wait drivers/net/wireless/realtek/rtw89/core.c:1110 CPU: 6 UID: 0 PID: 41377 Comm: kworker/u64:24 Not tainted 6.17.0-rc1+ #1 PREEMPT(lazy) Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS edk2-20250523-14.fc42 05/23/2025 Workqueue: events_unbound cfg80211_wiphy_work [cfg80211] Use-after-free write at 0x0000000020309d9d (in kfence-torvalds#251): rtw89_core_tx_kick_off_and_wait drivers/net/wireless/realtek/rtw89/core.c:1110 rtw89_core_scan_complete drivers/net/wireless/realtek/rtw89/core.c:5338 rtw89_hw_scan_complete_cb drivers/net/wireless/realtek/rtw89/fw.c:7979 rtw89_chanctx_proceed_cb drivers/net/wireless/realtek/rtw89/chan.c:3165 rtw89_chanctx_proceed drivers/net/wireless/realtek/rtw89/chan.h:141 rtw89_hw_scan_complete drivers/net/wireless/realtek/rtw89/fw.c:8012 rtw89_mac_c2h_scanofld_rsp drivers/net/wireless/realtek/rtw89/mac.c:5059 rtw89_fw_c2h_work drivers/net/wireless/realtek/rtw89/fw.c:6758 process_one_work kernel/workqueue.c:3241 worker_thread kernel/workqueue.c:3400 kthread kernel/kthread.c:463 ret_from_fork arch/x86/kernel/process.c:154 ret_from_fork_asm arch/x86/entry/entry_64.S:258 kfence-torvalds#251: 0x0000000056e2393d-0x000000009943cb62, size=232, cache=skbuff_head_cache allocated by task 41377 on cpu 6 at 77869.159548s (0.009551s ago): __alloc_skb net/core/skbuff.c:659 __netdev_alloc_skb net/core/skbuff.c:734 ieee80211_nullfunc_get net/mac80211/tx.c:5844 rtw89_core_send_nullfunc drivers/net/wireless/realtek/rtw89/core.c:3431 rtw89_core_scan_complete drivers/net/wireless/realtek/rtw89/core.c:5338 rtw89_hw_scan_complete_cb drivers/net/wireless/realtek/rtw89/fw.c:7979 rtw89_chanctx_proceed_cb drivers/net/wireless/realtek/rtw89/chan.c:3165 rtw89_chanctx_proceed drivers/net/wireless/realtek/rtw89/chan.c:3194 rtw89_hw_scan_complete drivers/net/wireless/realtek/rtw89/fw.c:8012 rtw89_mac_c2h_scanofld_rsp drivers/net/wireless/realtek/rtw89/mac.c:5059 rtw89_fw_c2h_work drivers/net/wireless/realtek/rtw89/fw.c:6758 process_one_work kernel/workqueue.c:3241 worker_thread kernel/workqueue.c:3400 kthread kernel/kthread.c:463 ret_from_fork arch/x86/kernel/process.c:154 ret_from_fork_asm arch/x86/entry/entry_64.S:258 freed by task 1045 on cpu 9 at 77869.168393s (0.001557s ago): ieee80211_tx_status_skb net/mac80211/status.c:1117 rtw89_pci_release_txwd_skb drivers/net/wireless/realtek/rtw89/pci.c:564 rtw89_pci_release_tx_skbs.isra.0 drivers/net/wireless/realtek/rtw89/pci.c:651 rtw89_pci_release_tx drivers/net/wireless/realtek/rtw89/pci.c:676 rtw89_pci_napi_poll drivers/net/wireless/realtek/rtw89/pci.c:4238 __napi_poll net/core/dev.c:7495 net_rx_action net/core/dev.c:7557 net/core/dev.c:7684 handle_softirqs kernel/softirq.c:580 do_softirq.part.0 kernel/softirq.c:480 __local_bh_enable_ip kernel/softirq.c:407 rtw89_pci_interrupt_threadfn drivers/net/wireless/realtek/rtw89/pci.c:927 irq_thread_fn kernel/irq/manage.c:1133 irq_thread kernel/irq/manage.c:1257 kthread kernel/kthread.c:463 ret_from_fork arch/x86/kernel/process.c:154 ret_from_fork_asm arch/x86/entry/entry_64.S:258 It is a consequence of a race between the waiting and the signaling side of the completion: Waiting thread Completing thread rtw89_core_tx_kick_off_and_wait() rcu_assign_pointer(skb_data->wait, wait) /* start waiting */ wait_for_completion_timeout() rtw89_pci_tx_status() rtw89_core_tx_wait_complete() rcu_read_lock() /* signals completion and * proceeds further */ complete(&wait->completion) rcu_read_unlock() ... /* frees skb_data */ ieee80211_tx_status_ni() /* returns (exit status doesn't matter) */ wait_for_completion_timeout() ... /* accesses the already freed skb_data */ rcu_assign_pointer(skb_data->wait, NULL) The completing side might proceed and free the underlying skb even before the waiting side is fully awoken and run to execution. Actually the race happens regardless of wait_for_completion_timeout() exit status, e.g. the waiting side may hit a timeout and the concurrent completing side is still able to free the skb. Skbs which are sent by rtw89_core_tx_kick_off_and_wait() are owned by the driver. They don't come from core ieee80211 stack so no need to pass them to ieee80211_tx_status_ni() on completing side. Introduce a work function which will act as a garbage collector for rtw89_tx_wait_info objects and the associated skbs. Thus no potentially heavy locks are required on the completing side. Found by Linux Verification Center (linuxtesting.org). Fixes: 1ae5ca6 ("wifi: rtw89: add function to wait for completion of TX skbs") Cc: stable@vger.kernel.org Suggested-by: Zong-Zhe Yang <kevin_yang@realtek.com> Signed-off-by: Fedor Pchelkin <pchelkin@ispras.ru> Acked-by: Ping-Ke Shih <pkshih@realtek.com> Signed-off-by: Ping-Ke Shih <pkshih@realtek.com> Link: https://patch.msgid.link/20250919210852.823912-2-pchelkin@ispras.ru
1 parent 533e60e commit 3e31a6b

File tree

4 files changed

+61
-9
lines changed

4 files changed

+61
-9
lines changed

drivers/net/wireless/realtek/rtw89/core.c

Lines changed: 24 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1135,6 +1135,14 @@ rtw89_core_tx_update_desc_info(struct rtw89_dev *rtwdev,
11351135
}
11361136
}
11371137

1138+
static void rtw89_tx_wait_work(struct wiphy *wiphy, struct wiphy_work *work)
1139+
{
1140+
struct rtw89_dev *rtwdev = container_of(work, struct rtw89_dev,
1141+
tx_wait_work.work);
1142+
1143+
rtw89_tx_wait_list_clear(rtwdev);
1144+
}
1145+
11381146
void rtw89_core_tx_kick_off(struct rtw89_dev *rtwdev, u8 qsel)
11391147
{
11401148
u8 ch_dma;
@@ -1152,25 +1160,32 @@ int rtw89_core_tx_kick_off_and_wait(struct rtw89_dev *rtwdev, struct sk_buff *sk
11521160
unsigned long time_left;
11531161
int ret = 0;
11541162

1163+
lockdep_assert_wiphy(rtwdev->hw->wiphy);
1164+
11551165
wait = kzalloc(sizeof(*wait), GFP_KERNEL);
11561166
if (!wait) {
11571167
rtw89_core_tx_kick_off(rtwdev, qsel);
11581168
return 0;
11591169
}
11601170

11611171
init_completion(&wait->completion);
1172+
wait->skb = skb;
11621173
rcu_assign_pointer(skb_data->wait, wait);
11631174

11641175
rtw89_core_tx_kick_off(rtwdev, qsel);
11651176
time_left = wait_for_completion_timeout(&wait->completion,
11661177
msecs_to_jiffies(timeout));
1167-
if (time_left == 0)
1168-
ret = -ETIMEDOUT;
1169-
else if (!wait->tx_done)
1170-
ret = -EAGAIN;
11711178

1172-
rcu_assign_pointer(skb_data->wait, NULL);
1173-
kfree_rcu(wait, rcu_head);
1179+
if (time_left == 0) {
1180+
ret = -ETIMEDOUT;
1181+
list_add_tail(&wait->list, &rtwdev->tx_waits);
1182+
wiphy_delayed_work_queue(rtwdev->hw->wiphy, &rtwdev->tx_wait_work,
1183+
RTW89_TX_WAIT_WORK_TIMEOUT);
1184+
} else {
1185+
if (!wait->tx_done)
1186+
ret = -EAGAIN;
1187+
rtw89_tx_wait_release(wait);
1188+
}
11741189

11751190
return ret;
11761191
}
@@ -5548,6 +5563,7 @@ void rtw89_core_stop(struct rtw89_dev *rtwdev)
55485563
wiphy_work_cancel(wiphy, &btc->dhcp_notify_work);
55495564
wiphy_work_cancel(wiphy, &btc->icmp_notify_work);
55505565
cancel_delayed_work_sync(&rtwdev->txq_reinvoke_work);
5566+
wiphy_delayed_work_cancel(wiphy, &rtwdev->tx_wait_work);
55515567
wiphy_delayed_work_cancel(wiphy, &rtwdev->track_work);
55525568
wiphy_delayed_work_cancel(wiphy, &rtwdev->track_ps_work);
55535569
wiphy_delayed_work_cancel(wiphy, &rtwdev->chanctx_work);
@@ -5773,6 +5789,7 @@ int rtw89_core_init(struct rtw89_dev *rtwdev)
57735789
INIT_LIST_HEAD(&rtwdev->scan_info.pkt_list[band]);
57745790
}
57755791
INIT_LIST_HEAD(&rtwdev->scan_info.chan_list);
5792+
INIT_LIST_HEAD(&rtwdev->tx_waits);
57765793
INIT_WORK(&rtwdev->ba_work, rtw89_core_ba_work);
57775794
INIT_WORK(&rtwdev->txq_work, rtw89_core_txq_work);
57785795
INIT_DELAYED_WORK(&rtwdev->txq_reinvoke_work, rtw89_core_txq_reinvoke_work);
@@ -5784,6 +5801,7 @@ int rtw89_core_init(struct rtw89_dev *rtwdev)
57845801
wiphy_delayed_work_init(&rtwdev->coex_rfk_chk_work, rtw89_coex_rfk_chk_work);
57855802
wiphy_delayed_work_init(&rtwdev->cfo_track_work, rtw89_phy_cfo_track_work);
57865803
wiphy_delayed_work_init(&rtwdev->mcc_prepare_done_work, rtw89_mcc_prepare_done_work);
5804+
wiphy_delayed_work_init(&rtwdev->tx_wait_work, rtw89_tx_wait_work);
57875805
INIT_DELAYED_WORK(&rtwdev->forbid_ba_work, rtw89_forbid_ba_work);
57885806
wiphy_delayed_work_init(&rtwdev->antdiv_work, rtw89_phy_antdiv_work);
57895807
rtwdev->txq_wq = alloc_workqueue("rtw89_tx_wq", WQ_UNBOUND | WQ_HIGHPRI, 0);

drivers/net/wireless/realtek/rtw89/core.h

Lines changed: 33 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3507,9 +3507,12 @@ struct rtw89_phy_rate_pattern {
35073507
bool enable;
35083508
};
35093509

3510+
#define RTW89_TX_WAIT_WORK_TIMEOUT msecs_to_jiffies(500)
35103511
struct rtw89_tx_wait_info {
35113512
struct rcu_head rcu_head;
3513+
struct list_head list;
35123514
struct completion completion;
3515+
struct sk_buff *skb;
35133516
bool tx_done;
35143517
};
35153518

@@ -6000,6 +6003,9 @@ struct rtw89_dev {
60006003
/* used to protect rpwm */
60016004
spinlock_t rpwm_lock;
60026005

6006+
struct list_head tx_waits;
6007+
struct wiphy_delayed_work tx_wait_work;
6008+
60036009
struct rtw89_cam_info cam_info;
60046010

60056011
struct sk_buff_head c2h_queue;
@@ -6256,6 +6262,26 @@ rtw89_assoc_link_rcu_dereference(struct rtw89_dev *rtwdev, u8 macid)
62566262
list_first_entry_or_null(&p->dlink_pool, typeof(*p->links_inst), dlink_schd); \
62576263
})
62586264

6265+
static inline void rtw89_tx_wait_release(struct rtw89_tx_wait_info *wait)
6266+
{
6267+
dev_kfree_skb_any(wait->skb);
6268+
kfree_rcu(wait, rcu_head);
6269+
}
6270+
6271+
static inline void rtw89_tx_wait_list_clear(struct rtw89_dev *rtwdev)
6272+
{
6273+
struct rtw89_tx_wait_info *wait, *tmp;
6274+
6275+
lockdep_assert_wiphy(rtwdev->hw->wiphy);
6276+
6277+
list_for_each_entry_safe(wait, tmp, &rtwdev->tx_waits, list) {
6278+
if (!completion_done(&wait->completion))
6279+
continue;
6280+
list_del(&wait->list);
6281+
rtw89_tx_wait_release(wait);
6282+
}
6283+
}
6284+
62596285
static inline int rtw89_hci_tx_write(struct rtw89_dev *rtwdev,
62606286
struct rtw89_core_tx_request *tx_req)
62616287
{
@@ -6265,6 +6291,7 @@ static inline int rtw89_hci_tx_write(struct rtw89_dev *rtwdev,
62656291
static inline void rtw89_hci_reset(struct rtw89_dev *rtwdev)
62666292
{
62676293
rtwdev->hci.ops->reset(rtwdev);
6294+
rtw89_tx_wait_list_clear(rtwdev);
62686295
}
62696296

62706297
static inline int rtw89_hci_start(struct rtw89_dev *rtwdev)
@@ -7345,23 +7372,27 @@ static inline struct sk_buff *rtw89_alloc_skb_for_rx(struct rtw89_dev *rtwdev,
73457372
return dev_alloc_skb(length);
73467373
}
73477374

7348-
static inline void rtw89_core_tx_wait_complete(struct rtw89_dev *rtwdev,
7375+
static inline bool rtw89_core_tx_wait_complete(struct rtw89_dev *rtwdev,
73497376
struct rtw89_tx_skb_data *skb_data,
73507377
bool tx_done)
73517378
{
73527379
struct rtw89_tx_wait_info *wait;
7380+
bool ret = false;
73537381

73547382
rcu_read_lock();
73557383

73567384
wait = rcu_dereference(skb_data->wait);
73577385
if (!wait)
73587386
goto out;
73597387

7388+
ret = true;
73607389
wait->tx_done = tx_done;
7361-
complete(&wait->completion);
7390+
/* Don't access skb anymore after completion */
7391+
complete_all(&wait->completion);
73627392

73637393
out:
73647394
rcu_read_unlock();
7395+
return ret;
73657396
}
73667397

73677398
static inline bool rtw89_is_mlo_1_1(struct rtw89_dev *rtwdev)

drivers/net/wireless/realtek/rtw89/pci.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -464,7 +464,8 @@ static void rtw89_pci_tx_status(struct rtw89_dev *rtwdev,
464464
struct rtw89_tx_skb_data *skb_data = RTW89_TX_SKB_CB(skb);
465465
struct ieee80211_tx_info *info;
466466

467-
rtw89_core_tx_wait_complete(rtwdev, skb_data, tx_status == RTW89_TX_DONE);
467+
if (rtw89_core_tx_wait_complete(rtwdev, skb_data, tx_status == RTW89_TX_DONE))
468+
return;
468469

469470
info = IEEE80211_SKB_CB(skb);
470471
ieee80211_tx_info_clear_status(info);

drivers/net/wireless/realtek/rtw89/ser.c

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -502,7 +502,9 @@ static void ser_reset_trx_st_hdl(struct rtw89_ser *ser, u8 evt)
502502
}
503503

504504
drv_stop_rx(ser);
505+
wiphy_lock(wiphy);
505506
drv_trx_reset(ser);
507+
wiphy_unlock(wiphy);
506508

507509
/* wait m3 */
508510
hal_send_m2_event(ser);

0 commit comments

Comments
 (0)