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

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

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