[PATCH] staging: rtl8723bs: fix mismatched free of HalData in rtw_sdio_if1_init()

[PATCH] staging: rtl8723bs: fix mismatched free of HalData in rtw_sdio_if1_init()

[Dawei Feng]

padapter->HalData is allocated via vzalloc(), but incorrectly freed
using kfree() in the rtw_sdio_if1_init() error path. Using kfree() to
release this vmalloc-backed buffer can lead to memory corruption.

Use rtw_hal_data_deinit() to pair the free correctly and free
HalData with vfree().

The bug was first flagged by an experimental static analysis tool we
are developing for kernel memory-management bugs. Manual inspection
confirms that the issue is still present in current mainline.

An x86_64 allyesconfig build showed no new warnings. As we do not have
suitable RTL8723BS SDIO hardware to test with, no runtime testing was
able to be performed.

Fixes: 554c0a3abf21 ("staging: Add rtl8723bs sdio wifi driver")
Signed-off-by: Zilin Guan <zilin@seu.edu.cn>
Signed-off-by: Dawei Feng <dawei.feng@seu.edu.cn>
---
 drivers/staging/rtl8723bs/os_dep/sdio_intf.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/rtl8723bs/os_dep/sdio_intf.c b/drivers/staging/rtl8723bs/os_dep/sdio_intf.c
index d0feb28b7043..67ef0e1271b0 100644
--- a/drivers/staging/rtl8723bs/os_dep/sdio_intf.c
+++ b/drivers/staging/rtl8723bs/os_dep/sdio_intf.c
@@ -287,8 +287,8 @@ static struct adapter *rtw_sdio_if1_init(struct dvobj_priv *dvobj, const struct
 	status = _SUCCESS;
 
 free_hal_data:
-	if (status != _SUCCESS && padapter->HalData)
-		kfree(padapter->HalData);
+	if (status != _SUCCESS)
+		rtw_hal_data_deinit(padapter);
 
 	if (status != _SUCCESS) {
 		rtw_wdev_unregister(padapter->rtw_wdev);
-- 
2.34.1

Re: [PATCH] staging: rtl8723bs: fix mismatched free of HalData in rtw_sdio_if1_init()

[Dan Carpenter]

On Mon, May 25, 2026 at 05:18:36PM +0800, Dawei Feng wrote:
> padapter->HalData is allocated via vzalloc(), but incorrectly freed
> using kfree() in the rtw_sdio_if1_init() error path. Using kfree() to
> release this vmalloc-backed buffer can lead to memory corruption.
> 
> Use rtw_hal_data_deinit() to pair the free correctly and free
> HalData with vfree().
> 
> The bug was first flagged by an experimental static analysis tool we
> are developing for kernel memory-management bugs.

The rest of the commit message from HERE

> Manual inspection
> confirms that the issue is still present in current mainline.
> 
> An x86_64 allyesconfig build showed no new warnings. As we do not have
> suitable RTL8723BS SDIO hardware to test with, no runtime testing was
> able to be performed.

to HERE should be put

> 
> Fixes: 554c0a3abf21 ("staging: Add rtl8723bs sdio wifi driver")
> Signed-off-by: Zilin Guan <zilin@seu.edu.cn>
> Signed-off-by: Dawei Feng <dawei.feng@seu.edu.cn>
> ---
  ^^^
Here under the --- cut off line.  We don't need this kind of meta
commentary about testing in the permanent git log.  Otherwise
the patch is correct.

regards,
dan carpenter

Re: [PATCH] staging: rtl8723bs: fix mismatched free of HalData in rtw_sdio_if1_init()

[Dawei Feng]

On Thu, May 28, 2026 at 15:30:18 Dan Carpenter wrote:
> > Manual inspection
> > confirms that the issue is still present in current mainline.
> > 
> > An x86_64 allyesconfig build showed no new warnings. As we do not have
> > suitable RTL8723BS SDIO hardware to test with, no runtime testing was
> > able to be performed.
>
> to HERE should be put
>
> > 
> > Fixes: 554c0a3abf21 ("staging: Add rtl8723bs sdio wifi driver")
> > Signed-off-by: Zilin Guan <zilin@seu.edu.cn>
> > Signed-off-by: Dawei Feng <dawei.feng@seu.edu.cn>
> > ---
>   ^^^
> Here under the --- cut off line.  We don't need this kind of meta
> commentary about testing in the permanent git log.  Otherwise
> the patch is correct.

Hi Dan,

Thank you for the review and for pointing this out. 

The reason the manual inspection and testing commentary was placed above
the `---` line is that we were strictly following the example template
provided in Documentation/process/researcher-guidelines.rst. 

In the researcher-guidelines[1], the example explicitly places the build
and hardware testing disclaimer before the Signed-off-by tags, which is
why we included it directly in the commit message.

Please let me know if you would like a v2 to adjust the position of the
mentioned commit log details.

[1] https://docs.kernel.org/process/researcher-guidelines.html

Best regards,
Dawei

Re: [PATCH] staging: rtl8723bs: fix mismatched free of HalData in rtw_sdio_if1_init()

[Dan Carpenter]

On Thu, May 28, 2026 at 06:15:42PM +0800, Dawei Feng wrote:
> On Thu, May 28, 2026 at 15:30:18 Dan Carpenter wrote:
> > > Manual inspection
> > > confirms that the issue is still present in current mainline.
> > > 
> > > An x86_64 allyesconfig build showed no new warnings. As we do not have
> > > suitable RTL8723BS SDIO hardware to test with, no runtime testing was
> > > able to be performed.
> >
> > to HERE should be put
> >
> > > 
> > > Fixes: 554c0a3abf21 ("staging: Add rtl8723bs sdio wifi driver")
> > > Signed-off-by: Zilin Guan <zilin@seu.edu.cn>
> > > Signed-off-by: Dawei Feng <dawei.feng@seu.edu.cn>
> > > ---
> >   ^^^
> > Here under the --- cut off line.  We don't need this kind of meta
> > commentary about testing in the permanent git log.  Otherwise
> > the patch is correct.
> 
> Hi Dan,
> 
> Thank you for the review and for pointing this out. 
> 
> The reason the manual inspection and testing commentary was placed above
> the `---` line is that we were strictly following the example template
> provided in Documentation/process/researcher-guidelines.rst. 

Ah, hm.  Perhaps, the rules are changing.  That's fine then.  I would
normally ask for a v2 but you copied from our template so that's on us.
No need to resend.

Reviewed-by: Dan Carpenter <error27@gmail.com>

regards,
dan carpenter