Skip to contentSkip to conversation

staging: rtl8723bs: fix protected RX frame validation in decrypt path

PATCH Superseded Tianchu Chen 2 participants 5 messages Sep 3 – Sep 10 Staging

v2 of 3 Superseded Latest is v3 →

lore
Share & tools
lore
#0 lore raw

PATCH v2 1/1 staging: rtl8723bs: fix protected RX frame validation in decrypt path

From ee6c5863754274f8dc40461adb5e5a98566aed58 Mon Sep 17 00:00:00 2001
From: Tianchu Chen <[email protected]>
Date: Wed, 19 Aug 2026 21:55:30 +0800
Subject: [PATCH v2] staging: rtl8723bs: fix protected RX frame validation in decrypt path

Resending this because a real device has been purchased from the second-hand market and confirms the bug is triggerable; details see below.

The RX software decrypt path mishandles crafted protected frames from a malicious AP in two ways:

1) decryptor() never checks that a protected frame is long enough to hold the 802.11 header plus the per-cipher trailer. All of rtw_wep_decrypt(), rtw_tkip_decrypt() and rtw_aes_decrypt() compute

	length = hdr.len - hdrlen - iv_len;

and act on the result, and recvframe_chkmic() similarly computes

	datalen = hdr.len - hdrlen - iv_len - icv_len - 8;

for TKIP (the 8-byte Michael MIC is not accounted for in icv_len). A frame shorter than hdrlen + iv_len + icv_len (+ 8 for TKIP) underflows these unsigned subtractions, turning into a ~4 GiB iteration count in aes_decipher()/arc4_crypt()/rtw_seccalctkipmic() or a ~4 GiB crc32_le() length, causing out-of-bounds reads/writes of up to ~4 GiB starting from the rx skb. Reject such frames in decryptor() before touching the IV; this covers all three ciphers and the TKIP MIC check, and also keeps the unconditional iv[3] access within the frame.

2) validate_80211w_mgmt() keeps using the frame - two memcpys through the stale rx_data pointer and a pkt_len update - before checking whether decryptor() returned NULL. On decrypt failure (e.g. a unicast protected deauth/disassoc/action frame with a bad MIC, which this path always software-decrypts) the skb has already been freed, so this is a use-after-free read/write, and the caller's error path frees the frame a second time, drifting free_recvframe_cnt. Bail out immediately when decryptor() fails, freeing the temporary mgmt_DATA buffer first.

Discovered by Atuin - Automated Vulnerability Discovery Engine.

Fixes 554c0a3abf21 ("staging: Add rtl8723bs sdio wifi driver")
Assisted-by LLM

---
v2: no code change; adds the real-hardware test report requested for LLM-assisted staging patches.

Test report
=========================

Victim: Wintel CX-W8 mini PC (Atom Z3735F, on-board RTL8723BS chip, staging r8723bs,
Debian 13.6.0/Kernel 6.12.94), bought from the second-hand market for this test.
Attacker: ESP32-S3 dev board running custom firmware as a rogue AP. Patching a proprietary
library is needed. Details available to reviewers upon request.

The decryptor bug is reachable at the association stage: the vulnerable decrypt path is taken before any key is installed, so the attacker never needs to pass key verification - the rogue AP's PSK does not need to match the victim's. Knowing an SSID the victim will auto-join is enough.

Once the victim associates, the rogue AP send 802.11 data frames with FromDS + Protected bit set and a total length of 24..31 bytes, i.e. shorter than their own CCMP IV (hdrlen 24 + iv_len 8). With no key in the HW CAM the frames reach decryptor() with bdecrypted == 0, and rtw_aes_decrypt() computes length = (24..31) - 24 - 8, a ~4 GiB underflow fed straight into aes_decipher().

Result: the victim kernel panics within seconds of association. Captured via netconsole:

[  939.937797] Oops: general protection fault, probably for non-canonical address 0x6de15a87d1c57856: 0000 [#1] PREEMPT SMP PTI
[  939.938075] CPU: 3 UID: 0 PID: 1637 Comm: Xorg Tainted: G         C         6.12.94+deb13-amd64 #1  Debian 6.12.94-1
[  939.938288] Tainted: [C]=CRAP
[  939.938368] Hardware name: Insyde Intel CX-W8/Intel CX-W8, BIOS M.W8RAAA01 04/17/2015
[  939.938515] RIP: 0010:i915_gem_do_execbuffer+0xbd1/0x2950 [i915]
[  939.939945] Code: e2 fe 48 89 50 20 48 8d 74 24 58 4c 89 e9 48 8d 7c 24 60 44 89 f2 e8 ce c2 ff ff 85 c0 0f 85 9d 03 00 00 49 8b bd b8 00 00 00 <48> 83 bf 50 04 00 00 00 74 31 e8 10 1b 01 00 85 c0 0f 85 91 18 00
[  939.940255] RSP: 0018:ffffccdfc0b17788 EFLAGS: 00010246
[  939.940382] RAX: 0000000000000000 RBX: 0000000000000009 RCX: 00000000fffffff5
[  939.940524] RDX: 0000000000000001 RSI: ffff8c5aa8d4d800 RDI: 6de15a87d1c57406
[  939.940664] RBP: 00000000000001f8 R08: ffff8c5aa92e4a90 R09: ffff8c5aaa953518
[  939.940805] R10: 0000000000000000 R11: 0000000000000000 R12: 00000000000003a9
[  939.940942] R13: ffff8c5af99f1680 R14: 0000000000000009 R15: 0000000000000000
[  939.941081] FS:  00007f3856594b00(0000) GS:ffff8c5afb980000(0000) knlGS:0000000000000000
[  939.941238] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[  939.941358] CR2: 00007f3856155770 CR3: 000000002b7e6000 CR4: 00000000001026f0
[  939.941500] Call Trace:
[  939.941579]  <TASK>
[  939.941702]  i915_gem_execbuffer2_ioctl+0x13b/0x260 [i915]
[  939.943134]  ? __pfx_i915_gem_execbuffer2_ioctl+0x10/0x10 [i915]
[  939.944543]  drm_ioctl_kernel+0xb0/0x100 [drm]
[  939.945069]  drm_ioctl+0x277/0x4d0 [drm]
[  939.945571]  ? __pfx_i915_gem_execbuffer2_ioctl+0x10/0x10 [i915]
[  939.946997]  __x64_sys_ioctl+0x97/0xc0
[  939.947119]  do_syscall_64+0x87/0x1b0
[  939.947235]  ? timerqueue_del+0x2e/0x50
[  939.947343]  ? tomoyo_init_request_info+0x99/0xc0
[  939.947470]  ? tomoyo_path_number_perm+0x8c/0x1f0
[  939.947589]  ? ww_mutex_lock_interruptible+0x18/0x80
[  939.947725]  ? i915_gem_madvise_ioctl+0x239/0x340 [i915]
[  939.949150]  ? __pfx_i915_gem_madvise_ioctl+0x10/0x10 [i915]
[  939.950571]  ? drm_ioctl_kernel+0xb0/0x100 [drm]
[  939.951086]  ? __check_object_size+0x50/0x210
[  939.955215]  ? _copy_to_user+0x36/0x50
[  939.959506]  ? drm_ioctl+0x2a1/0x4d0 [drm]
[  939.964159]  ? __pfx_i915_gem_madvise_ioctl+0x10/0x10 [i915]
[  939.969804]  ? arch_exit_to_user_mode_prepare.isra.0+0x16/0xa0
[  939.974146]  ? syscall_exit_to_user_mode+0x37/0x1b0
[  939.978488]  ? do_syscall_64+0x93/0x1b0
[  939.982872]  ? arch_exit_to_user_mode_prepare.isra.0+0x16/0xa0
[  939.987300]  entry_SYSCALL_64_after_hwframe+0x76/0x7e
[  939.991768] RIP: 0033:0x7f385691491b
[  939.996285] Code: 00 48 89 44 24 18 31 c0 48 8d 44 24 60 c7 04 24 10 00 00 00 48 89 44 24 08 48 8d 44 24 20 48 89 44 24 10 b8 10 00 00 00 0f 05 <89> c2 3d 00 f0 ff ff 77 1c 48 8b 44 24 18 64 48 2b 04 25 28 00 00
[  940.001138] RSP: 002b:00007ffdb0cf6710 EFLAGS: 00000246 ORIG_RAX: 0000000000000010
[  940.006061] RAX: ffffffffffffffda RBX: 0000564e32defb08 RCX: 00007f385691491b
[  940.011048] RDX: 00007ffdb0cf6790 RSI: 0000000040406469 RDI: 0000000000000013
[  940.016090] RBP: 0000000000000000 R08: 0000000000000000 R09: 0000000000000004
[  940.021150] R10: 0000000000000020 R11: 0000000000000246 R12: 00007ffdb0cf6790
[  940.026256] R13: 0000564e32dc3a50 R14: 00000000000000dc R15: 0000000000000013
[  940.031431]  </TASK>

The oops surfacing in i915 rather than in the driver is expected:
A non-canonical garbage pointer in an unrelated subsystem is the expected signature of the multi-GB linear decipher starting from the rx skb.

 drivers/staging/rtl8723bs/core/rtw_recv.c | 21 ++++++++++++++++++---
 1 file changed, 18 insertions(+), 3 deletions(-)
drivers/staging/rtl8723bs/core/rtw_recv.c +18 -3
index 7568fc514d7ce..4756e0fedd46f 100644
@@ -426,8 +426,21 @@ static union recv_frame *decryptor(struct adapter *padapter, union recv_frame *p
 	u32  res = _SUCCESS;
 
 	if (prxattrib->encrypt > 0) {
-		u8 *iv = precv_frame->u.hdr.rx_data + prxattrib->hdrlen;
+		u8 *iv;
+		u32 min_len = prxattrib->hdrlen + prxattrib->iv_len + prxattrib->icv_len;
 
+		/* TKIP appends an 8-byte Michael MIC that icv_len doesn't account for */
+		if (prxattrib->encrypt == _TKIP_)
+			min_len += 8;
+
+		/* a protected frame must be long enough to hold the IV and ICV/MIC */
+		if (precv_frame->u.hdr.len < min_len) {
+			rtw_free_recvframe(precv_frame,
+					   &padapter->recvpriv.free_recv_queue);
+			return NULL;
+		}
+
+		iv = precv_frame->u.hdr.rx_data + prxattrib->hdrlen;
 		prxattrib->key_index = (((iv[3]) >> 6) & 0x3);
 
 		if (prxattrib->key_index > WEP_KEYS) {
@@ -1395,6 +1408,10 @@ static signed int validate_80211w_mgmt(struct adapter *adapter, union recv_frame
 			if (!mgmt_DATA)
 				goto validate_80211w_fail;
 			precv_frame = decryptor(adapter, precv_frame);
+			if (!precv_frame) {
+				kfree(mgmt_DATA);
+				goto validate_80211w_fail;
+			}
 			/* save actual management data frame body */
 			memcpy(mgmt_DATA, ptr + pattrib->hdrlen + pattrib->iv_len, data_len);
 			/* overwrite the iv field */
@@ -1402,8 +1419,6 @@ static signed int validate_80211w_mgmt(struct adapter *adapter, union recv_frame
 			/* remove the iv and icv length */
 			pattrib->pkt_len = pattrib->pkt_len - pattrib->iv_len - pattrib->icv_len;
 			kfree(mgmt_DATA);
-			if (!precv_frame)
-				goto validate_80211w_fail;
 		} else if (is_multicast_ether_addr(GetAddr1Ptr(ptr)) &&
 			(subtype == WIFI_DEAUTH || subtype == WIFI_DISASSOC)) {
 			signed int BIP_ret = _SUCCESS;
Signature · 1 line
2.51.0
maintainer
#1 lore raw
Quoted 4 lines · Tianchu Chen
Quoted 1 line

From ee6c5863754274f8dc40461adb5e5a98566aed58 Mon Sep 17 00:00:00 2001

From: Tianchu Chen <[email protected]>
Date: Wed, 19 Aug 2026 21:55:30 +0800
Subject: [PATCH v2] staging: rtl8723bs: fix protected RX frame validation in decrypt path

This shouldn't be in the body of the email.

Quoted 2 lines

Resending this because a real device has been purchased from the second-hand market and confirms the bug is triggerable; details see below.

This shouldn't be here either, it goes below the --- line.

Quoted 128 lines

The RX software decrypt path mishandles crafted protected frames from a malicious AP in two ways:

1) decryptor() never checks that a protected frame is long enough to hold the 802.11 header plus the per-cipher trailer. All of rtw_wep_decrypt(), rtw_tkip_decrypt() and rtw_aes_decrypt() compute

	length = hdr.len - hdrlen - iv_len;

and act on the result, and recvframe_chkmic() similarly computes

	datalen = hdr.len - hdrlen - iv_len - icv_len - 8;

for TKIP (the 8-byte Michael MIC is not accounted for in icv_len). A frame shorter than hdrlen + iv_len + icv_len (+ 8 for TKIP) underflows these unsigned subtractions, turning into a ~4 GiB iteration count in aes_decipher()/arc4_crypt()/rtw_seccalctkipmic() or a ~4 GiB crc32_le() length, causing out-of-bounds reads/writes of up to ~4 GiB starting from the rx skb. Reject such frames in decryptor() before touching the IV; this covers all three ciphers and the TKIP MIC check, and also keeps the unconditional iv[3] access within the frame.

2) validate_80211w_mgmt() keeps using the frame - two memcpys through the stale rx_data pointer and a pkt_len update - before checking whether decryptor() returned NULL. On decrypt failure (e.g. a unicast protected deauth/disassoc/action frame with a bad MIC, which this path always software-decrypts) the skb has already been freed, so this is a use-after-free read/write, and the caller's error path frees the frame a second time, drifting free_recvframe_cnt. Bail out immediately when decryptor() fails, freeing the temporary mgmt_DATA buffer first.

Discovered by Atuin - Automated Vulnerability Discovery Engine.

Fixes 554c0a3abf21 ("staging: Add rtl8723bs sdio wifi driver")
Assisted-by LLM

---
v2: no code change; adds the real-hardware test report requested for LLM-assisted staging patches.

Test report
=========================

Victim: Wintel CX-W8 mini PC (Atom Z3735F, on-board RTL8723BS chip, staging r8723bs,
Debian 13.6.0/Kernel 6.12.94), bought from the second-hand market for this test.
Attacker: ESP32-S3 dev board running custom firmware as a rogue AP. Patching a proprietary
library is needed. Details available to reviewers upon request.

The decryptor bug is reachable at the association stage: the vulnerable decrypt path is taken before any key is installed, so the attacker never needs to pass key verification - the rogue AP's PSK does not need to match the victim's. Knowing an SSID the victim will auto-join is enough.

Once the victim associates, the rogue AP send 802.11 data frames with FromDS + Protected bit set and a total length of 24..31 bytes, i.e. shorter than their own CCMP IV (hdrlen 24 + iv_len 8). With no key in the HW CAM the frames reach decryptor() with bdecrypted == 0, and rtw_aes_decrypt() computes length = (24..31) - 24 - 8, a ~4 GiB underflow fed straight into aes_decipher().

Result: the victim kernel panics within seconds of association. Captured via netconsole:

[  939.937797] Oops: general protection fault, probably for non-canonical address 0x6de15a87d1c57856: 0000 [#1] PREEMPT SMP PTI
[  939.938075] CPU: 3 UID: 0 PID: 1637 Comm: Xorg Tainted: G         C         6.12.94+deb13-amd64 #1  Debian 6.12.94-1
[  939.938288] Tainted: [C]=CRAP
[  939.938368] Hardware name: Insyde Intel CX-W8/Intel CX-W8, BIOS M.W8RAAA01 04/17/2015
[  939.938515] RIP: 0010:i915_gem_do_execbuffer+0xbd1/0x2950 [i915]
[  939.939945] Code: e2 fe 48 89 50 20 48 8d 74 24 58 4c 89 e9 48 8d 7c 24 60 44 89 f2 e8 ce c2 ff ff 85 c0 0f 85 9d 03 00 00 49 8b bd b8 00 00 00 <48> 83 bf 50 04 00 00 00 74 31 e8 10 1b 01 00 85 c0 0f 85 91 18 00
[  939.940255] RSP: 0018:ffffccdfc0b17788 EFLAGS: 00010246
[  939.940382] RAX: 0000000000000000 RBX: 0000000000000009 RCX: 00000000fffffff5
[  939.940524] RDX: 0000000000000001 RSI: ffff8c5aa8d4d800 RDI: 6de15a87d1c57406
[  939.940664] RBP: 00000000000001f8 R08: ffff8c5aa92e4a90 R09: ffff8c5aaa953518
[  939.940805] R10: 0000000000000000 R11: 0000000000000000 R12: 00000000000003a9
[  939.940942] R13: ffff8c5af99f1680 R14: 0000000000000009 R15: 0000000000000000
[  939.941081] FS:  00007f3856594b00(0000) GS:ffff8c5afb980000(0000) knlGS:0000000000000000
[  939.941238] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[  939.941358] CR2: 00007f3856155770 CR3: 000000002b7e6000 CR4: 00000000001026f0
[  939.941500] Call Trace:
[  939.941579]  <TASK>
[  939.941702]  i915_gem_execbuffer2_ioctl+0x13b/0x260 [i915]
[  939.943134]  ? __pfx_i915_gem_execbuffer2_ioctl+0x10/0x10 [i915]
[  939.944543]  drm_ioctl_kernel+0xb0/0x100 [drm]
[  939.945069]  drm_ioctl+0x277/0x4d0 [drm]
[  939.945571]  ? __pfx_i915_gem_execbuffer2_ioctl+0x10/0x10 [i915]
[  939.946997]  __x64_sys_ioctl+0x97/0xc0
[  939.947119]  do_syscall_64+0x87/0x1b0
[  939.947235]  ? timerqueue_del+0x2e/0x50
[  939.947343]  ? tomoyo_init_request_info+0x99/0xc0
[  939.947470]  ? tomoyo_path_number_perm+0x8c/0x1f0
[  939.947589]  ? ww_mutex_lock_interruptible+0x18/0x80
[  939.947725]  ? i915_gem_madvise_ioctl+0x239/0x340 [i915]
[  939.949150]  ? __pfx_i915_gem_madvise_ioctl+0x10/0x10 [i915]
[  939.950571]  ? drm_ioctl_kernel+0xb0/0x100 [drm]
[  939.951086]  ? __check_object_size+0x50/0x210
[  939.955215]  ? _copy_to_user+0x36/0x50
[  939.959506]  ? drm_ioctl+0x2a1/0x4d0 [drm]
[  939.964159]  ? __pfx_i915_gem_madvise_ioctl+0x10/0x10 [i915]
[  939.969804]  ? arch_exit_to_user_mode_prepare.isra.0+0x16/0xa0
[  939.974146]  ? syscall_exit_to_user_mode+0x37/0x1b0
[  939.978488]  ? do_syscall_64+0x93/0x1b0
[  939.982872]  ? arch_exit_to_user_mode_prepare.isra.0+0x16/0xa0
[  939.987300]  entry_SYSCALL_64_after_hwframe+0x76/0x7e
[  939.991768] RIP: 0033:0x7f385691491b
[  939.996285] Code: 00 48 89 44 24 18 31 c0 48 8d 44 24 60 c7 04 24 10 00 00 00 48 89 44 24 08 48 8d 44 24 20 48 89 44 24 10 b8 10 00 00 00 0f 05 <89> c2 3d 00 f0 ff ff 77 1c 48 8b 44 24 18 64 48 2b 04 25 28 00 00
[  940.001138] RSP: 002b:00007ffdb0cf6710 EFLAGS: 00000246 ORIG_RAX: 0000000000000010
[  940.006061] RAX: ffffffffffffffda RBX: 0000564e32defb08 RCX: 00007f385691491b
[  940.011048] RDX: 00007ffdb0cf6790 RSI: 0000000040406469 RDI: 0000000000000013
[  940.016090] RBP: 0000000000000000 R08: 0000000000000000 R09: 0000000000000004
[  940.021150] R10: 0000000000000020 R11: 0000000000000246 R12: 00007ffdb0cf6790
[  940.026256] R13: 0000564e32dc3a50 R14: 00000000000000dc R15: 0000000000000013
[  940.031431]  </TASK>

The oops surfacing in i915 rather than in the driver is expected:
A non-canonical garbage pointer in an unrelated subsystem is the expected signature of the multi-GB linear decipher starting from the rx skb.

 drivers/staging/rtl8723bs/core/rtw_recv.c | 21 ++++++++++++++++++---
 1 file changed, 18 insertions(+), 3 deletions(-)
drivers/staging/rtl8723bs/core/rtw_recv.c +0 -0
index 7568fc514d7ce..4756e0fedd46f 100644

drivers/staging/rtl8723bs/core/rtw_recv.c static union recv_frame *decryptor(struct adapter *padapter, union recv_frame *p · line 430

Quoted 6 lines
drivers/staging/rtl8723bs/core/rtw_recv.c +2 -1
@@ -426,8 +426,21 @@ static union recv_frame *decryptor(struct adapter *padapter, union recv_frame *p
 	u32  res = _SUCCESS;
 
 	if (prxattrib->encrypt > 0) {
-		u8 *iv = precv_frame->u.hdr.rx_data + prxattrib->hdrlen;
+		u8 *iv;
+		u32 min_len = prxattrib->hdrlen + prxattrib->iv_len + prxattrib->icv_len;

Why will this not overflow?

drivers/staging/rtl8723bs/core/rtw_recv.c static union recv_frame *decryptor(struct adapter *padapter, union recv_frame *p · line 443

Quoted 13 lines
drivers/staging/rtl8723bs/core/rtw_recv.c +12 -0
+		/* TKIP appends an 8-byte Michael MIC that icv_len doesn't account for */
+		if (prxattrib->encrypt == _TKIP_)
+			min_len += 8;
+
+		/* a protected frame must be long enough to hold the IV and ICV/MIC */
+		if (precv_frame->u.hdr.len < min_len) {
+			rtw_free_recvframe(precv_frame,
+					   &padapter->recvpriv.free_recv_queue);
+			return NULL;
+		}
+
+		iv = precv_frame->u.hdr.rx_data + prxattrib->hdrlen;

What prevents this from overflowing?

thanks,

greg k-h

#2 lore raw

September 8, 2026 at 12:13 AM, "Greg KH" <[email protected] mailto:[email protected]?to=%22Greg%20KH%22%20%3Cgregkh%40linuxfoundation.org%3E > wrote:

-snip-

Quoted 11 lines
Quoted 11 lines
 
 drivers/staging/rtl8723bs/core/rtw_recv.c | 21 ++++++++++++++++++---
 1 file changed, 18 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/rtl8723bs/core/rtw_recv.c b/drivers/staging/rtl8723bs/core/rtw_recv.c index 7568fc514d7ce..4756e0fedd46f 100644
--- a/drivers/staging/rtl8723bs/core/rtw_recv.c
+++ b/drivers/staging/rtl8723bs/core/rtw_recv.c
@@ -426,8 +426,21 @@ static union recv_frame *decryptor(struct adapter *padapter, union recv_frame *p u32 res = _SUCCESS;

Quoted 6 lines
Quoted 5 lines

if (prxattrib->encrypt > 0) {
- u8 *iv = precv_frame->u.hdr.rx_data + prxattrib->hdrlen;
+ u8 *iv;
+ u32 min_len = prxattrib->hdrlen + prxattrib->iv_len + prxattrib->icv_len;

Why will this not overflow?

The overflow(without this fix) happens inside the per-suite decrypt routines when a received frame is shorter than this minimum. For example:

rtw_aes_decrypt() computes length = len - hdrlen - iv_len, so a 30-byte frame with hdrlen = 26 and iv_len = 8 wraps length to ~4GiB, and aes_decipher() then iterates num_blocks = (plen - 8) / 16 16-byte blocks, reading and writing gigabytes past the skb.

The WEP and TKIP decryptors start from the same subtraction and underflow the same way.

min_len is the minimum size of a legitimate protected frame: header + IV + ICV, plus the 8-byte Michael MIC for TKIP.

The min_len computation itself cannot wrap either: all three addends are u8 fields, hdrlen is at most 36 and iv_len/icv_len are per-suite constants (max 18/16), so the sum stays below 80 even with the TKIP +8. A frame shorter than min_len cannot even hold its IV and ICV, so only malformed frames are dropped.

Quoted 14 lines
Quoted 14 lines

+ /* TKIP appends an 8-byte Michael MIC that icv_len doesn't account for */
+ if (prxattrib->encrypt == _TKIP_)
+ min_len += 8;
+
+ /* a protected frame must be long enough to hold the IV and ICV/MIC */
+ if (precv_frame->u.hdr.len < min_len) {
+ rtw_free_recvframe(precv_frame,
+ &padapter->recvpriv.free_recv_queue);
+ return NULL;
+ }
+
+ iv = precv_frame->u.hdr.rx_data + prxattrib->hdrlen;

Quoted 1 line

What prevents this from overflowing?

The check above keeps the offset within the frame: it guarantees len >= hdrlen + iv_len + icv_len, and iv_len >= 4 for every suite, so len >= hdrlen + 4 and both iv and the iv[3] dereference stay within the first len bytes ([rx_data, rx_data + len)).

Those len bytes are in turn inside the allocation: hdr.len is set by recvframe_put() only after pkt_exceeds_tail() verified that pkt_len bytes were actually copied from the RX FIFO into an skb sized for them (rtl8723bs_recv.c), so any offset below len is inside the buffer.

This is also why the iv assignment moved: the iv[3] read that follows it would otherwise be an out-of-bounds read when the frame is shorter than hdrlen + 4. The IV may only be examined once the frame is known to actually contain it.

Quoted 5 lines

thanks,

greg k-h

Regarding the format issue being mentioned earlier, I can send a v3 patch. Also, I believe decryptor() is also where this check should belongs.

It is the head of the whole post-handle pipeline (decryptor -> chk_defrag -> portctrl -> indicate, rtw_recv.c:2082+), so one check covers not just the three decrypt routines but every later consumer of hdrlen + iv_len offsets. For TKIP the invariant has to hold before both stages of the WPA model - decryption and the Michael MIC verification in recvframe_chkmic(), which subtracts 8 more; the +8 in min_len exists for that second stage, and decryptor() is the only point preceding both.

This matches the WPA model in mac80211: both ieee80211_crypto_tkip_decrypt() and ieee80211_crypto_ccmp_decrypt() reject frames too short for header + IV + ICV/MIC at their entry (net/mac80211/wpa.c).

Since I have the real hardware, I can also verify the execution flow at runtime with a build that adds a temporary printk in this path if that would help.

Please let me know if a v3 is needed, or if there is any further verification you would like me to do on my side.

Best regards,

Tianchu

maintainer
#3 lore raw
Quoted 79 lines · Tianchu Chen

September 8, 2026 at 12:13 AM, "Greg KH" <[email protected] mailto:[email protected]?to=%22Greg%20KH%22%20%3Cgregkh%40linuxfoundation.org%3E > wrote:

-snip-

Quoted 11 lines
Quoted 11 lines
 
 drivers/staging/rtl8723bs/core/rtw_recv.c | 21 ++++++++++++++++++---
 1 file changed, 18 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/rtl8723bs/core/rtw_recv.c b/drivers/staging/rtl8723bs/core/rtw_recv.c index 7568fc514d7ce..4756e0fedd46f 100644
--- a/drivers/staging/rtl8723bs/core/rtw_recv.c
+++ b/drivers/staging/rtl8723bs/core/rtw_recv.c
@@ -426,8 +426,21 @@ static union recv_frame *decryptor(struct adapter *padapter, union recv_frame *p u32 res = _SUCCESS;

Quoted 6 lines
Quoted 5 lines

if (prxattrib->encrypt > 0) {
- u8 *iv = precv_frame->u.hdr.rx_data + prxattrib->hdrlen;
+ u8 *iv;
+ u32 min_len = prxattrib->hdrlen + prxattrib->iv_len + prxattrib->icv_len;

Why will this not overflow?

The overflow(without this fix) happens inside the per-suite decrypt routines when a received frame is shorter than this minimum. For example:

rtw_aes_decrypt() computes length = len - hdrlen - iv_len, so a 30-byte frame with hdrlen = 26 and iv_len = 8 wraps length to ~4GiB, and aes_decipher() then iterates num_blocks = (plen - 8) / 16 16-byte blocks, reading and writing gigabytes past the skb.

The WEP and TKIP decryptors start from the same subtraction and underflow the same way.

min_len is the minimum size of a legitimate protected frame: header + IV + ICV, plus the 8-byte Michael MIC for TKIP.

The min_len computation itself cannot wrap either: all three addends are u8 fields, hdrlen is at most 36 and iv_len/icv_len are per-suite constants (max 18/16), so the sum stays below 80 even with the TKIP +8. A frame shorter than min_len cannot even hold its IV and ICV, so only malformed frames are dropped.

Quoted 14 lines
Quoted 14 lines

+ /* TKIP appends an 8-byte Michael MIC that icv_len doesn't account for */
+ if (prxattrib->encrypt == _TKIP_)
+ min_len += 8;
+
+ /* a protected frame must be long enough to hold the IV and ICV/MIC */
+ if (precv_frame->u.hdr.len < min_len) {
+ rtw_free_recvframe(precv_frame,
+ &padapter->recvpriv.free_recv_queue);
+ return NULL;
+ }
+
+ iv = precv_frame->u.hdr.rx_data + prxattrib->hdrlen;

Quoted 1 line

What prevents this from overflowing?

The check above keeps the offset within the frame: it guarantees len >= hdrlen + iv_len + icv_len, and iv_len >= 4 for every suite, so len >= hdrlen + 4 and both iv and the iv[3] dereference stay within the first len bytes ([rx_data, rx_data + len)).

Those len bytes are in turn inside the allocation: hdr.len is set by recvframe_put() only after pkt_exceeds_tail() verified that pkt_len bytes were actually copied from the RX FIFO into an skb sized for them (rtl8723bs_recv.c), so any offset below len is inside the buffer.

This is also why the iv assignment moved: the iv[3] read that follows it would otherwise be an out-of-bounds read when the frame is shorter than hdrlen + 4. The IV may only be examined once the frame is known to actually contain it.

Quoted 1 line
Quoted 7 lines
Quoted 4 lines

thanks,

greg k-h

Regarding the format issue being mentioned earlier, I can send a v3 patch. Also, I believe decryptor() is also where this check should belongs.

Please send a v3.

thanks,

greg k-h

#4 lore raw
Quoted 88 lines · Greg KH
Quoted 85 lines · Tianchu Chen

September 8, 2026 at 12:13 AM, "Greg KH" <[email protected] mailto:[email protected]?to=%22Greg%20KH%22%20%3Cgregkh%40linuxfoundation.org%3E > wrote:

-snip-

Quoted 9 lines

drivers/staging/rtl8723bs/core/rtw_recv.c | 21 ++++++++++++++++++---

1 file changed, 18 insertions(+), 3 deletions(-)
drivers/staging/rtl8723bs/core/rtw_recv.c +0 -0
index 7568fc514d7ce..4756e0fedd46f 100644

drivers/staging/rtl8723bs/core/rtw_recv.c static union recv_frame *decryptor(struct adapter *padapter, union recv_frame *p · line 431

Quoted 7 lines
drivers/staging/rtl8723bs/core/rtw_recv.c +2 -1
@@ -426,8 +426,21 @@ static union recv_frame *decryptor(struct adapter *padapter, union recv_frame *p
 u32 res = _SUCCESS;
 
 if (prxattrib->encrypt > 0) {
- u8 *iv = precv_frame->u.hdr.rx_data + prxattrib->hdrlen;
+ u8 *iv;
+ u32 min_len = prxattrib->hdrlen + prxattrib->iv_len + prxattrib->icv_len;
 

Why will this not overflow?

The overflow(without this fix) happens inside the per-suite decrypt routines when a received frame is shorter than this minimum. For example:

rtw_aes_decrypt() computes length = len - hdrlen - iv_len, so a 30-byte frame with hdrlen = 26 and iv_len = 8 wraps length to ~4GiB, and aes_decipher() then iterates num_blocks = (plen - 8) / 16 16-byte blocks, reading and writing gigabytes past the skb.

The WEP and TKIP decryptors start from the same subtraction and underflow the same way.

min_len is the minimum size of a legitimate protected frame: header + IV + ICV, plus the 8-byte Michael MIC for TKIP.

The min_len computation itself cannot wrap either: all three addends are u8 fields, hdrlen is at most 36 and iv_len/icv_len are per-suite constants (max 18/16), so the sum stays below 80 even with the TKIP +8. A frame shorter than min_len cannot even hold its IV and ICV, so only malformed frames are dropped.

Quoted 4 lines
drivers/staging/rtl8723bs/core/rtw_recv.c +3 -0
+ /* TKIP appends an 8-byte Michael MIC that icv_len doesn't account for */
+ if (prxattrib->encrypt == _TKIP_)
+ min_len += 8;

drivers/staging/rtl8723bs/core/rtw_recv.c static union recv_frame *decryptor(struct adapter *padapter, union recv_frame *p · line 445

Quoted 10 lines
drivers/staging/rtl8723bs/core/rtw_recv.c +9 -0
+
+ /* a protected frame must be long enough to hold the IV and ICV/MIC */
+ if (precv_frame->u.hdr.len < min_len) {
+ rtw_free_recvframe(precv_frame,
+ &padapter->recvpriv.free_recv_queue);
+ return NULL;
+ }
+
+ iv = precv_frame->u.hdr.rx_data + prxattrib->hdrlen;
 

What prevents this from overflowing?

The check above keeps the offset within the frame: it guarantees len >= hdrlen + iv_len + icv_len, and iv_len >= 4 for every suite, so len >= hdrlen + 4 and both iv and the iv[3] dereference stay within the first len bytes ([rx_data, rx_data + len)).

Those len bytes are in turn inside the allocation: hdr.len is set by recvframe_put() only after pkt_exceeds_tail() verified that pkt_len bytes were actually copied from the RX FIFO into an skb sized for them (rtl8723bs_recv.c), so any offset below len is inside the buffer.

This is also why the iv assignment moved: the iv[3] read that follows it would otherwise be an out-of-bounds read when the frame is shorter than hdrlen + 4. The IV may only be examined once the frame is known to actually contain it.

thanks,

greg k-h

Quoted 9 lines
Quoted 3 lines

Regarding the format issue being mentioned earlier, I can send a v3 patch. Also, I believe decryptor() is also where this check should belongs.

Please send a v3.

thanks,

greg k-h

Hi Greg,

V3 has been sent: https://lore.kernel.org/all/[email protected]/

Best regards,

Tianchu