[PATCH] staging: rtl8723bs: fix heap buffer overflow in recvframe_defrag()

[PATCH] 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 recvframe_put() validates that the buffer
has sufficient space. If the total reassembled payload exceeds the
receive buffer capacity, this results in a heap buffer overflow.

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

Add a bounds check before the memcpy() to verify that the fragment
payload fits within the remaining buffer space, using the same error
handling pattern already present in the function.

Cc: stable@vger.kernel.org
Signed-off-by: Delene Tchio Romuald <delenetchior1@gmail.com>
---
 drivers/staging/rtl8723bs/core/rtw_recv.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/rtl8723bs/core/rtw_recv.c b/drivers/staging/rtl8723bs/core/rtw_recv.c
index 337671b12..901f4b1ff 100644
--- a/drivers/staging/rtl8723bs/core/rtw_recv.c
+++ b/drivers/staging/rtl8723bs/core/rtw_recv.c
@@ -1132,7 +1132,13 @@ static union recv_frame *recvframe_defrag(struct adapter *adapter,
 		/* append  to first fragment frame's tail (if privacy frame, pull the ICV) */
 		recvframe_pull_tail(prframe, pfhdr->attrib.icv_len);
 
-		/* memcpy */
+		/* Verify the receiving buffer has enough space for the fragment */
+		if (pnfhdr->len > (uint)(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(pfhdr->rx_tail, pnfhdr->rx_data, pnfhdr->len);
 
 		recvframe_put(prframe, pnfhdr->len);
-- 
2.43.0

Re: [PATCH] staging: rtl8723bs: fix heap buffer overflow in recvframe_defrag()

[Greg KH]

On Sat, Apr 04, 2026 at 10:11:41PM +0100, Delene Tchio Romuald wrote:
> In recvframe_defrag(), a memcpy() copies fragment data into the
> reassembly buffer before recvframe_put() validates that the buffer
> has sufficient space. If the total reassembled payload exceeds the
> receive buffer capacity, this results in a heap buffer overflow.
> 
> An attacker within WiFi radio range can exploit this by sending
> crafted 802.11 fragmented frames. No authentication is required.
> 
> Add a bounds check before the memcpy() to verify that the fragment
> payload fits within the remaining buffer space, using the same error
> handling pattern already present in the function.
> 
> Cc: stable@vger.kernel.org
> Signed-off-by: Delene Tchio Romuald <delenetchior1@gmail.com>
> ---
>  drivers/staging/rtl8723bs/core/rtw_recv.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/rtl8723bs/core/rtw_recv.c b/drivers/staging/rtl8723bs/core/rtw_recv.c
> index 337671b12..901f4b1ff 100644
> --- a/drivers/staging/rtl8723bs/core/rtw_recv.c
> +++ b/drivers/staging/rtl8723bs/core/rtw_recv.c
> @@ -1132,7 +1132,13 @@ static union recv_frame *recvframe_defrag(struct adapter *adapter,
>  		/* append  to first fragment frame's tail (if privacy frame, pull the ICV) */
>  		recvframe_pull_tail(prframe, pfhdr->attrib.icv_len);
>  
> -		/* memcpy */
> +		/* Verify the receiving buffer has enough space for the fragment */
> +		if (pnfhdr->len > (uint)(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(pfhdr->rx_tail, pnfhdr->rx_data, pnfhdr->len);
>  
>  		recvframe_put(prframe, pnfhdr->len);
> -- 
> 2.43.0
> 
> 

As you sent this to a public list, can you resend it to the proper set
that scripts/get_maintainer.pl shows to use?

Also, do you have this hardware to test this change (and the other
changes you made to this driver)?

And finally, how did you find this problem?

thanks,

greg k-h

Re: [PATCH] staging: rtl8723bs: fix heap buffer overflow in recvframe_defrag()

[Romuald Delene]

On Mon, May 04, 2026 at 10:27:00AM +0200, Greg KH wrote:
> For some reason you sent this only to me, which is a bit rude to
> everyone else on the mailing list.  I'll be glad to respond if you
> resend it to everyone.

You are right and I am sorry, this exchange should have been on
the list from the start. Thank you for the gentle correction;
I will be more careful going forward.

Resending the methodology below so everyone has the same context.

For reference, the rebased series is now on the list as
[PATCH v6 0/5] and has received Reviewed-by from Dan Carpenter
and Luka Gejak.

The exchange this answers (also sent privately on April 5,
quoted here for everyone's benefit) was:

  > > To answer your questions:
  > >
  > > I do not own any RTL8723BS hardware. The problem was discovered
  > > during a code review of `recvframe_defrag()` and has not been
  > > tested on any hardware.
  >
  > What type of "manual review"?  How did you trace the data flow
  > here exactly?
  >
  > thanks,
  > greg k-h

The methodology explanation :

----- -------------------------------------------------------------------------------
-----

I reviewed memory operations (memcpy, kmalloc, buffer pointer
manipulation) in the rtl8723bs driver, focusing on rtw_recv.c which
has 40 memcpy calls handling network-received data.

I traced the buffer pointers (rx_data, rx_tail, rx_end) through the
inline helpers in rtw_recv.h to verify bounds checking before each
copy.

The recv_frame_hdr structure (rtw_recv.h:280-297) has four pointers
that define the buffer layout:

  rx_head --> rx_data --> rx_tail --> rx_end

rx_tail marks the end of valid data, rx_end marks the end of the
allocated buffer. The space available for writing is rx_end - rx_tail.

The defrag path in recvframe_defrag() (rtw_recv.c:1074) stood out
because it is the only place where memcpy is called before
recvframe_put validates the buffer space. In each iteration of the
while loop (line 1108), three operations happen in sequence:

 1. recvframe_pull(pnextrframe, wlanhdr_offset) at line 1130 strips
    the 802.11 header from the incoming fragment by advancing its
    rx_data pointer (rtw_recv.h:369). After this, pnfhdr->len
    contains only the payload size.

 2. recvframe_pull_tail(prframe, icv_len) at line 1133 removes the
    ICV from the accumulation buffer by moving rx_tail backward
    (rtw_recv.h:420), freeing a few bytes of space.

 3. memcpy(pfhdr->rx_tail, pnfhdr->rx_data, pnfhdr->len) at line
    1136 copies the fragment payload into the accumulation buffer.
    Then recvframe_put(prframe, pnfhdr->len) at line 1138 advances
    rx_tail and checks if rx_tail > rx_end (rtw_recv.h:395-399).

The problem is the ordering of step 3: memcpy writes pnfhdr->len
bytes at rx_tail BEFORE recvframe_put checks whether rx_tail + len
exceeds rx_end. If the accumulated fragments exceed the buffer,
memcpy writes past rx_end into adjacent heap memory. The return
value of recvframe_put (NULL on overflow) is also never checked,
so execution continues normally.

The fragment payloads are attacker-controlled (received over the
air) and 802.11 fragmentation is handled at the MAC layer before
any authentication, so no association or key exchange is required.

Best regards,
 Romuald