[PATCH v1] staging: rtl8723bs: fix stale recv_frame free in recv_func_posthandle()

[PATCH v1] staging: rtl8723bs: fix stale recv_frame free in recv_func_posthandle()

[Yuho Choi]

recv_func_posthandle() saved the original recv_frame pointer before
calling recvframe_chk_defrag().

On the last-fragment reassembly path, recvframe_chk_defrag() may return
the first fragment as the new frame while freeing the original
last-fragment frame when draining the defrag queue.

If process_recv_indicatepkts() then fails, recv_func_posthandle() frees
the saved pre-defrag pointer again, which can result in a stale pointer
free.

Free the current recv_frame on the failure path instead of the saved
pre-defrag pointer.

Fixes: 554c0a3abf21 ("staging: Add rtl8723bs sdio wifi driver")
Co-developed-by: Myeonghun Pak <mhun512@gmail.com>
Signed-off-by: Myeonghun Pak <mhun512@gmail.com>
Co-developed-by: Ijae Kim <ae878000@gmail.com>
Signed-off-by: Ijae Kim <ae878000@gmail.com>
Co-developed-by: Taegyu Kim <tmk5904@psu.edu>
Signed-off-by: Taegyu Kim <tmk5904@psu.edu>
Signed-off-by: Yuho Choi <dbgh9129@gmail.com>
---
 drivers/staging/rtl8723bs/core/rtw_recv.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/staging/rtl8723bs/core/rtw_recv.c b/drivers/staging/rtl8723bs/core/rtw_recv.c
index 337671b1211f0..a404b6fc97723 100644
--- a/drivers/staging/rtl8723bs/core/rtw_recv.c
+++ b/drivers/staging/rtl8723bs/core/rtw_recv.c
@@ -2139,7 +2139,6 @@ static int recv_func_prehandle(struct adapter *padapter, union recv_frame *rfram
 static int recv_func_posthandle(struct adapter *padapter, union recv_frame *prframe)
 {
 	int ret = _SUCCESS;
-	union recv_frame *orig_prframe = prframe;
 	struct recv_priv *precvpriv = &padapter->recvpriv;
 	struct __queue *pfree_recv_queue = &padapter->recvpriv.free_recv_queue;
 
@@ -2163,7 +2162,7 @@ static int recv_func_posthandle(struct adapter *padapter, union recv_frame *prfr
 
 	ret = process_recv_indicatepkts(padapter, prframe);
 	if (ret != _SUCCESS) {
-		rtw_free_recvframe(orig_prframe, pfree_recv_queue);/* free this recv_frame */
+		rtw_free_recvframe(prframe, pfree_recv_queue);/* free this recv_frame */
 		goto _recv_data_drop;
 	}
 
-- 
2.50.1 (Apple Git-155)

Re: [PATCH v1] staging: rtl8723bs: fix stale recv_frame free in recv_func_posthandle()

[Andy Shevchenko]

On Mon, Apr 20, 2026 at 12:27:34AM -0400, Yuho Choi wrote:
> recv_func_posthandle() saved the original recv_frame pointer before
> calling recvframe_chk_defrag().
> 
> On the last-fragment reassembly path, recvframe_chk_defrag() may return
> the first fragment as the new frame while freeing the original
> last-fragment frame when draining the defrag queue.
> 
> If process_recv_indicatepkts() then fails, recv_func_posthandle() frees
> the saved pre-defrag pointer again, which can result in a stale pointer
> free.
> 
> Free the current recv_frame on the failure path instead of the saved
> pre-defrag pointer.
> 
> Fixes: 554c0a3abf21 ("staging: Add rtl8723bs sdio wifi driver")
> Co-developed-by: Myeonghun Pak <mhun512@gmail.com>
> Signed-off-by: Myeonghun Pak <mhun512@gmail.com>
> Co-developed-by: Ijae Kim <ae878000@gmail.com>
> Signed-off-by: Ijae Kim <ae878000@gmail.com>
> Co-developed-by: Taegyu Kim <tmk5904@psu.edu>
> Signed-off-by: Taegyu Kim <tmk5904@psu.edu>

Same. Are you, folks, doing some AI/static analyser tool?

> Signed-off-by: Yuho Choi <dbgh9129@gmail.com>

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v1] staging: rtl8723bs: fix stale recv_frame free in recv_func_posthandle()

[Greg Kroah-Hartman]

On Mon, Apr 20, 2026 at 12:27:34AM -0400, Yuho Choi wrote:
> recv_func_posthandle() saved the original recv_frame pointer before
> calling recvframe_chk_defrag().
> 
> On the last-fragment reassembly path, recvframe_chk_defrag() may return
> the first fragment as the new frame while freeing the original
> last-fragment frame when draining the defrag queue.
> 
> If process_recv_indicatepkts() then fails, recv_func_posthandle() frees
> the saved pre-defrag pointer again, which can result in a stale pointer
> free.
> 
> Free the current recv_frame on the failure path instead of the saved
> pre-defrag pointer.

Can you cause this to happen in any way?  Given the age of this code,
and the crazy paths here, I'm loath to change this without lots of
testing with a real device, have you done so?

thanks,

greg k-h

Re: [PATCH v1] staging: rtl8723bs: fix stale recv_frame free in recv_func_posthandle()

[Dan Carpenter]

On Mon, Apr 20, 2026 at 12:27:34AM -0400, Yuho Choi wrote:
> recv_func_posthandle() saved the original recv_frame pointer before
> calling recvframe_chk_defrag().
> 
> On the last-fragment reassembly path, recvframe_chk_defrag() may return
> the first fragment as the new frame while freeing the original
> last-fragment frame when draining the defrag queue.
> 
> If process_recv_indicatepkts() then fails, recv_func_posthandle() frees
> the saved pre-defrag pointer again, which can result in a stale pointer
> free.

You seem to be saying that process_recv_indicatepkts() frees
orig_prframe.  And, sure, that's true.  But then it returns NULL and we
goto _recv_data_drop so we don't hit this path.

This seems like a false positive.

regards,
dan carpenter

> 
> Free the current recv_frame on the failure path instead of the saved
> pre-defrag pointer.
> 
> Fixes: 554c0a3abf21 ("staging: Add rtl8723bs sdio wifi driver")