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

Dan Carpenter
2026-04-15T19:56:12Z

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