[PATCH] staging: rtl8723bs: properly validate the data in rtw_get_ie_ex()

[PATCH] staging: rtl8723bs: properly validate the data in rtw_get_ie_ex()

[Greg Kroah-Hartman]

Just like in commit 154828bf9559 ("staging: rtl8723bs: fix out-of-bounds
read in rtw_get_ie() parser"), we don't trust the data in the frame so
we should check the length better before acting on it

Cc: Navaneeth K <knavaneeth786@gmail.com>
Cc: stable <stable@kernel.org>
Assisted-by: gkh_clanker_2000
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
Navaneeth, any chance you can test this or at least verify my logic is
correct here?  I got a "hit" from a tool that the work you did in your
commit also needs to be done here, and I _think_ I got it right but do
not have the hardware to test this with at all.  Thanks!

 drivers/staging/rtl8723bs/core/rtw_ieee80211.c | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/rtl8723bs/core/rtw_ieee80211.c b/drivers/staging/rtl8723bs/core/rtw_ieee80211.c
index 6cf217e21593..3e2b5e6b07f9 100644
--- a/drivers/staging/rtl8723bs/core/rtw_ieee80211.c
+++ b/drivers/staging/rtl8723bs/core/rtw_ieee80211.c
@@ -186,20 +186,25 @@ u8 *rtw_get_ie_ex(u8 *in_ie, uint in_len, u8 eid, u8 *oui, u8 oui_len, u8 *ie, u
 
 	cnt = 0;
 
-	while (cnt < in_len) {
+	while (cnt + 2 <= in_len) {
+		u8 ie_len = in_ie[cnt + 1];
+
+		if (cnt + 2 + ie_len > in_len)
+			break;
+
 		if (eid == in_ie[cnt]
-			&& (!oui || !memcmp(&in_ie[cnt+2], oui, oui_len))) {
+			&& (!oui || (ie_len >= oui_len && !memcmp(&in_ie[cnt + 2], oui, oui_len)))) {
 			target_ie = &in_ie[cnt];
 
 			if (ie)
-				memcpy(ie, &in_ie[cnt], in_ie[cnt+1]+2);
+				memcpy(ie, &in_ie[cnt], ie_len + 2);
 
 			if (ielen)
-				*ielen = in_ie[cnt+1]+2;
+				*ielen = ie_len + 2;
 
 			break;
 		}
-		cnt += in_ie[cnt+1]+2; /* goto next */
+		cnt += ie_len + 2; /* goto next */
 	}
 
 	return target_ie;
-- 
2.53.0

Re: [PATCH] staging: rtl8723bs: properly validate the data in rtw_get_ie_ex()

[Navaneeth K]

I don't have the physical RTL8723BS hardware on hand anymore to test a 
live connection, but I was able to verify your logic thoroughly in 
user-space.
To be absolutely sure, I extracted both the old and patched functions 
into a standalone C harness and ran them through Asan and AFL++.
Feeding a crafted 5-byte allocation(with a lying length byte) to the old 
code predictably triggered a 47-byte heap-buffer-overflow right at the 
memcpy.
Against your patched code, I ran that same payload along with 20 other 
edge-case tests (1-byte buffers, empty bodies, OUI boundary mismatches, 
etc.). It cleanly rejected all of them with zero ASan errors.
I also compiled the patched function as an AFL++ target and let it 
freely mutate the EID, length, and body bytes. After over 100,000 
executions, it reported 0 crashes and 0 hangs.
The logic is definitely solid. Changing the loop guard to "while (cnt + 
2 <= in_len)" guarantees we always have at least 2 bytes before we touch 
the EID and length. Reading ie_len once and explicitly checking if it 
exceeds in_len completely stops the memcpy from reading past the end of 
the allocation.

I have the raw ASan logs, AFL stats, and the C test harnesses saved if 
needed!.

Tested-by: Navaneeth K <knavaneeth786@gmail.com>
Reviewed-by: Navaneeth K <knavaneeth786@gmail.com>

On 23-02-2026 19:01, Greg Kroah-Hartman wrote:
> Just like in commit 154828bf9559 ("staging: rtl8723bs: fix out-of-bounds
> read in rtw_get_ie() parser"), we don't trust the data in the frame so
> we should check the length better before acting on it
> 
> Cc: Navaneeth K <knavaneeth786@gmail.com>
> Cc: stable <stable@kernel.org>
> Assisted-by: gkh_clanker_2000
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> ---
> Navaneeth, any chance you can test this or at least verify my logic is
> correct here?  I got a "hit" from a tool that the work you did in your
> commit also needs to be done here, and I _think_ I got it right but do
> not have the hardware to test this with at all.  Thanks!
> 
>   drivers/staging/rtl8723bs/core/rtw_ieee80211.c | 15 ++++++++++-----
>   1 file changed, 10 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/staging/rtl8723bs/core/rtw_ieee80211.c b/drivers/staging/rtl8723bs/core/rtw_ieee80211.c
> index 6cf217e21593..3e2b5e6b07f9 100644
> --- a/drivers/staging/rtl8723bs/core/rtw_ieee80211.c
> +++ b/drivers/staging/rtl8723bs/core/rtw_ieee80211.c
> @@ -186,20 +186,25 @@ u8 *rtw_get_ie_ex(u8 *in_ie, uint in_len, u8 eid, u8 *oui, u8 oui_len, u8 *ie, u
>   
>   	cnt = 0;
>   
> -	while (cnt < in_len) {
> +	while (cnt + 2 <= in_len) {
> +		u8 ie_len = in_ie[cnt + 1];
> +
> +		if (cnt + 2 + ie_len > in_len)
> +			break;
> +
>   		if (eid == in_ie[cnt]
> -			&& (!oui || !memcmp(&in_ie[cnt+2], oui, oui_len))) {
> +			&& (!oui || (ie_len >= oui_len && !memcmp(&in_ie[cnt + 2], oui, oui_len)))) {
>   			target_ie = &in_ie[cnt];
>   
>   			if (ie)
> -				memcpy(ie, &in_ie[cnt], in_ie[cnt+1]+2);
> +				memcpy(ie, &in_ie[cnt], ie_len + 2);
>   
>   			if (ielen)
> -				*ielen = in_ie[cnt+1]+2;
> +				*ielen = ie_len + 2;
>   
>   			break;
>   		}
> -		cnt += in_ie[cnt+1]+2; /* goto next */
> +		cnt += ie_len + 2; /* goto next */
>   	}
>   
>   	return target_ie;

Re: [PATCH] staging: rtl8723bs: properly validate the data in rtw_get_ie_ex()

[Greg Kroah-Hartman]

On Mon, Feb 23, 2026 at 08:53:54PM +0530, Navaneeth K wrote:
> I don't have the physical RTL8723BS hardware on hand anymore to test a live
> connection, but I was able to verify your logic thoroughly in user-space.
> To be absolutely sure, I extracted both the old and patched functions into a
> standalone C harness and ran them through Asan and AFL++.
> Feeding a crafted 5-byte allocation(with a lying length byte) to the old
> code predictably triggered a 47-byte heap-buffer-overflow right at the
> memcpy.
> Against your patched code, I ran that same payload along with 20 other
> edge-case tests (1-byte buffers, empty bodies, OUI boundary mismatches,
> etc.). It cleanly rejected all of them with zero ASan errors.
> I also compiled the patched function as an AFL++ target and let it freely
> mutate the EID, length, and body bytes. After over 100,000 executions, it
> reported 0 crashes and 0 hangs.
> The logic is definitely solid. Changing the loop guard to "while (cnt + 2 <=
> in_len)" guarantees we always have at least 2 bytes before we touch the EID
> and length. Reading ie_len once and explicitly checking if it exceeds in_len
> completely stops the memcpy from reading past the end of the allocation.
> 
> I have the raw ASan logs, AFL stats, and the C test harnesses saved if
> needed!.
> 
> Tested-by: Navaneeth K <knavaneeth786@gmail.com>
> Reviewed-by: Navaneeth K <knavaneeth786@gmail.com>

Wow, that's way more than I expected, thank you!  Raw logs and test
harness is not needed, I'll trust you, worst case, we revert it if
someone complains it breaks their device :)

thanks,

greg k-h