* [PATCH v2] staging: rtl8723bs: fix protected RX frame validation in decrypt path
@ 2026-09-03 9:19 Tianchu Chen
2026-09-07 16:13 ` Greg KH
0 siblings, 1 reply; 5+ messages in thread
From: Tianchu Chen @ 2026-09-03 9:19 UTC (permalink / raw)
To: gregkh, hansg; +Cc: linux-staging, linux-kernel
From ee6c5863754274f8dc40461adb5e5a98566aed58 Mon Sep 17 00:00:00 2001
From: Tianchu Chen <flynnnchen@tencent.com>
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
Cc: stable@vger.kernel.org
Signed-off-by: Tianchu Chen <flynnnchen@tencent.com>
---
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(-)
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;
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;
--
2.51.0
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH v2] staging: rtl8723bs: fix protected RX frame validation in decrypt path
2026-09-03 9:19 [PATCH v2] staging: rtl8723bs: fix protected RX frame validation in decrypt path Tianchu Chen
@ 2026-09-07 16:13 ` Greg KH
2026-09-08 16:02 ` Tianchu Chen
0 siblings, 1 reply; 5+ messages in thread
From: Greg KH @ 2026-09-07 16:13 UTC (permalink / raw)
To: Tianchu Chen; +Cc: hansg, linux-staging, linux-kernel
On Thu, Sep 03, 2026 at 09:19:34AM +0000, Tianchu Chen wrote:
> >From ee6c5863754274f8dc40461adb5e5a98566aed58 Mon Sep 17 00:00:00 2001
> From: Tianchu Chen <flynnnchen@tencent.com>
> 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.
> 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.
>
> 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
> Cc: stable@vger.kernel.org
> Signed-off-by: Tianchu Chen <flynnnchen@tencent.com>
> ---
> 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(-)
>
> 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;
>
> 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?
>
> + /* 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
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH v2] staging: rtl8723bs: fix protected RX frame validation in decrypt path
2026-09-07 16:13 ` Greg KH
@ 2026-09-08 16:02 ` Tianchu Chen
2026-09-09 7:44 ` Greg KH
0 siblings, 1 reply; 5+ messages in thread
From: Tianchu Chen @ 2026-09-08 16:02 UTC (permalink / raw)
To: Greg KH; +Cc: hansg, linux-staging, linux-kernel
September 8, 2026 at 12:13 AM, "Greg KH" <gregkh@linuxfoundation.org mailto:gregkh@linuxfoundation.org?to=%22Greg%20KH%22%20%3Cgregkh%40linuxfoundation.org%3E > wrote:
-snip-
> >
> > 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;
> >
> > 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.
> >
> > + /* 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?
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
>
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
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH v2] staging: rtl8723bs: fix protected RX frame validation in decrypt path
2026-09-08 16:02 ` Tianchu Chen
@ 2026-09-09 7:44 ` Greg KH
2026-09-10 8:41 ` Tianchu Chen
0 siblings, 1 reply; 5+ messages in thread
From: Greg KH @ 2026-09-09 7:44 UTC (permalink / raw)
To: Tianchu Chen; +Cc: hansg, linux-staging, linux-kernel
On Tue, Sep 08, 2026 at 04:02:15PM +0000, Tianchu Chen wrote:
> September 8, 2026 at 12:13 AM, "Greg KH" <gregkh@linuxfoundation.org mailto:gregkh@linuxfoundation.org?to=%22Greg%20KH%22%20%3Cgregkh%40linuxfoundation.org%3E > wrote:
>
>
> -snip-
>
> > >
> > > 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;
> > >
> > > 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.
>
>
> > >
> > > + /* 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?
>
> 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
> >
>
> 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
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH v2] staging: rtl8723bs: fix protected RX frame validation in decrypt path
2026-09-09 7:44 ` Greg KH
@ 2026-09-10 8:41 ` Tianchu Chen
0 siblings, 0 replies; 5+ messages in thread
From: Tianchu Chen @ 2026-09-10 8:41 UTC (permalink / raw)
To: Greg KH; +Cc: hansg, linux-staging, linux-kernel
September 9, 2026 at 3:44 PM, "Greg KH" <gregkh@linuxfoundation.org mailto:gregkh@linuxfoundation.org?to=%22Greg%20KH%22%20%3Cgregkh%40linuxfoundation.org%3E > wrote:
>
> On Tue, Sep 08, 2026 at 04:02:15PM +0000, Tianchu Chen wrote:
>
> >
> > September 8, 2026 at 12:13 AM, "Greg KH" <gregkh@linuxfoundation.org mailto:gregkh@linuxfoundation.org?to=%22Greg%20KH%22%20%3Cgregkh%40linuxfoundation.org%3E > wrote:
> >
> >
> > -snip-
> >
> > >
> > > 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;
> > >
> > > 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.
> >
> >
> > >
> > > + /* 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?
> >
> > 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
> >
> >
> > 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/8ee3bdd1c45034200cb1aa9a7e9b575b9584643a@linux.dev/
Best regards,
Tianchu
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2026-09-10 8:41 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-09-03 9:19 [PATCH v2] staging: rtl8723bs: fix protected RX frame validation in decrypt path Tianchu Chen
2026-09-07 16:13 ` Greg KH
2026-09-08 16:02 ` Tianchu Chen
2026-09-09 7:44 ` Greg KH
2026-09-10 8:41 ` Tianchu Chen
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®