[PATCH] zd1211rw/mac: Fix out-of-bounds upon RX when unassociated

Lubomir Rintel posted 1 patch 2 months ago
drivers/net/wireless/zydas/zd1211rw/zd_mac.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
[PATCH] zd1211rw/mac: Fix out-of-bounds upon RX when unassociated
Posted by Lubomir Rintel 2 months ago
Upon plugging the adapter, UBSAN complains about an out of bounds read:

  zd1211rw 1-12:1.0: firmware version 4725
  zd1211rw 1-12:1.0: zd1211b chip 083a:4505 v4810 high 00-13-f7 AL2230_RF pa0 g--NS
  ------------[ cut here ]------------
  UBSAN: array-index-out-of-bounds in drivers/net/wireless/zydas/zd1211rw/zd_mac.c:1059:26
  index -1 is out of range for type 'ieee80211_channel [14]'
  CPU: 0 UID: 0 PID: 0 Comm: swapper/0 Not tainted 6.17.12-300.fc43.x86_64 #1 PREEMPT(lazy)
  Hardware name: MSI MS-7A68/B250 KRAIT GAMING (MS-7A68), BIOS H.00 12/15/2016
  Call Trace:
   <IRQ>
   dump_stack_lvl+0x5d/0x80
   ubsan_epilogue+0x5/0x2b
   __ubsan_handle_out_of_bounds.cold+0x54/0x59
   zd_mac_rx+0x453/0x470 [zd1211rw]
   ? raise_softirq_irqoff+0x9/0x30
   rx_urb_complete+0x178/0x260 [zd1211rw]
   ? queue_work_on+0x73/0x80
   ? led_trigger_blink_oneshot+0x78/0xa0
   __usb_hcd_giveback_urb+0x9d/0x120
   usb_giveback_urb_bh+0xb1/0x140
   process_one_work+0x18f/0x350
   bh_worker+0x1ac/0x210
   tasklet_action+0x10/0x30
   handle_softirqs+0xed/0x340
   __irq_exit_rcu+0xcb/0xf0
   common_interrupt+0x85/0xa0
   </IRQ>
   <TASK>
   asm_common_interrupt+0x26/0x40
  RIP: 0010:cpuidle_enter_state+0xcc/0x660

  Code: 00 00 e8 f7 36 ef fe e8 72 f0 ff ff 49 89 c4 0f
        1f 44 00 00 31 ff e8 23 55 ed fe 45 84 ff 0f 85
        02 02 00 00 fb 0f 1f 44 00 00 <85> ed 0f 88 d3 01
        00 00 4c 63 f5 49 83 fe 0a 0f 83 9f 04 00 00 49
  RSP: 0018:ffffffff9dc03df8 EFLAGS: 00000246
  RAX: ffff8a5440447000 RBX: ffff8a53df03ea40 RCX: 0000000000000000
  RDX: 001bd812ca957581 RSI: 00000000248799d1 RDI: 0000000000000000
  RBP: 0000000000000002 R08: fffffffc74360e92 R09: ffff8a53df02cbe0
  R10: 001bd81656731313 R11: 0000000000000000 R12: 001bd812ca957581
  R13: ffffffff9df0fa60 R14: 0000000000000002 R15: 0000000000000000
   ? cpuidle_enter_state+0xbd/0x660
   cpuidle_enter+0x31/0x50
   cpuidle_idle_call+0xf5/0x160
   ? ktime_get+0x3c/0xf0
   do_idle+0x78/0xd0
   cpu_startup_entry+0x29/0x30
   rest_init+0xe7/0xf0
   start_kernel+0x49d/0x4a0
   x86_64_start_reservations+0x24/0x30
   x86_64_start_kernel+0x126/0x130
   common_startup_64+0x13e/0x141
   </TASK>
  ---[ end trace ]---

It is correct: if zd_mac_rx() is called when unassociated, the channel
is 0, becomes -1 under assumption there's a proper channel number
starting from 1.

Add a missing bounds check.

Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>
---
 drivers/net/wireless/zydas/zd1211rw/zd_mac.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/zydas/zd1211rw/zd_mac.c b/drivers/net/wireless/zydas/zd1211rw/zd_mac.c
index 03948f0b4628..c59d1929a69c 100644
--- a/drivers/net/wireless/zydas/zd1211rw/zd_mac.c
+++ b/drivers/net/wireless/zydas/zd1211rw/zd_mac.c
@@ -1022,6 +1022,7 @@ int zd_mac_rx(struct ieee80211_hw *hw, const u8 *buffer, unsigned int length)
 	const struct rx_status *status;
 	struct sk_buff *skb;
 	int bad_frame = 0;
+	u8 channel;
 	__le16 fc;
 	int need_padding;
 	int i;
@@ -1055,7 +1056,9 @@ int zd_mac_rx(struct ieee80211_hw *hw, const u8 *buffer, unsigned int length)
 		}
 	}
 
-	stats.freq = zd_channels[_zd_chip_get_channel(&mac->chip) - 1].center_freq;
+	channel = _zd_chip_get_channel(&mac->chip);
+	if (channel > 0 && channel <= ARRAY_SIZE(zd_channels))
+		stats.freq = zd_channels[channel - 1].center_freq;
 	stats.band = NL80211_BAND_2GHZ;
 	stats.signal = zd_check_signal(hw, status->signal_strength);
 
-- 
2.52.0
Re: [PATCH] zd1211rw/mac: Fix out-of-bounds upon RX when unassociated
Posted by Johannes Berg 1 month, 3 weeks ago
On Wed, 2026-04-15 at 19:08 +0200, Lubomir Rintel wrote:
> Upon plugging the adapter, UBSAN complains about an out of bounds read:

> It is correct: if zd_mac_rx() is called when unassociated, the channel
> is 0, becomes -1 under assumption there's a proper channel number
> starting from 1.

The 'unassociated' part here seems rather misleading - it's basically
when it's called before mac80211 configures the channel?

Maybe zd_rf_init() should set the channel to 1 or whatever the default
channel of the device is? I'm surprised it even receives anything before
being properly configured. But I also see no way to ever go to an
invalid channel number again after the initial configuration of a
channel.

Never mind that the reporting is also racy, but I guess it doesn't
really matter too much ... I had a couple of these devices but they all
broke.

I don't mind applying this patch either, but please edit the commit
message to not mislead about association. If I'm reading it correctly
then all that matters is that it receives before a channel config, and
that only happens as a race during init.

johannes