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