divide by zero in ath9k_htc_choose_bslot()
The attached demo uses usbip to pretend to be a USB device for
drivers/net/wireless/ath/ath9k/
It sets up the wifi interface, and then maliciously generates a frame that claims to be on the USB endpoint that ath9k_htc_rx_msg() passes to ath9k_wmi_ctrl_rx(). The cmd_id in the frame is 0x1002, or WMI_SWBA_EVENTID, which causes ath9k_wmi_ctrl_rx() to wake up the ath9k_wmi_event_tasklet. Which calls ath9k_htc_swba(), which calls ath9k_htc_choose_bslot(), which says
intval = priv->cur_beacon_conf.beacon_interval;
tsf = be64_to_cpu(swba->tsf);
tsftu = TSF_TO_TU(tsf >> 32, tsf);
slot = ((tsftu % intval) * ATH9K_HTC_MAX_BCN_VIF) / intval;
slot = ATH9K_HTC_MAX_BCN_VIF - slot - 1;At this point beacon_interval is zero, so this divides by zero. On an amd64, a fault. On a RISC-V, which produces -1 for divide by zero, slot ends up as 2, which is too large; later on, slot=2 causes ath9k_htc_send_buffered() to index off the end of the bslot array; ieee80211_get_buffered_bc() then dereferences the resulting bad vif pointer.
vif = priv->beacon.bslot[slot];
skb = ieee80211_get_buffered_bc(priv->hw, vif);Changing ath9k_htc_choose_bslot() to return zero if intval is zero makes the crash go away. I don't know if that would be correct with a real Atheros device, but it probably doesn't matter since I imagine this would only ever arise with a broken or malicious USB device.
# uname -a Linux xxx 6.14.0-rc7 #18 SMP PREEMPT_DYNAMIC Thu Mar 27 11:26:30 EDT 2025 x86_64 x86_64 x86_64 GNU/Linux # cc usbath1c.c # ./a.out ... Oops: divide error: 0000 [#1] PREEMPT SMP PTI CPU: 2 UID: 0 PID: 29 Comm: ksoftirqd/2 Not tainted 6.14.0-rc7 #18 Hardware name: FreeBSD BHYVE/BHYVE, BIOS 14.0 10/17/2021 RIP: 0010:ath9k_htc_swba+0x5f/0x3a0 Code: 0f 85 93 02 00 00 48 8b 06 0f b7 8d c0 03 00 00 4c 8d ad b0 03 00 00 4c 89 ef 48 0f c8 48 89 c2 48 c1 ea 20 0f ac d0 0a 31 d2 <f7> f1 8d 04 12 31 d2 f7 f1 89 c3 e8 31 23 56 00 b8 01 00 00 00 29 RSP: 0018:ffffbc49c014fd88 EFLAGS: 00010246 RAX: 0000000000000000 RBX: ffff9ef116c1ef40 RCX: 0000000000000000 RDX: 0000000000000000 RSI: ffff9ef102fc006c RDI: ffff9ef116f56390 RBP: ffff9ef116f55fe0 R08: 0000000000000800 R09: 0000000000000000 R10: ffffffff862070c0 R11: 0000000000000210 R12: ffff9ef104b0c0cc R13: ffff9ef116f56390 R14: ffff9ef104b0c060 R15: 0000000000000210 FS: 0000000000000000(0000) GS:ffff9ef42fa80000(0000) knlGS:0000000000000000 CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 CR2: 00007ffd54f0c1dc CR3: 00000001024f4001 CR4: 00000000003706f0 DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 Call Trace: <TASK> ? die+0x31/0x80 ? do_trap+0xd8/0x100 ? ath9k_htc_swba+0x5f/0x3a0 ? do_error_trap+0x60/0x80 ? ath9k_htc_swba+0x5f/0x3a0 ? exc_divide_error+0x39/0x50 ? ath9k_htc_swba+0x5f/0x3a0 ? asm_exc_divide_error+0x1a/0x20 ? ath9k_htc_swba+0x5f/0x3a0 ? sched_balance_newidle.isra.0+0x29f/0x3a0 ath9k_wmi_event_tasklet+0x33/0x150 tasklet_action_common+0xb3/0x220 handle_softirqs+0xc5/0x2b0 ? __pfx_smpboot_thread_fn+0x10/0x10 run_ksoftirqd+0x20/0x30 smpboot_thread_fn+0xd8/0x1d0 kthread+0xe9/0x1f0 ? __pfx_kthread+0x10/0x10 ret_from_fork+0x2f/0x50 ? __pfx_kthread+0x10/0x10 ret_from_fork_asm+0x1a/0x30 </TASK> Modules linked in: ---[ end trace 0000000000000000 ]--- RIP: 0010:ath9k_htc_swba+0x5f/0x3a0
Robert Morris
[email protected]
Toke Høiland-Jørgensen
maintainerQuoted 24 lines · [email protected] writes:
The attached demo uses usbip to pretend to be a USB device for
drivers/net/wireless/ath/ath9k/It sets up the wifi interface, and then maliciously generates a frame that claims to be on the USB endpoint that
ath9k_htc_rx_msg()passes toath9k_wmi_ctrl_rx(). Thecmd_idin the frame is0x1002, orWMI_SWBA_EVENTID, which causesath9k_wmi_ctrl_rx()to wake up theath9k_wmi_event_tasklet. Which callsath9k_htc_swba(), which callsath9k_htc_choose_bslot(), which saysintval = priv->cur_beacon_conf.beacon_interval; tsf = be64_to_cpu(swba->tsf); tsftu = TSF_TO_TU(tsf >> 32, tsf); slot = ((tsftu % intval) * ATH9K_HTC_MAX_BCN_VIF) / intval; slot = ATH9K_HTC_MAX_BCN_VIF - slot - 1;At this point
beacon_intervalis zero, so this divides by zero. On an amd64, a fault. On a RISC-V, which produces -1 for divide by zero, slot ends up as 2, which is too large; later on, slot=2 causesath9k_htc_send_buffered()to index off the end of the bslot array;ieee80211_get_buffered_bc()then dereferences the resulting bad vif pointer.
Quoted 7 lines
vif = priv->beacon.bslot[slot]; skb = ieee80211_get_buffered_bc(priv->hw, vif);Changing
ath9k_htc_choose_bslot()to return zero if intval is zero makes the crash go away. I don't know if that would be correct with a real Atheros device, but it probably doesn't matter since I imagine this would only ever arise with a broken or malicious USB device.
Thank you for the report, and the thorough analysis!
So I guess this happens because your malicious device sends the SWBA command before beacons have actually been enabled (and ath9k_htc_beacon_config() called to set the inverval)? In which case I think a patch like the below makes more sense than relying on ath9k_htc_choose_bslot() returning 0. WDYT?
-Toke
drivers/net/wireless/ath/ath9k/htc_drv_beacon.c +3 -0
@@ -290,6 +290,9 @@ void ath9k_htc_swba(struct ath9k_htc_priv *priv,struct ath_common *common =ath9k_hw_common (priv->ah);int slot; +if (!priv->cur_beacon_conf.enable_beacon) +return ; +if (swba->beacon_pending !=0 ) { priv->beacon.bmisscnt++;if (priv->beacon.bmisscnt >BSTUCK_THRESHOLD ) {
Toke,
Yes, your fix makes the problem go away for me, and does seem like the better approach.
Robert
Quoted 14 lines
drivers/net/wireless/ath/ath9k/htc_drv_beacon.c +3
-0@@ -290,6 +290,9 @@ void ath9k_htc_swba(struct ath9k_htc_priv *priv,struct ath_common *common =ath9k_hw_common (priv->ah);int slot; +if (!priv->cur_beacon_conf.enable_beacon) +return ; +if (swba->beacon_pending !=0 ) { priv->beacon.bmisscnt++;if (priv->beacon.bmisscnt >BSTUCK_THRESHOLD ) {
Quoted 15 lines · On 3/31/2025 7:17 AM, [email protected] wrote:
Toke,
Yes, your fix makes the problem go away for me, and does seem like the better approach.
Robert
Quoted 8 lines
drivers/net/wireless/ath/ath9k/htc_drv_beacon.c +0
-0@@ -290,6 +290,9 @@ void ath9k_htc_swba(struct ath9k_htc_priv *priv,struct ath_common *common =ath9k_hw_common (priv->ah);int slot;
Quoted 7 lines
drivers/net/wireless/ath/ath9k/htc_drv_beacon.c void ath9k_htc_swba(struct ath9k_htc_priv *priv, · line 298
Quoted 6 lines
drivers/net/wireless/ath/ath9k/htc_drv_beacon.c +3
-0+if (!priv->cur_beacon_conf.enable_beacon) +return ; +if (swba->beacon_pending !=0 ) { priv->beacon.bmisscnt++;if (priv->beacon.bmisscnt >BSTUCK_THRESHOLD ) {
Toke, care to submit a patch I can apply?
Thanks,
/jeff
Toke Høiland-Jørgensen
maintainerQuoted 24 lines · Jeff Johnson
Quoted 15 lines · On 3/31/2025 7:17 AM, [email protected] wrote:
Toke,
Yes, your fix makes the problem go away for me, and does seem like the better approach.
Robert
Quoted 8 lines
drivers/net/wireless/ath/ath9k/htc_drv_beacon.c +0
-0@@ -290,6 +290,9 @@ void ath9k_htc_swba(struct ath9k_htc_priv *priv,struct ath_common *common =ath9k_hw_common (priv->ah);int slot;Quoted 7 lines
drivers/net/wireless/ath/ath9k/htc_drv_beacon.c void ath9k_htc_swba(struct ath9k_htc_priv *priv, · line 298
Quoted 6 lines
drivers/net/wireless/ath/ath9k/htc_drv_beacon.c +3
-0+if (!priv->cur_beacon_conf.enable_beacon) +return ; +if (swba->beacon_pending !=0 ) { priv->beacon.bmisscnt++;if (priv->beacon.bmisscnt >BSTUCK_THRESHOLD ) {
Quoted 1 line
Toke, care to submit a patch I can apply?
Yup, will do.
-Toke