[PATCH v2 0/3] staging: rtl8723bs: fix error handling and memory leaks

[PATCH v2 0/3] staging: rtl8723bs: fix error handling and memory leaks

[Samasth Norway Ananda]

This series fixes memory leaks and missing error checks in rtl8723bs:

1. Firmware not released on error paths in rtl8723b_FirmwareDownload().
2. Buffer not freed when cfg80211_inform_bss_frame() fails.
3. Missing IS_ERR() check for kthread_run() in rtl8723b_start_thread().

Changes in v2:
-> Patch 1: Call release_firmware() directly in error paths instead of
using intermediate goto label.
-> Dropped patch 4 (rtw_wdev_alloc) to study cleanup chain further.

Samasth Norway Ananda (3):
  staging: rtl8723bs: fix firmware memory leak on error
  staging: rtl8723bs: fix memory leak in rtw_cfg80211_inform_bss()
  staging: rtl8723bs: add IS_ERR() check for kthread_run()

 drivers/staging/rtl8723bs/hal/rtl8723b_hal_init.c | 4 ++++
 drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c | 4 +++-
 2 files changed, 7 insertions(+), 1 deletion(-)

-- 
2.50.1

[PATCH v2 1/3] staging: rtl8723bs: fix firmware memory leak on error

[Samasth Norway Ananda]

After successfully calling request_firmware(), if the firmware size
check fails or if kmemdup() fails, the code jumps to the exit label
without calling release_firmware(), causing a memory leak. Call
release_firmware() directly in each error path before jumping to cleanup
label.

Signed-off-by: Samasth Norway Ananda <samasth.norway.ananda@oracle.com>
---
 drivers/staging/rtl8723bs/hal/rtl8723b_hal_init.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/staging/rtl8723bs/hal/rtl8723b_hal_init.c b/drivers/staging/rtl8723bs/hal/rtl8723b_hal_init.c
index 57c83f332e74..56ceedd5a26a 100644
--- a/drivers/staging/rtl8723bs/hal/rtl8723b_hal_init.c
+++ b/drivers/staging/rtl8723bs/hal/rtl8723b_hal_init.c
@@ -346,12 +346,14 @@ s32 rtl8723b_FirmwareDownload(struct adapter *padapter, bool  bUsedWoWLANFw)
 
 	if (fw->size > FW_8723B_SIZE) {
 		rtStatus = _FAIL;
+		release_firmware(fw);
 		goto exit;
 	}
 
 	pFirmware->fw_buffer_sz = kmemdup(fw->data, fw->size, GFP_KERNEL);
 	if (!pFirmware->fw_buffer_sz) {
 		rtStatus = _FAIL;
+		release_firmware(fw);
 		goto exit;
 	}
 
-- 
2.50.1

Re: [PATCH v2 1/3] staging: rtl8723bs: fix firmware memory leak on error

[Dan Carpenter]

On Thu, Jan 08, 2026 at 10:16:09AM -0800, Samasth Norway Ananda wrote:
> After successfully calling request_firmware(), if the firmware size
> check fails or if kmemdup() fails, the code jumps to the exit label
> without calling release_firmware(), causing a memory leak. Call
> release_firmware() directly in each error path before jumping to cleanup
> label.
> 
> Signed-off-by: Samasth Norway Ananda <samasth.norway.ananda@oracle.com>

Reviewed-by: Dan Carpenter <dan.carpenter@linaro.org>

regards,
dan carpenter

[PATCH v2 2/3] staging: rtl8723bs: fix memory leak in rtw_cfg80211_inform_bss()

[Samasth Norway Ananda]

After successfully allocating buf with kzalloc(), if
cfg80211_inform_bss_frame() returns NULL, the code jumps to the exit
label without freeing buf, causing a memory leak. Add kfree(buf) before
the goto to properly free the buffer in this error case.

Signed-off-by: Samasth Norway Ananda <samasth.norway.ananda@oracle.com>
---
 drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c b/drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c
index 60edeae1cffe..d80e23cfdf8d 100644
--- a/drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c
+++ b/drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c
@@ -314,8 +314,10 @@ struct cfg80211_bss *rtw_cfg80211_inform_bss(struct adapter *padapter, struct wl
 	bss = cfg80211_inform_bss_frame(wiphy, notify_channel, (struct ieee80211_mgmt *)buf,
 					len, notify_signal, GFP_ATOMIC);
 
-	if (unlikely(!bss))
+	if (unlikely(!bss)) {
+		kfree(buf);
 		goto exit;
+	}
 
 	cfg80211_put_bss(wiphy, bss);
 	kfree(buf);
-- 
2.50.1

Re: [PATCH v2 2/3] staging: rtl8723bs: fix memory leak in rtw_cfg80211_inform_bss()

[Dan Carpenter]

On Thu, Jan 08, 2026 at 10:16:10AM -0800, Samasth Norway Ananda wrote:
> After successfully allocating buf with kzalloc(), if
> cfg80211_inform_bss_frame() returns NULL, the code jumps to the exit
> label without freeing buf, causing a memory leak. Add kfree(buf) before
> the goto to properly free the buffer in this error case.
> 
> Signed-off-by: Samasth Norway Ananda <samasth.norway.ananda@oracle.com>
> ---
>  drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c b/drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c
> index 60edeae1cffe..d80e23cfdf8d 100644
> --- a/drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c
> +++ b/drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c
> @@ -314,8 +314,10 @@ struct cfg80211_bss *rtw_cfg80211_inform_bss(struct adapter *padapter, struct wl
>  	bss = cfg80211_inform_bss_frame(wiphy, notify_channel, (struct ieee80211_mgmt *)buf,
>  					len, notify_signal, GFP_ATOMIC);
>  
> -	if (unlikely(!bss))
> +	if (unlikely(!bss)) {
> +		kfree(buf);
>  		goto exit;
> +	}
>  
>  	cfg80211_put_bss(wiphy, bss);
>  	kfree(buf);

This code is so ugly but that's not really related to your patch...

Reviewed-by: Dan Carpenter <dan.carpenter@linaro.org>

regards,
dan carpenter

[PATCH v2 3/3] staging: rtl8723bs: add IS_ERR() check for kthread_run()

[Samasth Norway Ananda]

kthread_run() returns an ERR_PTR on failure, not NULL. Without this
check, rtl8723b_stop_thread() would later check "if
(xmitpriv->SdioXmitThread)" which evaluates to true for error pointers,
potentially causing issues when trying to complete or wait on an invalid
thread. Set the pointer to NULL on failure to prevent later code from
attempting to use an invalid thread pointer.

Signed-off-by: Samasth Norway Ananda <samasth.norway.ananda@oracle.com>
---
 drivers/staging/rtl8723bs/hal/rtl8723b_hal_init.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/staging/rtl8723bs/hal/rtl8723b_hal_init.c b/drivers/staging/rtl8723bs/hal/rtl8723b_hal_init.c
index 56ceedd5a26a..27d490204fcc 100644
--- a/drivers/staging/rtl8723bs/hal/rtl8723b_hal_init.c
+++ b/drivers/staging/rtl8723bs/hal/rtl8723b_hal_init.c
@@ -2922,6 +2922,8 @@ void rtl8723b_start_thread(struct adapter *padapter)
 	struct xmit_priv *xmitpriv = &padapter->xmitpriv;
 
 	xmitpriv->SdioXmitThread = kthread_run(rtl8723bs_xmit_thread, padapter, "RTWHALXT");
+	if (IS_ERR(xmitpriv->SdioXmitThread))
+		xmitpriv->SdioXmitThread = NULL
 }
 
 void rtl8723b_stop_thread(struct adapter *padapter)
-- 
2.50.1

Re: [PATCH v2 3/3] staging: rtl8723bs: add IS_ERR() check for kthread_run()

[Greg KH]

On Thu, Jan 08, 2026 at 10:16:11AM -0800, Samasth Norway Ananda wrote:
> kthread_run() returns an ERR_PTR on failure, not NULL. Without this
> check, rtl8723b_stop_thread() would later check "if
> (xmitpriv->SdioXmitThread)" which evaluates to true for error pointers,
> potentially causing issues when trying to complete or wait on an invalid
> thread. Set the pointer to NULL on failure to prevent later code from
> attempting to use an invalid thread pointer.
> 
> Signed-off-by: Samasth Norway Ananda <samasth.norway.ananda@oracle.com>
> ---
>  drivers/staging/rtl8723bs/hal/rtl8723b_hal_init.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/staging/rtl8723bs/hal/rtl8723b_hal_init.c b/drivers/staging/rtl8723bs/hal/rtl8723b_hal_init.c
> index 56ceedd5a26a..27d490204fcc 100644
> --- a/drivers/staging/rtl8723bs/hal/rtl8723b_hal_init.c
> +++ b/drivers/staging/rtl8723bs/hal/rtl8723b_hal_init.c
> @@ -2922,6 +2922,8 @@ void rtl8723b_start_thread(struct adapter *padapter)
>  	struct xmit_priv *xmitpriv = &padapter->xmitpriv;
>  
>  	xmitpriv->SdioXmitThread = kthread_run(rtl8723bs_xmit_thread, padapter, "RTWHALXT");
> +	if (IS_ERR(xmitpriv->SdioXmitThread))
> +		xmitpriv->SdioXmitThread = NULL

Shouldn't the function be returning an errror if this fails instead of
relying on the pointer to be NULL?

And this is a "wrapper function" and only called in one place, why not
just get rid of it entirely?  Same for rtl8723b_stop_thread()?

thanks,

greg k-h

Re: [External] : Re: [PATCH v2 3/3] staging: rtl8723bs: add IS_ERR() check for kthread_run()

[samasth.norway.ananda]

On 1/8/26 8:50 PM, Greg KH wrote:
> On Thu, Jan 08, 2026 at 10:16:11AM -0800, Samasth Norway Ananda wrote:
>> kthread_run() returns an ERR_PTR on failure, not NULL. Without this
>> check, rtl8723b_stop_thread() would later check "if
>> (xmitpriv->SdioXmitThread)" which evaluates to true for error pointers,
>> potentially causing issues when trying to complete or wait on an invalid
>> thread. Set the pointer to NULL on failure to prevent later code from
>> attempting to use an invalid thread pointer.
>>
>> Signed-off-by: Samasth Norway Ananda <samasth.norway.ananda@oracle.com>
>> ---
>>   drivers/staging/rtl8723bs/hal/rtl8723b_hal_init.c | 2 ++
>>   1 file changed, 2 insertions(+)
>>
>> diff --git a/drivers/staging/rtl8723bs/hal/rtl8723b_hal_init.c b/drivers/staging/rtl8723bs/hal/rtl8723b_hal_init.c
>> index 56ceedd5a26a..27d490204fcc 100644
>> --- a/drivers/staging/rtl8723bs/hal/rtl8723b_hal_init.c
>> +++ b/drivers/staging/rtl8723bs/hal/rtl8723b_hal_init.c
>> @@ -2922,6 +2922,8 @@ void rtl8723b_start_thread(struct adapter *padapter)
>>   	struct xmit_priv *xmitpriv = &padapter->xmitpriv;
>>   
>>   	xmitpriv->SdioXmitThread = kthread_run(rtl8723bs_xmit_thread, padapter, "RTWHALXT");
>> +	if (IS_ERR(xmitpriv->SdioXmitThread))
>> +		xmitpriv->SdioXmitThread = NULL
> 
> Shouldn't the function be returning an errror if this fails instead of
> relying on the pointer to be NULL?
> 
> And this is a "wrapper function" and only called in one place, why not
> just get rid of it entirely?  Same for rtl8723b_stop_thread()?

Hi Greg,

Thanks for the feedback. You're right, just setting the pointer to NULL 
just hides the error. The wrapper functions are unnecessary as well.

I will send a v3 that removes rtl8723b_start_thread(), 
rtl8723b_stop_thread(), rtw_hal_start_thread() and rtw_hal_stop_thread() 
entirely, inlining the kthread handling directly to 
rtw_start_drv_threads() and rtw_stop_drv_threads() with proper IS_ERR() 
checking.

Thanks,
Samasth.

> 
> thanks,
> 
> greg k-h