[PATCH v4 0/5] staging: rtl8723bs: fix multiple security vulnerabilities

[PATCH v4 0/5] staging: rtl8723bs: fix multiple security vulnerabilities

[Delene Tchio Romuald]

This series fixes five remotely-triggerable memory safety issues in
the rtl8723bs driver. All of them are reachable from the air by an
attacker within WiFi radio range, without authentication, via
crafted management or data frames:

  1. Heap buffer overflow in recvframe_defrag() when reassembling
     fragmented frames whose total payload exceeds the receive
     buffer capacity.
  2. Integer underflow in TKIP MIC verification when a frame is
     shorter than the sum of header, IV, ICV and MIC sizes.
  3. Out-of-bounds read in portctrl() when a non-EAPOL frame is
     shorter than the 802.11 header + IV + LLC + ether_type.
  4. Out-of-bounds reads in three IE walkers (rtw_get_wapi_ie(),
     rtw_get_sec_ie(), rtw_get_wps_ie()) due to missing validation
     of the TLV length byte.
  5. Integer underflow in rtw_wep_decrypt() when a WEP frame is
     shorter than the header + IV.

Each patch was found by code review and is not tested on hardware.

Changes since v3:
 - Patch 1/5 (recvframe_defrag): check the return values of
   recvframe_pull() and recvframe_pull_tail(); on failure those
   helpers revert their pointer updates and return NULL, so the
   subsequent rx_end - rx_tail bounds check must not run on stale
   pointers (Dan Carpenter).
 - Patch 1/5: drop the unnecessary (uint) cast in the bounds
   check (Dan Carpenter).
 - All patches: add Fixes: tag pointing at the driver import and
   add the stable backport tag, per Dan Carpenter's request.
 - Patches 2-5: carry Reviewed-by: Luka Gejak. Patch 1/5 lost
   Luka's tag because the code changed.

Changes since v2:
 - Sent as numbered series with cover letter.
 - Cc list regenerated from scripts/get_maintainer.pl.

Changes since v1:
 - Rebased on staging-next (v1 was based on v7.0-rc6 and did not
   apply).

Delene Tchio Romuald (5):
  staging: rtl8723bs: fix heap buffer overflow in recvframe_defrag()
  staging: rtl8723bs: fix integer underflow in TKIP MIC verification
  staging: rtl8723bs: fix out-of-bounds read in portctrl()
  staging: rtl8723bs: fix out-of-bounds reads in IE parsing functions
  staging: rtl8723bs: fix negative length in WEP decryption

 .../staging/rtl8723bs/core/rtw_ieee80211.c    | 15 ++++-
 drivers/staging/rtl8723bs/core/rtw_recv.c     | 55 ++++++++++++++-----
 drivers/staging/rtl8723bs/core/rtw_security.c |  6 ++
 3 files changed, 60 insertions(+), 16 deletions(-)


base-commit: bf9c95f3eeefb7fc4b4a6380cc23f1dca744e379
--
2.43.0

[PATCH v4 1/5] staging: rtl8723bs: fix heap buffer overflow in recvframe_defrag()

[Delene Tchio Romuald]

In recvframe_defrag(), a memcpy() copies fragment data into the
reassembly buffer before validating that the buffer has sufficient
space. If the total reassembled payload exceeds the receive buffer
capacity, this results in a heap buffer overflow.

Additionally, the return values of recvframe_pull() and
recvframe_pull_tail() were ignored. On failure those helpers revert
their pointer updates and return NULL; continuing past such a
failure would leave pfhdr->rx_tail at its pre-strip value, so the
subsequent bounds check against rx_end - rx_tail would operate on
stale pointers.

An attacker within WiFi radio range can exploit this by sending
crafted 802.11 fragmented frames. No authentication is required.

Check the return values of recvframe_pull() and recvframe_pull_tail(),
then verify that the fragment payload fits within the remaining
buffer space before the memcpy().

Found by reviewing memory operations in the driver and tracing
buffer pointer manipulation through rtw_recv.h inline helpers.
Not tested on hardware.

Fixes: 554c0a3abf216 ("staging: Add rtl8723bs sdio wifi driver")
Cc: stable@vger.kernel.org
Signed-off-by: Delene Tchio Romuald <delenetchior1@gmail.com>
---
v4: check return values of recvframe_pull() and recvframe_pull_tail();
    drop unnecessary (uint) cast; add Fixes: tag and Cc: stable
    (Dan Carpenter). Luka Gejak's Reviewed-by dropped because the
    code changed.
v3: rebased on staging-next; sent as numbered series with proper
    Cc from get_maintainer.pl.
v2: rebased on staging-next (v1 was based on v7.0-rc6 and did not
    apply).

 drivers/staging/rtl8723bs/core/rtw_recv.c | 20 +++++++++++++++++---
 1 file changed, 17 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/rtl8723bs/core/rtw_recv.c b/drivers/staging/rtl8723bs/core/rtw_recv.c
index f78194d508dfc..a739c2bada2a1 100644
--- a/drivers/staging/rtl8723bs/core/rtw_recv.c
+++ b/drivers/staging/rtl8723bs/core/rtw_recv.c
@@ -1127,12 +1127,26 @@ static union recv_frame *recvframe_defrag(struct adapter *adapter,
 
 		wlanhdr_offset = pnfhdr->attrib.hdrlen + pnfhdr->attrib.iv_len;
 
-		recvframe_pull(pnextrframe, wlanhdr_offset);
+		if (!recvframe_pull(pnextrframe, wlanhdr_offset)) {
+			rtw_free_recvframe(prframe, pfree_recv_queue);
+			rtw_free_recvframe_queue(defrag_q, pfree_recv_queue);
+			return NULL;
+		}
 
 		/* append  to first fragment frame's tail (if privacy frame, pull the ICV) */
-		recvframe_pull_tail(prframe, pfhdr->attrib.icv_len);
+		if (!recvframe_pull_tail(prframe, pfhdr->attrib.icv_len)) {
+			rtw_free_recvframe(prframe, pfree_recv_queue);
+			rtw_free_recvframe_queue(defrag_q, pfree_recv_queue);
+			return NULL;
+		}
+
+		/* Verify the receiving buffer has enough space for the fragment */
+		if (pnfhdr->len > pfhdr->rx_end - pfhdr->rx_tail) {
+			rtw_free_recvframe(prframe, pfree_recv_queue);
+			rtw_free_recvframe_queue(defrag_q, pfree_recv_queue);
+			return NULL;
+		}
 
-		/* memcpy */
 		memcpy(pfhdr->rx_tail, pnfhdr->rx_data, pnfhdr->len);
 
 		recvframe_put(prframe, pnfhdr->len);
-- 
2.43.0

Re: [PATCH v4 1/5] staging: rtl8723bs: fix heap buffer overflow in recvframe_defrag()

[Dan Carpenter]

On Wed, Apr 15, 2026 at 07:54:57PM +0100, Delene Tchio Romuald wrote:
> In recvframe_defrag(), a memcpy() copies fragment data into the
> reassembly buffer before validating that the buffer has sufficient
> space. If the total reassembled payload exceeds the receive buffer
> capacity, this results in a heap buffer overflow.
> 
> Additionally, the return values of recvframe_pull() and
> recvframe_pull_tail() were ignored. On failure those helpers revert
> their pointer updates and return NULL; continuing past such a
> failure would leave pfhdr->rx_tail at its pre-strip value, so the
> subsequent bounds check against rx_end - rx_tail would operate on
> stale pointers.
> 
> An attacker within WiFi radio range can exploit this by sending
> crafted 802.11 fragmented frames. No authentication is required.
> 
> Check the return values of recvframe_pull() and recvframe_pull_tail(),
> then verify that the fragment payload fits within the remaining
> buffer space before the memcpy().
> 
> Found by reviewing memory operations in the driver and tracing
> buffer pointer manipulation through rtw_recv.h inline helpers.
> Not tested on hardware.
> 
> Fixes: 554c0a3abf216 ("staging: Add rtl8723bs sdio wifi driver")
> Cc: stable@vger.kernel.org
> Signed-off-by: Delene Tchio Romuald <delenetchior1@gmail.com>
> ---
> v4: check return values of recvframe_pull() and recvframe_pull_tail();
>     drop unnecessary (uint) cast; add Fixes: tag and Cc: stable
>     (Dan Carpenter). Luka Gejak's Reviewed-by dropped because the
>     code changed.
> v3: rebased on staging-next; sent as numbered series with proper
>     Cc from get_maintainer.pl.
> v2: rebased on staging-next (v1 was based on v7.0-rc6 and did not
>     apply).
> 
>  drivers/staging/rtl8723bs/core/rtw_recv.c | 20 +++++++++++++++++---
>  1 file changed, 17 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/staging/rtl8723bs/core/rtw_recv.c b/drivers/staging/rtl8723bs/core/rtw_recv.c
> index f78194d508dfc..a739c2bada2a1 100644
> --- a/drivers/staging/rtl8723bs/core/rtw_recv.c
> +++ b/drivers/staging/rtl8723bs/core/rtw_recv.c
> @@ -1127,12 +1127,26 @@ static union recv_frame *recvframe_defrag(struct adapter *adapter,
>  
>  		wlanhdr_offset = pnfhdr->attrib.hdrlen + pnfhdr->attrib.iv_len;
>  
> -		recvframe_pull(pnextrframe, wlanhdr_offset);
> +		if (!recvframe_pull(pnextrframe, wlanhdr_offset)) {
> +			rtw_free_recvframe(prframe, pfree_recv_queue);
> +			rtw_free_recvframe_queue(defrag_q, pfree_recv_queue);
> +			return NULL;

We have four blocks now that do exactly this...  Add a cleanup at the
end of the function:

	return pframe;

out_err:
	rtw_free_recvframe(prframe, pfree_recv_queue);
	rtw_free_recvframe_queue(defrag_q, pfree_recv_queue);
	return NULL;

Btw, I'm planning to review the other patches in the series so hold off a
day before resending.  If I haven't reviewed them by End Of Day on
Thursday, then I have gotten busy and feel free to resend at that point.

regards,
dan carpenter

[PATCH v4 2/5] staging: rtl8723bs: fix integer underflow in TKIP MIC verification

[Delene Tchio Romuald]

In recvframe_chkmic(), the payload length is computed as:

    datalen = precvframe->u.hdr.len - prxattrib->hdrlen
              - prxattrib->iv_len - prxattrib->icv_len - 8;

All operands are unsigned. If the receive frame is shorter than the
sum of the header, IV, ICV and MIC sizes, this subtraction wraps
around and datalen becomes a huge unsigned value. That value is then
passed to rtw_secmicappend(), which reads past the end of the
receive buffer and can leak kernel memory or trigger a crash.

An attacker within WiFi radio range can exploit this by sending a
crafted short TKIP-encrypted frame. No authentication is required.

Validate that the frame is large enough for the TKIP MIC
computation before the subtraction.

Found by reviewing length arithmetic in the TKIP receive path.
Not tested on hardware.

Fixes: 554c0a3abf216 ("staging: Add rtl8723bs sdio wifi driver")
Cc: stable@vger.kernel.org
Reviewed-by: Luka Gejak <luka.gejak@linux.dev>
Signed-off-by: Delene Tchio Romuald <delenetchior1@gmail.com>
---
v4: add Fixes: tag and Cc: stable (Dan Carpenter); carry Luka Gejak's
    Reviewed-by.
v3: rebased on staging-next; sent as numbered series with proper
    Cc from get_maintainer.pl.
v2: rebased on staging-next (v1 was based on v7.0-rc6 and did not
    apply).

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

diff --git a/drivers/staging/rtl8723bs/core/rtw_recv.c b/drivers/staging/rtl8723bs/core/rtw_recv.c
index a739c2bada2a1..00b69571bbb83 100644
--- a/drivers/staging/rtl8723bs/core/rtw_recv.c
+++ b/drivers/staging/rtl8723bs/core/rtw_recv.c
@@ -390,6 +390,13 @@ static signed int recvframe_chkmic(struct adapter *adapter,  union recv_frame *p
 				mickey = &stainfo->dot11tkiprxmickey.skey[0];
 			}
 
+			/* Ensure the frame is large enough for TKIP MIC verification */
+			if (precvframe->u.hdr.len <= prxattrib->hdrlen +
+			    prxattrib->iv_len + prxattrib->icv_len + 8) {
+				res = _FAIL;
+				goto exit;
+			}
+
 			datalen = precvframe->u.hdr.len - prxattrib->hdrlen - prxattrib->iv_len - prxattrib->icv_len - 8;/* icv_len included the mic code */
 			pframe = precvframe->u.hdr.rx_data;
 			payload = pframe + prxattrib->hdrlen + prxattrib->iv_len;
-- 
2.43.0

[PATCH v4 3/5] staging: rtl8723bs: fix out-of-bounds read in portctrl()

[Delene Tchio Romuald]

In portctrl(), when 802.1X port control is enabled and a non-EAPOL
frame is received, the ether_type is read from the LLC header
without verifying that the frame actually contains enough bytes to
hold the MAC header, IV and the LLC header plus two bytes of
ether_type. For sufficiently short frames, the memcpy() that loads
be_tmp reads past the end of the receive buffer.

An attacker within WiFi radio range can exploit this by sending a
crafted short frame. No authentication is required.

Validate the frame length before dereferencing the LLC header; drop
the frame if it is too short.

Found by reviewing length validation in the receive path.
Not tested on hardware.

Fixes: 554c0a3abf216 ("staging: Add rtl8723bs sdio wifi driver")
Cc: stable@vger.kernel.org
Reviewed-by: Luka Gejak <luka.gejak@linux.dev>
Signed-off-by: Delene Tchio Romuald <delenetchior1@gmail.com>
---
v4: add Fixes: tag and Cc: stable (Dan Carpenter); carry Luka Gejak's
    Reviewed-by.
v3: rebased on staging-next; sent as numbered series with proper
    Cc from get_maintainer.pl.
v2: rebased on staging-next (v1 was based on v7.0-rc6 and did not
    apply).

 drivers/staging/rtl8723bs/core/rtw_recv.c | 28 +++++++++++++++--------
 1 file changed, 18 insertions(+), 10 deletions(-)

diff --git a/drivers/staging/rtl8723bs/core/rtw_recv.c b/drivers/staging/rtl8723bs/core/rtw_recv.c
index 00b69571bbb83..c0a1c2ab710ee 100644
--- a/drivers/staging/rtl8723bs/core/rtw_recv.c
+++ b/drivers/staging/rtl8723bs/core/rtw_recv.c
@@ -539,17 +539,25 @@ static union recv_frame *portctrl(struct adapter *adapter, union recv_frame *pre
 
 			prtnframe = precv_frame;
 
-			/* get ether_type */
-			ptr = ptr + pfhdr->attrib.hdrlen + pfhdr->attrib.iv_len + LLC_HEADER_LENGTH;
-			memcpy(&be_tmp, ptr, 2);
-			ether_type = ntohs(be_tmp);
-
-			if (ether_type == eapol_type)
-				prtnframe = precv_frame;
-			else {
-				/* free this frame */
-				rtw_free_recvframe(precv_frame, &adapter->recvpriv.free_recv_queue);
+			/* Ensure frame has LLC header and ether_type */
+			if (pfhdr->len < pattrib->hdrlen +
+			    pattrib->iv_len + LLC_HEADER_LENGTH + 2) {
+				rtw_free_recvframe(precv_frame,
+						   &adapter->recvpriv.free_recv_queue);
 				prtnframe = NULL;
+			} else {
+				/* get ether_type */
+				ptr += pattrib->hdrlen +
+				       pattrib->iv_len +
+				       LLC_HEADER_LENGTH;
+				memcpy(&be_tmp, ptr, 2);
+				ether_type = ntohs(be_tmp);
+
+				if (ether_type != eapol_type) {
+					rtw_free_recvframe(precv_frame,
+							   &adapter->recvpriv.free_recv_queue);
+					prtnframe = NULL;
+				}
 			}
 		} else {
 			/* allowed */
-- 
2.43.0

Re: [PATCH v4 3/5] staging: rtl8723bs: fix out-of-bounds read in portctrl()

[Dan Carpenter]

On Wed, Apr 15, 2026 at 07:54:59PM +0100, Delene Tchio Romuald wrote:
> In portctrl(), when 802.1X port control is enabled and a non-EAPOL
> frame is received, the ether_type is read from the LLC header
> without verifying that the frame actually contains enough bytes to
> hold the MAC header, IV and the LLC header plus two bytes of
> ether_type. For sufficiently short frames, the memcpy() that loads
> be_tmp reads past the end of the receive buffer.
> 
> An attacker within WiFi radio range can exploit this by sending a
> crafted short frame. No authentication is required.
> 
> Validate the frame length before dereferencing the LLC header; drop
> the frame if it is too short.
> 
> Found by reviewing length validation in the receive path.
> Not tested on hardware.
> 
> Fixes: 554c0a3abf216 ("staging: Add rtl8723bs sdio wifi driver")
> Cc: stable@vger.kernel.org
> Reviewed-by: Luka Gejak <luka.gejak@linux.dev>
> Signed-off-by: Delene Tchio Romuald <delenetchior1@gmail.com>
> ---
> v4: add Fixes: tag and Cc: stable (Dan Carpenter); carry Luka Gejak's
>     Reviewed-by.
> v3: rebased on staging-next; sent as numbered series with proper
>     Cc from get_maintainer.pl.
> v2: rebased on staging-next (v1 was based on v7.0-rc6 and did not
>     apply).
> 
>  drivers/staging/rtl8723bs/core/rtw_recv.c | 28 +++++++++++++++--------
>  1 file changed, 18 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/staging/rtl8723bs/core/rtw_recv.c b/drivers/staging/rtl8723bs/core/rtw_recv.c
> index 00b69571bbb83..c0a1c2ab710ee 100644
> --- a/drivers/staging/rtl8723bs/core/rtw_recv.c
> +++ b/drivers/staging/rtl8723bs/core/rtw_recv.c
> @@ -539,17 +539,25 @@ static union recv_frame *portctrl(struct adapter *adapter, union recv_frame *pre
>  
>  			prtnframe = precv_frame;
>  
> -			/* get ether_type */
> -			ptr = ptr + pfhdr->attrib.hdrlen + pfhdr->attrib.iv_len + LLC_HEADER_LENGTH;
> -			memcpy(&be_tmp, ptr, 2);
> -			ether_type = ntohs(be_tmp);
> -
> -			if (ether_type == eapol_type)
> -				prtnframe = precv_frame;
> -			else {
> -				/* free this frame */
> -				rtw_free_recvframe(precv_frame, &adapter->recvpriv.free_recv_queue);
> +			/* Ensure frame has LLC header and ether_type */
> +			if (pfhdr->len < pattrib->hdrlen +
> +			    pattrib->iv_len + LLC_HEADER_LENGTH + 2) {
> +				rtw_free_recvframe(precv_frame,
> +						   &adapter->recvpriv.free_recv_queue);
>  				prtnframe = NULL;

I feel like it's sort of weird to write this as a pfhdr->len < condition.
I feel like the untrusted part of the condition is the pattrib->hdrlen
stuff and normally you would put the untrusted parts on the left.  I
kind of see what you're saying that the packet is too small, but to me
I see it as the hdrlen is too big...  But, also since you found the bug
then you get to choose the style on this, so do which ever way you feel
is best.

It would be better if instead of setting "prtnframe = NULL;" here,
you just did "return NULL;" instead.  You've followed the pattern of
the existing code, but the rule is that if the function has a 100 lines
of bad style code, you should add 1 line of good style even if it's
inconsistent.

It makes the code slightly better and it makes the diff a lot smaller
and clearer.

regards,
dan carpenter

diff --git a/drivers/staging/rtl8723bs/core/rtw_recv.c b/drivers/staging/rtl8723bs/core/rtw_recv.c
index f78194d508df..9cedca1bd83a 100644
--- a/drivers/staging/rtl8723bs/core/rtw_recv.c
+++ b/drivers/staging/rtl8723bs/core/rtw_recv.c
@@ -531,6 +531,13 @@ static union recv_frame *portctrl(struct adapter *adapter, union recv_frame *pre
 			/* only accept EAPOL frame */
 
 			prtnframe = precv_frame;
+			/* Ensure frame has LLC header and ether_type */
+			if (pfhdr->len < pattrib->hdrlen +
+			    pattrib->iv_len + LLC_HEADER_LENGTH + 2) {
+				rtw_free_recvframe(precv_frame,
+						   &adapter->recvpriv.free_recv_queue);
+				return NULL;
+			}
 
 			/* get ether_type */
 			ptr = ptr + pfhdr->attrib.hdrlen + pfhdr->attrib.iv_len + LLC_HEADER_LENGTH;

Re: [PATCH v4 3/5] staging: rtl8723bs: fix out-of-bounds read in portctrl()

[Luka Gejak]

On Thu Apr 16, 2026 at 6:36 PM CEST, Dan Carpenter wrote:
> On Wed, Apr 15, 2026 at 07:54:59PM +0100, Delene Tchio Romuald wrote:
>> In portctrl(), when 802.1X port control is enabled and a non-EAPOL
>> frame is received, the ether_type is read from the LLC header
>> without verifying that the frame actually contains enough bytes to
>> hold the MAC header, IV and the LLC header plus two bytes of
>> ether_type. For sufficiently short frames, the memcpy() that loads
>> be_tmp reads past the end of the receive buffer.
>> 
>> An attacker within WiFi radio range can exploit this by sending a
>> crafted short frame. No authentication is required.
>> 
>> Validate the frame length before dereferencing the LLC header; drop
>> the frame if it is too short.
>> 
>> Found by reviewing length validation in the receive path.
>> Not tested on hardware.
>> 
>> Fixes: 554c0a3abf216 ("staging: Add rtl8723bs sdio wifi driver")
>> Cc: stable@vger.kernel.org
>> Reviewed-by: Luka Gejak <luka.gejak@linux.dev>
>> Signed-off-by: Delene Tchio Romuald <delenetchior1@gmail.com>
>> ---
>> v4: add Fixes: tag and Cc: stable (Dan Carpenter); carry Luka Gejak's
>>     Reviewed-by.
>> v3: rebased on staging-next; sent as numbered series with proper
>>     Cc from get_maintainer.pl.
>> v2: rebased on staging-next (v1 was based on v7.0-rc6 and did not
>>     apply).
>> 
>>  drivers/staging/rtl8723bs/core/rtw_recv.c | 28 +++++++++++++++--------
>>  1 file changed, 18 insertions(+), 10 deletions(-)
>> 
>> diff --git a/drivers/staging/rtl8723bs/core/rtw_recv.c b/drivers/staging/rtl8723bs/core/rtw_recv.c
>> index 00b69571bbb83..c0a1c2ab710ee 100644
>> --- a/drivers/staging/rtl8723bs/core/rtw_recv.c
>> +++ b/drivers/staging/rtl8723bs/core/rtw_recv.c
>> @@ -539,17 +539,25 @@ static union recv_frame *portctrl(struct adapter *adapter, union recv_frame *pre
>>  
>>  			prtnframe = precv_frame;
>>  
>> -			/* get ether_type */
>> -			ptr = ptr + pfhdr->attrib.hdrlen + pfhdr->attrib.iv_len + LLC_HEADER_LENGTH;
>> -			memcpy(&be_tmp, ptr, 2);
>> -			ether_type = ntohs(be_tmp);
>> -
>> -			if (ether_type == eapol_type)
>> -				prtnframe = precv_frame;
>> -			else {
>> -				/* free this frame */
>> -				rtw_free_recvframe(precv_frame, &adapter->recvpriv.free_recv_queue);
>> +			/* Ensure frame has LLC header and ether_type */
>> +			if (pfhdr->len < pattrib->hdrlen +
>> +			    pattrib->iv_len + LLC_HEADER_LENGTH + 2) {
>> +				rtw_free_recvframe(precv_frame,
>> +						   &adapter->recvpriv.free_recv_queue);
>>  				prtnframe = NULL;
>
> I feel like it's sort of weird to write this as a pfhdr->len < condition.
> I feel like the untrusted part of the condition is the pattrib->hdrlen
> stuff and normally you would put the untrusted parts on the left.  I
> kind of see what you're saying that the packet is too small, but to me
> I see it as the hdrlen is too big...  But, also since you found the bug
> then you get to choose the style on this, so do which ever way you feel
> is best.
>
> It would be better if instead of setting "prtnframe = NULL;" here,
> you just did "return NULL;" instead.  You've followed the pattern of
> the existing code, but the rule is that if the function has a 100 lines
> of bad style code, you should add 1 line of good style even if it's
> inconsistent.
Hi Dan,
I wasn't aware of that. I thought in such cases you should follow the 
pattern of existing code. I will make sure to note that in future 
reviews.
Best regards,
Luka Gejak
>
> It makes the code slightly better and it makes the diff a lot smaller
> and clearer.
>
> regards,
> dan carpenter
>
> diff --git a/drivers/staging/rtl8723bs/core/rtw_recv.c b/drivers/staging/rtl8723bs/core/rtw_recv.c
> index f78194d508df..9cedca1bd83a 100644
> --- a/drivers/staging/rtl8723bs/core/rtw_recv.c
> +++ b/drivers/staging/rtl8723bs/core/rtw_recv.c
> @@ -531,6 +531,13 @@ static union recv_frame *portctrl(struct adapter *adapter, union recv_frame *pre
>  			/* only accept EAPOL frame */
>  
>  			prtnframe = precv_frame;
> +			/* Ensure frame has LLC header and ether_type */
> +			if (pfhdr->len < pattrib->hdrlen +
> +			    pattrib->iv_len + LLC_HEADER_LENGTH + 2) {
> +				rtw_free_recvframe(precv_frame,
> +						   &adapter->recvpriv.free_recv_queue);
> +				return NULL;
> +			}
>  
>  			/* get ether_type */
>  			ptr = ptr + pfhdr->attrib.hdrlen + pfhdr->attrib.iv_len + LLC_HEADER_LENGTH;

[PATCH v4 4/5] staging: rtl8723bs: fix out-of-bounds reads in IE parsing functions

[Delene Tchio Romuald]

rtw_get_wapi_ie(), rtw_get_sec_ie() and rtw_get_wps_ie() walk a
buffer of Information Elements using the TLV length field without
first verifying that the length byte itself is inside the buffer,
and without verifying that the element's declared length fits
inside the remaining buffer. Both conditions can be reached with
crafted input, causing reads past the end of the buffer.

An attacker within WiFi radio range can exploit this by sending
crafted beacon or probe-response frames carrying truncated or
oversized IEs. No authentication is required.

Ensure the length byte is inside the buffer (cnt + 1 < in_len)
and break out of the loop if the declared element length would
read past in_len.

Found by reviewing bounds checks in IE walkers.
Not tested on hardware.

Fixes: 554c0a3abf216 ("staging: Add rtl8723bs sdio wifi driver")
Cc: stable@vger.kernel.org
Reviewed-by: Luka Gejak <luka.gejak@linux.dev>
Signed-off-by: Delene Tchio Romuald <delenetchior1@gmail.com>
---
v4: add Fixes: tag and Cc: stable (Dan Carpenter); carry Luka Gejak's
    Reviewed-by.
v3: rebased on staging-next; sent as numbered series with proper
    Cc from get_maintainer.pl.
v2: rebased on staging-next (v1 was based on v7.0-rc6 and did not
    apply).

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

diff --git a/drivers/staging/rtl8723bs/core/rtw_ieee80211.c b/drivers/staging/rtl8723bs/core/rtw_ieee80211.c
index 72b7f731dd471..e0fed3f42de0c 100644
--- a/drivers/staging/rtl8723bs/core/rtw_ieee80211.c
+++ b/drivers/staging/rtl8723bs/core/rtw_ieee80211.c
@@ -582,9 +582,12 @@ int rtw_get_wapi_ie(u8 *in_ie, uint in_len, u8 *wapi_ie, u16 *wapi_len)
 
 	cnt = (_TIMESTAMP_ + _BEACON_ITERVAL_ + _CAPABILITY_);
 
-	while (cnt < in_len) {
+	while (cnt + 1 < in_len) {
 		authmode = in_ie[cnt];
 
+		if (cnt + 2 + in_ie[cnt + 1] > in_len)
+			break;
+
 		if (authmode == WLAN_EID_BSS_AC_ACCESS_DELAY &&
 		    (!memcmp(&in_ie[cnt + 6], wapi_oui1, 4) ||
 		     !memcmp(&in_ie[cnt + 6], wapi_oui2, 4))) {
@@ -615,9 +618,12 @@ void rtw_get_sec_ie(u8 *in_ie, uint in_len, u8 *rsn_ie, u16 *rsn_len, u8 *wpa_ie
 
 	cnt = (_TIMESTAMP_ + _BEACON_ITERVAL_ + _CAPABILITY_);
 
-	while (cnt < in_len) {
+	while (cnt + 1 < in_len) {
 		authmode = in_ie[cnt];
 
+		if (cnt + 2 + in_ie[cnt + 1] > in_len)
+			break;
+
 		if ((authmode == WLAN_EID_VENDOR_SPECIFIC) &&
 		    (!memcmp(&in_ie[cnt + 2], &wpa_oui[0], 4))) {
 			if (wpa_ie)
@@ -658,9 +664,12 @@ u8 *rtw_get_wps_ie(u8 *in_ie, uint in_len, u8 *wps_ie, uint *wps_ielen)
 
 	cnt = 0;
 
-	while (cnt < in_len) {
+	while (cnt + 1 < in_len) {
 		eid = in_ie[cnt];
 
+		if (cnt + 2 + in_ie[cnt + 1] > in_len)
+			break;
+
 		if ((eid == WLAN_EID_VENDOR_SPECIFIC) && (!memcmp(&in_ie[cnt + 2], wps_oui, 4))) {
 			wpsie_ptr = &in_ie[cnt];
 
-- 
2.43.0

Re: [PATCH v4 4/5] staging: rtl8723bs: fix out-of-bounds reads in IE parsing functions

[Dan Carpenter]

On Wed, Apr 15, 2026 at 07:55:00PM +0100, Delene Tchio Romuald wrote:
> rtw_get_wapi_ie(), rtw_get_sec_ie() and rtw_get_wps_ie() walk a
> buffer of Information Elements using the TLV length field without
> first verifying that the length byte itself is inside the buffer,
> and without verifying that the element's declared length fits
> inside the remaining buffer. Both conditions can be reached with
> crafted input, causing reads past the end of the buffer.
> 
> An attacker within WiFi radio range can exploit this by sending
> crafted beacon or probe-response frames carrying truncated or
> oversized IEs. No authentication is required.
> 
> Ensure the length byte is inside the buffer (cnt + 1 < in_len)
> and break out of the loop if the declared element length would
> read past in_len.
> 
> Found by reviewing bounds checks in IE walkers.
> Not tested on hardware.
> 
> Fixes: 554c0a3abf216 ("staging: Add rtl8723bs sdio wifi driver")
> Cc: stable@vger.kernel.org
> Reviewed-by: Luka Gejak <luka.gejak@linux.dev>
> Signed-off-by: Delene Tchio Romuald <delenetchior1@gmail.com>
> ---
> v4: add Fixes: tag and Cc: stable (Dan Carpenter); carry Luka Gejak's
>     Reviewed-by.
> v3: rebased on staging-next; sent as numbered series with proper
>     Cc from get_maintainer.pl.
> v2: rebased on staging-next (v1 was based on v7.0-rc6 and did not
>     apply).
> 
>  drivers/staging/rtl8723bs/core/rtw_ieee80211.c | 15 ++++++++++++---
>  1 file changed, 12 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/staging/rtl8723bs/core/rtw_ieee80211.c b/drivers/staging/rtl8723bs/core/rtw_ieee80211.c
> index 72b7f731dd471..e0fed3f42de0c 100644
> --- a/drivers/staging/rtl8723bs/core/rtw_ieee80211.c
> +++ b/drivers/staging/rtl8723bs/core/rtw_ieee80211.c
> @@ -582,9 +582,12 @@ int rtw_get_wapi_ie(u8 *in_ie, uint in_len, u8 *wapi_ie, u16 *wapi_len)
>  
>  	cnt = (_TIMESTAMP_ + _BEACON_ITERVAL_ + _CAPABILITY_);
>  
> -	while (cnt < in_len) {
> +	while (cnt + 1 < in_len) {
>  		authmode = in_ie[cnt];
>  
> +		if (cnt + 2 + in_ie[cnt + 1] > in_len)
> +			break;

It's a pity this function doesn't return negative error codes.

> +
>  		if (authmode == WLAN_EID_BSS_AC_ACCESS_DELAY &&
>  		    (!memcmp(&in_ie[cnt + 6], wapi_oui1, 4) ||
>  		     !memcmp(&in_ie[cnt + 6], wapi_oui2, 4))) {
                             ^^^^^^^^^^^^^^
here we are assuming the in_len is at least "cnt + 6 + 4" so we need
something like:

		if (cnt + 2 + in_ie[cnt + 1] > in_len)
			break;

		if (authmode == WLAN_EID_BSS_AC_ACCESS_DELAY) {
			if (cnt + 10 > in_len)
				break;
			if (!memcmp(&in_ie[cnt + 6], wapi_oui1, 4) || ...

> @@ -615,9 +618,12 @@ void rtw_get_sec_ie(u8 *in_ie, uint in_len, u8 *rsn_ie, u16 *rsn_len, u8 *wpa_ie
>  
>  	cnt = (_TIMESTAMP_ + _BEACON_ITERVAL_ + _CAPABILITY_);
>  
> -	while (cnt < in_len) {
> +	while (cnt + 1 < in_len) {
>  		authmode = in_ie[cnt];
>  
> +		if (cnt + 2 + in_ie[cnt + 1] > in_len)
> +			break;
> +
>  		if ((authmode == WLAN_EID_VENDOR_SPECIFIC) &&
>  		    (!memcmp(&in_ie[cnt + 2], &wpa_oui[0], 4))) {

Same in the other places as well.

regards,
dan carpenter

[PATCH v4 5/5] staging: rtl8723bs: fix negative length in WEP decryption

[Delene Tchio Romuald]

In rtw_wep_decrypt(), the payload length is computed as:

    length = frame->len - prxattrib->hdrlen - prxattrib->iv_len;

All operands are unsigned. If the frame is shorter than the sum of
the header length and the IV length, this subtraction wraps around
and length becomes a huge unsigned value. That value is then used
to drive an arc4_crypt() call that reads and writes past the end
of the receive buffer.

An attacker within WiFi radio range can exploit this by sending a
crafted short WEP-encrypted frame. No authentication is required.

Validate that the frame is large enough to contain a WEP payload
before computing length.

Found by reviewing length arithmetic in the WEP decrypt path.
Not tested on hardware.

Fixes: 554c0a3abf216 ("staging: Add rtl8723bs sdio wifi driver")
Cc: stable@vger.kernel.org
Reviewed-by: Luka Gejak <luka.gejak@linux.dev>
Signed-off-by: Delene Tchio Romuald <delenetchior1@gmail.com>
---
v4: add Fixes: tag and Cc: stable (Dan Carpenter); carry Luka Gejak's
    Reviewed-by.
v3: rebased on staging-next; sent as numbered series with proper
    Cc from get_maintainer.pl.
v2: rebased on staging-next (v1 was based on v7.0-rc6 and did not
    apply).

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

diff --git a/drivers/staging/rtl8723bs/core/rtw_security.c b/drivers/staging/rtl8723bs/core/rtw_security.c
index a00504ff29109..f3bc2240749a4 100644
--- a/drivers/staging/rtl8723bs/core/rtw_security.c
+++ b/drivers/staging/rtl8723bs/core/rtw_security.c
@@ -113,6 +113,12 @@ void rtw_wep_decrypt(struct adapter  *padapter, u8 *precvframe)
 		memcpy(&wepkey[0], iv, 3);
 		/* memcpy(&wepkey[3], &psecuritypriv->dot11DefKey[psecuritypriv->dot11PrivacyKeyIndex].skey[0], keylength); */
 		memcpy(&wepkey[3], &psecuritypriv->dot11DefKey[keyindex].skey[0], keylength);
+
+		/* Ensure the frame is long enough for WEP decryption */
+		if (((union recv_frame *)precvframe)->u.hdr.len <=
+		    prxattrib->hdrlen + prxattrib->iv_len)
+			return;
+
 		length = ((union recv_frame *)precvframe)->u.hdr.len - prxattrib->hdrlen - prxattrib->iv_len;
 
 		payload = pframe + prxattrib->iv_len + prxattrib->hdrlen;
-- 
2.43.0

Re: [PATCH v4 5/5] staging: rtl8723bs: fix negative length in WEP decryption

[Dan Carpenter]

On Wed, Apr 15, 2026 at 07:55:01PM +0100, Delene Tchio Romuald wrote:
> In rtw_wep_decrypt(), the payload length is computed as:
> 
>     length = frame->len - prxattrib->hdrlen - prxattrib->iv_len;
> 
> All operands are unsigned. If the frame is shorter than the sum of
> the header length and the IV length, this subtraction wraps around
> and length becomes a huge unsigned value. That value is then used
> to drive an arc4_crypt() call that reads and writes past the end
> of the receive buffer.
> 
> An attacker within WiFi radio range can exploit this by sending a
> crafted short WEP-encrypted frame. No authentication is required.
> 
> Validate that the frame is large enough to contain a WEP payload
> before computing length.
> 
> Found by reviewing length arithmetic in the WEP decrypt path.
> Not tested on hardware.
> 
> Fixes: 554c0a3abf216 ("staging: Add rtl8723bs sdio wifi driver")
> Cc: stable@vger.kernel.org
> Reviewed-by: Luka Gejak <luka.gejak@linux.dev>
> Signed-off-by: Delene Tchio Romuald <delenetchior1@gmail.com>
> ---
> v4: add Fixes: tag and Cc: stable (Dan Carpenter); carry Luka Gejak's
>     Reviewed-by.
> v3: rebased on staging-next; sent as numbered series with proper
>     Cc from get_maintainer.pl.
> v2: rebased on staging-next (v1 was based on v7.0-rc6 and did not
>     apply).
> 
>  drivers/staging/rtl8723bs/core/rtw_security.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/staging/rtl8723bs/core/rtw_security.c b/drivers/staging/rtl8723bs/core/rtw_security.c
> index a00504ff29109..f3bc2240749a4 100644
> --- a/drivers/staging/rtl8723bs/core/rtw_security.c
> +++ b/drivers/staging/rtl8723bs/core/rtw_security.c
> @@ -113,6 +113,12 @@ void rtw_wep_decrypt(struct adapter  *padapter, u8 *precvframe)
>  		memcpy(&wepkey[0], iv, 3);
>  		/* memcpy(&wepkey[3], &psecuritypriv->dot11DefKey[psecuritypriv->dot11PrivacyKeyIndex].skey[0], keylength); */
>  		memcpy(&wepkey[3], &psecuritypriv->dot11DefKey[keyindex].skey[0], keylength);
> +
> +		/* Ensure the frame is long enough for WEP decryption */
> +		if (((union recv_frame *)precvframe)->u.hdr.len <=
> +		    prxattrib->hdrlen + prxattrib->iv_len)
> +			return;

LGTM.  Thanks!

regards,
dan carpenter