[v2] staging: rtl8723bs: fix missing frame length checks in OnAuthClient

[PATCH v2] staging: rtl8723bs: fix missing frame length checks in OnAuthClient

Posted by Alexandru Hossu 2 months ago

OnAuthClient() accesses pframe without first verifying that pkt_len is
large enough to contain a valid 802.11 management frame header:

- get_da(pframe) reads bytes 4-9, requiring pkt_len >= 10
- GetPrivacy(pframe) reads the FC field at bytes 0-1

Additionally, when pkt_len < WLAN_HDR_A3_LEN + _AUTH_IE_OFFSET_ the
unsigned subtraction passed to rtw_get_ie() wraps around, causing it
to scan well past the end of the buffer.

Add an early check against WLAN_HDR_A3_LEN before any pframe access,
and a second check against WLAN_HDR_A3_LEN + offset + 6 after computing
offset to guard the seq/status reads and the rtw_get_ie() call.

Suggested-by: Dan Carpenter <error27@gmail.com>
Fixes: 554c0a3abf21 ("staging: Add rtl8723bs sdio wifi driver")
Cc: stable@vger.kernel.org
Signed-off-by: Alexandru Hossu <hossu.alexandru@gmail.com>
---
Changes in v2:
- Replace incorrect Reported-by tag with Suggested-by: Dan spotted the
  missing length check during code review of the heap overflow fix; he
  did not file a separate bug report
- Add missing version changelog (the initial submission was incorrectly
  labeled v2; no v1 was ever sent to the list)

 drivers/staging/rtl8723bs/core/rtw_mlme_ext.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/staging/rtl8723bs/core/rtw_mlme_ext.c b/drivers/staging/rtl8723bs/core/rtw_mlme_ext.c
index 90f27665667a..884cd39ec756 100644
--- a/drivers/staging/rtl8723bs/core/rtw_mlme_ext.c
+++ b/drivers/staging/rtl8723bs/core/rtw_mlme_ext.c
@@ -860,6 +860,9 @@ unsigned int OnAuthClient(struct adapter *padapter, union recv_frame *precv_fram
 	u8 *pframe = precv_frame->u.hdr.rx_data;
 	uint pkt_len = precv_frame->u.hdr.len;
 
+	if (pkt_len < WLAN_HDR_A3_LEN)
+		goto authclnt_fail;
+
 	/* check A1 matches or not */
 	if (memcmp(myid(&(padapter->eeprompriv)), get_da(pframe), ETH_ALEN))
 		return _SUCCESS;
@@ -869,6 +872,9 @@ unsigned int OnAuthClient(struct adapter *padapter, union recv_frame *precv_fram
 
 	offset = (GetPrivacy(pframe)) ? 4 : 0;
 
+	if (pkt_len < WLAN_HDR_A3_LEN + offset + 6)
+		goto authclnt_fail;
+
 	seq	= le16_to_cpu(*(__le16 *)((SIZE_PTR)pframe + WLAN_HDR_A3_LEN + offset + 2));
 	status	= le16_to_cpu(*(__le16 *)((SIZE_PTR)pframe + WLAN_HDR_A3_LEN + offset + 4));
 
-- 
2.53.0

Re: [PATCH v2] staging: rtl8723bs: fix missing frame length checks in OnAuthClient

Posted by Dan Carpenter 2 months ago

On Tue, Apr 14, 2026 at 11:39:59PM +0200, Alexandru Hossu wrote:
> OnAuthClient() accesses pframe without first verifying that pkt_len is
> large enough to contain a valid 802.11 management frame header:
> 
> - get_da(pframe) reads bytes 4-9, requiring pkt_len >= 10
> - GetPrivacy(pframe) reads the FC field at bytes 0-1
> 
> Additionally, when pkt_len < WLAN_HDR_A3_LEN + _AUTH_IE_OFFSET_ the
> unsigned subtraction passed to rtw_get_ie() wraps around, causing it
> to scan well past the end of the buffer.
> 
> Add an early check against WLAN_HDR_A3_LEN before any pframe access,
> and a second check against WLAN_HDR_A3_LEN + offset + 6 after computing
> offset to guard the seq/status reads and the rtw_get_ie() call.
> 
> Suggested-by: Dan Carpenter <error27@gmail.com>
> Fixes: 554c0a3abf21 ("staging: Add rtl8723bs sdio wifi driver")
> Cc: stable@vger.kernel.org
> Signed-off-by: Alexandru Hossu <hossu.alexandru@gmail.com>
> ---

Looks good!

Reviewed-by: Dan Carpenter <error27@gmail.com>

regards,
dan carpenter

Re: [PATCH v2] staging: rtl8723bs: fix missing frame length checks in OnAuthClient

Posted by Greg KH 2 months ago

On Tue, Apr 14, 2026 at 11:39:59PM +0200, Alexandru Hossu wrote:
> OnAuthClient() accesses pframe without first verifying that pkt_len is
> large enough to contain a valid 802.11 management frame header:
> 
> - get_da(pframe) reads bytes 4-9, requiring pkt_len >= 10
> - GetPrivacy(pframe) reads the FC field at bytes 0-1
> 
> Additionally, when pkt_len < WLAN_HDR_A3_LEN + _AUTH_IE_OFFSET_ the
> unsigned subtraction passed to rtw_get_ie() wraps around, causing it
> to scan well past the end of the buffer.
> 
> Add an early check against WLAN_HDR_A3_LEN before any pframe access,
> and a second check against WLAN_HDR_A3_LEN + offset + 6 after computing
> offset to guard the seq/status reads and the rtw_get_ie() call.
> 
> Suggested-by: Dan Carpenter <error27@gmail.com>
> Fixes: 554c0a3abf21 ("staging: Add rtl8723bs sdio wifi driver")
> Cc: stable@vger.kernel.org
> Signed-off-by: Alexandru Hossu <hossu.alexandru@gmail.com>
> ---
> Changes in v2:
> - Replace incorrect Reported-by tag with Suggested-by: Dan spotted the
>   missing length check during code review of the heap overflow fix; he
>   did not file a separate bug report
> - Add missing version changelog (the initial submission was incorrectly
>   labeled v2; no v1 was ever sent to the list)

So this is really v3?

And based on the thread, there was a v1, this one just replaced that.

If not, then I'm totally confused, please resend ALL pending patches for
this driver that you have as a patch series, properly versioned.

thanks,

greg k-h

Re: [PATCH v2] staging: rtl8723bs: fix missing frame length checks in OnAuthClient

Posted by Luka Gejak 2 months ago

Re: [PATCH v2] staging: rtl8723bs: fix missing frame length checks in OnAuthClient

Posted by Luka Gejak 2 months ago

On Wed Apr 15, 2026 at 7:18 AM CEST, Luka Gejak wrote:
>

Ignore this email, aerc(my email client) is having quirks.
Best regards,
Luka Gejak

Re: [PATCH v2] staging: rtl8723bs: fix missing frame length checks in OnAuthClient

Posted by Luka Gejak 2 months ago

On Wed Apr 15, 2026 at 6:56 AM CEST, Greg KH wrote:
> On Tue, Apr 14, 2026 at 11:39:59PM +0200, Alexandru Hossu wrote:
>> OnAuthClient() accesses pframe without first verifying that pkt_len is
>> large enough to contain a valid 802.11 management frame header:
>> 
>> - get_da(pframe) reads bytes 4-9, requiring pkt_len >= 10
>> - GetPrivacy(pframe) reads the FC field at bytes 0-1
>> 
>> Additionally, when pkt_len < WLAN_HDR_A3_LEN + _AUTH_IE_OFFSET_ the
>> unsigned subtraction passed to rtw_get_ie() wraps around, causing it
>> to scan well past the end of the buffer.
>> 
>> Add an early check against WLAN_HDR_A3_LEN before any pframe access,
>> and a second check against WLAN_HDR_A3_LEN + offset + 6 after computing
>> offset to guard the seq/status reads and the rtw_get_ie() call.
>> 
>> Suggested-by: Dan Carpenter <error27@gmail.com>
>> Fixes: 554c0a3abf21 ("staging: Add rtl8723bs sdio wifi driver")
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Alexandru Hossu <hossu.alexandru@gmail.com>
>> ---
>> Changes in v2:
>> - Replace incorrect Reported-by tag with Suggested-by: Dan spotted the
>>   missing length check during code review of the heap overflow fix; he
>>   did not file a separate bug report
>> - Add missing version changelog (the initial submission was incorrectly
>>   labeled v2; no v1 was ever sent to the list)
>
> So this is really v3?
>

...

This would actually be v4.
v1: [1]
v2: [2]
v3: [3]
v4: [4]
Best regards,
Luka Gejak

[1]: https://lore.kernel.org/linux-staging/20260413202824.740653-1-hossu.alexandru@gmail.com/
[2]: https://lore.kernel.org/linux-staging/20260414100804.871764-1-hossu.alexandru@gmail.com/
[3]: https://lore.kernel.org/linux-staging/20260414145350.903996-1-hossu.alexandru@gmail.com/
[4]: https://lore.kernel.org/linux-staging/20260414213959.1028301-1-hossu.alexandru@gmail.com/

Re: [PATCH v2] staging: rtl8723bs: fix missing frame length checks in OnAuthClient

Posted by Luka Gejak 2 months ago

On Tue Apr 14, 2026 at 11:39 PM CEST, Alexandru Hossu wrote:
> OnAuthClient() accesses pframe without first verifying that pkt_len is
> large enough to contain a valid 802.11 management frame header:
>
> - get_da(pframe) reads bytes 4-9, requiring pkt_len >= 10
> - GetPrivacy(pframe) reads the FC field at bytes 0-1
>
> Additionally, when pkt_len < WLAN_HDR_A3_LEN + _AUTH_IE_OFFSET_ the
> unsigned subtraction passed to rtw_get_ie() wraps around, causing it
> to scan well past the end of the buffer.
>
> Add an early check against WLAN_HDR_A3_LEN before any pframe access,
> and a second check against WLAN_HDR_A3_LEN + offset + 6 after computing
> offset to guard the seq/status reads and the rtw_get_ie() call.
>
> Suggested-by: Dan Carpenter <error27@gmail.com>
> Fixes: 554c0a3abf21 ("staging: Add rtl8723bs sdio wifi driver")
> Cc: stable@vger.kernel.org
> Signed-off-by: Alexandru Hossu <hossu.alexandru@gmail.com>
> ---
> Changes in v2:
> - Replace incorrect Reported-by tag with Suggested-by: Dan spotted the
>   missing length check during code review of the heap overflow fix; he
>   did not file a separate bug report
> - Add missing version changelog (the initial submission was incorrectly
>   labeled v2; no v1 was ever sent to the list)
>
>  drivers/staging/rtl8723bs/core/rtw_mlme_ext.c | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/drivers/staging/rtl8723bs/core/rtw_mlme_ext.c b/drivers/staging/rtl8723bs/core/rtw_mlme_ext.c
> index 90f27665667a..884cd39ec756 100644
> --- a/drivers/staging/rtl8723bs/core/rtw_mlme_ext.c
> +++ b/drivers/staging/rtl8723bs/core/rtw_mlme_ext.c
> @@ -860,6 +860,9 @@ unsigned int OnAuthClient(struct adapter *padapter, union recv_frame *precv_fram
>  	u8 *pframe = precv_frame->u.hdr.rx_data;
>  	uint pkt_len = precv_frame->u.hdr.len;
>  
> +	if (pkt_len < WLAN_HDR_A3_LEN)
> +		goto authclnt_fail;
> +
>  	/* check A1 matches or not */
>  	if (memcmp(myid(&(padapter->eeprompriv)), get_da(pframe), ETH_ALEN))
>  		return _SUCCESS;
> @@ -869,6 +872,9 @@ unsigned int OnAuthClient(struct adapter *padapter, union recv_frame *precv_fram
>  
>  	offset = (GetPrivacy(pframe)) ? 4 : 0;
>  
> +	if (pkt_len < WLAN_HDR_A3_LEN + offset + 6)
> +		goto authclnt_fail;
> +
>  	seq	= le16_to_cpu(*(__le16 *)((SIZE_PTR)pframe + WLAN_HDR_A3_LEN + offset + 2));
>  	status	= le16_to_cpu(*(__le16 *)((SIZE_PTR)pframe + WLAN_HDR_A3_LEN + offset + 4));
>  

LGTM.
Reviewed-by: Luka Gejak <luka.gejak@linux.dev>
Best regards,
Luka Gejak