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

[Dan Carpenter <error27@gmail.com>]

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