Skip to contentSkip to conversation

divide by zero in ath9k_htc_choose_bslot()

DISCUSSION [email protected] 3 participants 5 messages Mar 27, 2025 – Apr 2, 2025 Qualcomm Atheros ATH9K wireless driver

lore
Share & tools
lore
#0 lore raw

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]

#1 lore raw
Quoted 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 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.

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
index 547634f82183..81fa7cbad892 100644
@@ -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) {
#2 lore raw

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
index 547634f82183..81fa7cbad892 100644
@@ -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) {
#3 lore raw
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
index 547634f82183..81fa7cbad892 100644
@@ -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

#4 lore raw
Quoted 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
index 547634f82183..81fa7cbad892 100644
@@ -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