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@xxxxxxxxx>
Reviewed-by: Navaneeth K <knavaneeth786@xxxxxxxxx>

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@xxxxxxxxx>
> Cc: stable <stable@xxxxxxxxxx>
> Assisted-by: gkh_clanker_2000
> Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
> ---
> 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;