[PATCH] wifi: ath12k: release QMI handle on late init failures

[PATCH] wifi: ath12k: release QMI handle on late init failures

[Guangshuo Li]

The change referenced by the Fixes tag makes hardware group creation
skip the device whose ath12k_core_soc_create() failed, and only destroys
devices that were created successfully before it.

That avoids releasing an uninitialized QMI handle when qmi_handle_init()
fails, but it also means that ath12k_qmi_init_service() must clean up any
resources it has already acquired before returning an error.

qmi_handle_init() can succeed before later initialization steps fail. If
the QMI event workqueue allocation fails, or qmi_add_lookup() fails, the
function returns an error without releasing the already initialized QMI
handle. Since the failed device is now skipped by the hardware group
rollback path, that handle is leaked.

Release the QMI handle on those late failure paths and clear ab->qmi.ab
so the failed device is left in the same state as the qmi_handle_init()
failure path.

Fixes: 088a099690e4 ("wifi: ath12k: fix error handling in creating hardware group")
Signed-off-by: Guangshuo Li <lgs201920130244@gmail.com>
---
 drivers/net/wireless/ath/ath12k/qmi.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/ath/ath12k/qmi.c b/drivers/net/wireless/ath/ath12k/qmi.c
index fd762b5d7bb5..31e9fa9a62be 100644
--- a/drivers/net/wireless/ath/ath12k/qmi.c
+++ b/drivers/net/wireless/ath/ath12k/qmi.c
@@ -4054,7 +4054,8 @@ int ath12k_qmi_init_service(struct ath12k_base *ab)
 	ab->qmi.event_wq = alloc_ordered_workqueue("ath12k_qmi_driver_event", 0);
 	if (!ab->qmi.event_wq) {
 		ath12k_err(ab, "failed to allocate workqueue\n");
-		return -EFAULT;
+		ret = -EFAULT;
+		goto err_release_qmi_handle;
 	}
 
 	INIT_LIST_HEAD(&ab->qmi.event_list);
@@ -4067,9 +4068,15 @@ int ath12k_qmi_init_service(struct ath12k_base *ab)
 	if (ret < 0) {
 		ath12k_warn(ab, "failed to add qmi lookup\n");
 		destroy_workqueue(ab->qmi.event_wq);
-		return ret;
+		goto err_release_qmi_handle;
 	}
 
+	return ret;
+
+err_release_qmi_handle:
+	qmi_handle_release(&ab->qmi.handle);
+	ab->qmi.ab = NULL;
+
 	return ret;
 }
 
-- 
2.43.0

Re: [PATCH] wifi: ath12k: release QMI handle on late init failures

[Vasanthakumar Thiagarajan]

On 7/8/2026 3:39 PM, Guangshuo Li wrote:
> The change referenced by the Fixes tag makes hardware group creation
> skip the device whose ath12k_core_soc_create() failed, and only destroys
> devices that were created successfully before it.
> 
> That avoids releasing an uninitialized QMI handle when qmi_handle_init()
> fails, but it also means that ath12k_qmi_init_service() must clean up any
> resources it has already acquired before returning an error.
> 
> qmi_handle_init() can succeed before later initialization steps fail. If
> the QMI event workqueue allocation fails, or qmi_add_lookup() fails, the
> function returns an error without releasing the already initialized QMI
> handle. Since the failed device is now skipped by the hardware group
> rollback path, that handle is leaked.
> 
> Release the QMI handle on those late failure paths and clear ab->qmi.ab
> so the failed device is left in the same state as the qmi_handle_init()
> failure path.
> 
> Fixes: 088a099690e4 ("wifi: ath12k: fix error handling in creating hardware group")
> Signed-off-by: Guangshuo Li <lgs201920130244@gmail.com>

Reviewed-by: Vasanthakumar Thiagarajan <vasanthakumar.thiagarajan@oss.qualcomm.com>

btw, similar leak may be there in ath11k also, care to fix that as well?

Re: [PATCH] wifi: ath12k: release QMI handle on late init failures

[Baochen Qiang]

On 7/8/2026 6:09 PM, Guangshuo Li wrote:
> The change referenced by the Fixes tag makes hardware group creation
> skip the device whose ath12k_core_soc_create() failed, and only destroys
> devices that were created successfully before it.
> 
> That avoids releasing an uninitialized QMI handle when qmi_handle_init()
> fails, but it also means that ath12k_qmi_init_service() must clean up any
> resources it has already acquired before returning an error.
> 
> qmi_handle_init() can succeed before later initialization steps fail. If
> the QMI event workqueue allocation fails, or qmi_add_lookup() fails, the
> function returns an error without releasing the already initialized QMI
> handle. Since the failed device is now skipped by the hardware group
> rollback path, that handle is leaked.
> 
> Release the QMI handle on those late failure paths and clear ab->qmi.ab
> so the failed device is left in the same state as the qmi_handle_init()
> failure path.
> 
> Fixes: 088a099690e4 ("wifi: ath12k: fix error handling in creating hardware group")
> Signed-off-by: Guangshuo Li <lgs201920130244@gmail.com>
> ---
>  drivers/net/wireless/ath/ath12k/qmi.c | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/wireless/ath/ath12k/qmi.c b/drivers/net/wireless/ath/ath12k/qmi.c
> index fd762b5d7bb5..31e9fa9a62be 100644
> --- a/drivers/net/wireless/ath/ath12k/qmi.c
> +++ b/drivers/net/wireless/ath/ath12k/qmi.c
> @@ -4054,7 +4054,8 @@ int ath12k_qmi_init_service(struct ath12k_base *ab)
>  	ab->qmi.event_wq = alloc_ordered_workqueue("ath12k_qmi_driver_event", 0);
>  	if (!ab->qmi.event_wq) {
>  		ath12k_err(ab, "failed to allocate workqueue\n");
> -		return -EFAULT;
> +		ret = -EFAULT;
> +		goto err_release_qmi_handle;
>  	}
>  
>  	INIT_LIST_HEAD(&ab->qmi.event_list);
> @@ -4067,9 +4068,15 @@ int ath12k_qmi_init_service(struct ath12k_base *ab)
>  	if (ret < 0) {
>  		ath12k_warn(ab, "failed to add qmi lookup\n");
>  		destroy_workqueue(ab->qmi.event_wq);
> -		return ret;
> +		goto err_release_qmi_handle;
>  	}
>  
> +	return ret;
> +
> +err_release_qmi_handle:
> +	qmi_handle_release(&ab->qmi.handle);
> +	ab->qmi.ab = NULL;

should this be ?

	ab->qmi.handle = NULL;

> +
>  	return ret;
>  }
>  

Re: [PATCH] wifi: ath12k: release QMI handle on late init failures

[Baochen Qiang]

On 7/13/2026 11:17 AM, Baochen Qiang wrote:
> 
> 
> On 7/8/2026 6:09 PM, Guangshuo Li wrote:
>> The change referenced by the Fixes tag makes hardware group creation
>> skip the device whose ath12k_core_soc_create() failed, and only destroys
>> devices that were created successfully before it.
>>
>> That avoids releasing an uninitialized QMI handle when qmi_handle_init()
>> fails, but it also means that ath12k_qmi_init_service() must clean up any
>> resources it has already acquired before returning an error.
>>
>> qmi_handle_init() can succeed before later initialization steps fail. If
>> the QMI event workqueue allocation fails, or qmi_add_lookup() fails, the
>> function returns an error without releasing the already initialized QMI
>> handle. Since the failed device is now skipped by the hardware group
>> rollback path, that handle is leaked.
>>
>> Release the QMI handle on those late failure paths and clear ab->qmi.ab
>> so the failed device is left in the same state as the qmi_handle_init()
>> failure path.
>>
>> Fixes: 088a099690e4 ("wifi: ath12k: fix error handling in creating hardware group")
>> Signed-off-by: Guangshuo Li <lgs201920130244@gmail.com>
>> ---
>>  drivers/net/wireless/ath/ath12k/qmi.c | 11 +++++++++--
>>  1 file changed, 9 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/net/wireless/ath/ath12k/qmi.c b/drivers/net/wireless/ath/ath12k/qmi.c
>> index fd762b5d7bb5..31e9fa9a62be 100644
>> --- a/drivers/net/wireless/ath/ath12k/qmi.c
>> +++ b/drivers/net/wireless/ath/ath12k/qmi.c
>> @@ -4054,7 +4054,8 @@ int ath12k_qmi_init_service(struct ath12k_base *ab)
>>  	ab->qmi.event_wq = alloc_ordered_workqueue("ath12k_qmi_driver_event", 0);
>>  	if (!ab->qmi.event_wq) {
>>  		ath12k_err(ab, "failed to allocate workqueue\n");
>> -		return -EFAULT;
>> +		ret = -EFAULT;
>> +		goto err_release_qmi_handle;
>>  	}
>>  
>>  	INIT_LIST_HEAD(&ab->qmi.event_list);
>> @@ -4067,9 +4068,15 @@ int ath12k_qmi_init_service(struct ath12k_base *ab)
>>  	if (ret < 0) {
>>  		ath12k_warn(ab, "failed to add qmi lookup\n");
>>  		destroy_workqueue(ab->qmi.event_wq);
>> -		return ret;
>> +		goto err_release_qmi_handle;
>>  	}
>>  
>> +	return ret;
>> +
>> +err_release_qmi_handle:
>> +	qmi_handle_release(&ab->qmi.handle);
>> +	ab->qmi.ab = NULL;
> 
> should this be ?
> 
> 	ab->qmi.handle = NULL;

I was wrong, please ignore this comment.

> 
>> +
>>  	return ret;
>>  }
>>  
> 

Re: [PATCH] wifi: ath12k: release QMI handle on late init failures

[Baochen Qiang]

On 7/8/2026 6:09 PM, Guangshuo Li wrote:
> The change referenced by the Fixes tag makes hardware group creation
> skip the device whose ath12k_core_soc_create() failed, and only destroys
> devices that were created successfully before it.
> 
> That avoids releasing an uninitialized QMI handle when qmi_handle_init()
> fails, but it also means that ath12k_qmi_init_service() must clean up any
> resources it has already acquired before returning an error.
> 
> qmi_handle_init() can succeed before later initialization steps fail. If
> the QMI event workqueue allocation fails, or qmi_add_lookup() fails, the
> function returns an error without releasing the already initialized QMI
> handle. Since the failed device is now skipped by the hardware group
> rollback path, that handle is leaked.
> 
> Release the QMI handle on those late failure paths and clear ab->qmi.ab
> so the failed device is left in the same state as the qmi_handle_init()
> failure path.
> 
> Fixes: 088a099690e4 ("wifi: ath12k: fix error handling in creating hardware group")
> Signed-off-by: Guangshuo Li <lgs201920130244@gmail.com>
> ---
>  drivers/net/wireless/ath/ath12k/qmi.c | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/wireless/ath/ath12k/qmi.c b/drivers/net/wireless/ath/ath12k/qmi.c
> index fd762b5d7bb5..31e9fa9a62be 100644
> --- a/drivers/net/wireless/ath/ath12k/qmi.c
> +++ b/drivers/net/wireless/ath/ath12k/qmi.c
> @@ -4054,7 +4054,8 @@ int ath12k_qmi_init_service(struct ath12k_base *ab)
>  	ab->qmi.event_wq = alloc_ordered_workqueue("ath12k_qmi_driver_event", 0);
>  	if (!ab->qmi.event_wq) {
>  		ath12k_err(ab, "failed to allocate workqueue\n");
> -		return -EFAULT;
> +		ret = -EFAULT;
> +		goto err_release_qmi_handle;
>  	}
>  
>  	INIT_LIST_HEAD(&ab->qmi.event_list);
> @@ -4067,9 +4068,15 @@ int ath12k_qmi_init_service(struct ath12k_base *ab)
>  	if (ret < 0) {
>  		ath12k_warn(ab, "failed to add qmi lookup\n");
>  		destroy_workqueue(ab->qmi.event_wq);
> -		return ret;
> +		goto err_release_qmi_handle;
>  	}
>  
> +	return ret;
> +
> +err_release_qmi_handle:
> +	qmi_handle_release(&ab->qmi.handle);
> +	ab->qmi.ab = NULL;

since ath12k_qmi_deinit_service() NULL-checks ab->qmi.ab to determine whether qmi service
init succeeds, does it make more sense to move ab->qmi.ab initialization in
ath12k_qmi_init_service() to the last step? This achieves symmetry and also avoids reset
in the failure path.

> +
>  	return ret;
>  }
>