Re: [PATCH v7] wifi: ath11k: fix resource leak on error in ext IRQ setup

[Baochen Qiang]

On 7/28/2026 11:13 AM, ZhaoJinming wrote:
> In ath11k_ahb_config_irq(), when a CE request_irq() fails, the function
> returns the error immediately without freeing the CE IRQs that were
> successfully registered in previous loop iterations. The probe error
> path does not call ath11k_ahb_free_irq() either, so the previously
> registered CE IRQ handlers remain attached to the interrupt lines and
> are never released.
> 
> In ath11k_ahb_config_ext_irq(), when an external request_irq() fails,
> the error is only logged and the loop continues. The function then
> returns 0 indicating success, leaving the device in a partially
> configured state where some external IRQs are not registered. This
> causes enable_irq()/disable_irq()/free_irq() to be called on
> unregistered IRQs during runtime and remove/shutdown, triggering
> WARN_ON(!desc->action), and missing interrupt handlers lead to data
> loss.
> 
> Additionally, if alloc_netdev_dummy() fails for a later IRQ group, the
> function returns -ENOMEM without freeing the ext IRQs and napi_ndev
> that were successfully set up for earlier groups.
> 
> Fix all three issues: propagate the error up to the caller and unwind
> all successfully registered IRQs and allocated resources on failure.
> Also move ab->irq_num[irq_idx] assignment after request_irq() succeeds
> in the ext IRQ path to match the CE IRQ path and avoid storing a stale
> IRQ number on failure.
> 
> Link: https://lore.kernel.org/all/b6903572-52cc-4c41-b9cf-5845e387c176@xxxxxxxxxxxxxxxx/

this is not the right way to track old versions. If want to do that the right place is in
the changelog.

Note if you use b4 tool this tracking burden can be done automatically.

> Signed-off-by: ZhaoJinming <zhaojinming@xxxxxxxxxxxxx>
> ---
> 
> Changes in v7:
> - Resend in a new thread. No content changes from v6.

I mean here.

> 
> Changes in v6:
> - Move ab->irq_num[irq_idx] = irq after request_irq() succeeds in the
>   ext IRQ path to match the CE IRQ logic and avoid storing a stale IRQ
>   number on failure.
> 
> Changes in v5:
> - Resend as a fresh thread. No content changes from v4.
> 
> Changes in v4:
> - Drop the napi_ndev NULL guard in ath11k_ahb_free_ext_irq_grp() as it
>   is unreachable in all existing call paths. A silent guard against an
>   impossible condition hides bugs instead of exposing them.
> 
> Changes in v3:
> - Extract ath11k_ahb_free_ext_irq_grp() to handle per-group ext IRQ
>   cleanup and refactor ath11k_ahb_free_ext_irq() to use it.
> - On request_irq() failure in ath11k_ahb_config_ext_irq(), immediately
>   clean up the current group via ath11k_ahb_free_ext_irq_grp() before
>   jumping to the error label to unwind earlier groups.
> - Move u32 num_irq declaration before the irq_grp assignment to follow
>   declaration-before-statement ordering.
> - Extract ath11k_ahb_free_ce_irqs() to handle partial/full CE IRQ cleanup
>   and refactor ath11k_ahb_free_irq() to use it, replacing the duplicated
>   err_ce_irq cleanup loop in ath11k_ahb_config_irq().
> 
> Changes in v2:
> - Move `irq_grp` declaration from for-loop body to function scope to fix
>   compile error in err_request_irq error path.
> - Set ret = -ENOMEM before goto err_request_irq on alloc_netdev_dummy()
>   failure to avoid returning uninitialized value.
> - When ath11k_ahb_config_ext_irq() fails, unwind the already-registered
>   CE IRQs via a shared err_ce_irq cleanup path to avoid leaking them.
> 
> diff --git a/drivers/net/wireless/ath/ath11k/ahb.c b/drivers/net/wireless/ath/ath11k/ahb.c
> index 1e1dea485760..ec5bf5c9fd79 100644
> --- a/drivers/net/wireless/ath/ath11k/ahb.c
> +++ b/drivers/net/wireless/ath/ath11k/ahb.c
> @@ -431,36 +431,44 @@ static void ath11k_ahb_init_qmi_ce_config(struct ath11k_base *ab)
>  	ab->qmi.service_ins_id = ab->hw_params.qmi_service_ins_id;
>  }
>  
> -static void ath11k_ahb_free_ext_irq(struct ath11k_base *ab)
> +static void ath11k_ahb_free_ext_irq_grp(struct ath11k_base *ab,
> +					struct ath11k_ext_irq_grp *irq_grp)
>  {
> -	int i, j;
> -
> -	for (i = 0; i < ATH11K_EXT_IRQ_GRP_NUM_MAX; i++) {
> -		struct ath11k_ext_irq_grp *irq_grp = &ab->ext_irq_grp[i];
> +	int j;
>  
> -		for (j = 0; j < irq_grp->num_irq; j++)
> -			free_irq(ab->irq_num[irq_grp->irqs[j]], irq_grp);
> +	for (j = 0; j < irq_grp->num_irq; j++)
> +		free_irq(ab->irq_num[irq_grp->irqs[j]], irq_grp);
>  
> -		netif_napi_del(&irq_grp->napi);
> -		free_netdev(irq_grp->napi_ndev);
> -	}
> +	netif_napi_del(&irq_grp->napi);
> +	free_netdev(irq_grp->napi_ndev);
>  }
>  
> -static void ath11k_ahb_free_irq(struct ath11k_base *ab)
> +static void ath11k_ahb_free_ext_irq(struct ath11k_base *ab)
>  {
> -	int irq_idx;
>  	int i;
>  
> -	if (ab->hw_params.hybrid_bus_type)
> -		return ath11k_pcic_free_irq(ab);
> +	for (i = 0; i < ATH11K_EXT_IRQ_GRP_NUM_MAX; i++)
> +		ath11k_ahb_free_ext_irq_grp(ab, &ab->ext_irq_grp[i]);
> +}
>  
> -	for (i = 0; i < ab->hw_params.ce_count; i++) {
> +static void ath11k_ahb_free_ce_irqs(struct ath11k_base *ab, int max_idx)
> +{
> +	int irq_idx, i;
> +
> +	for (i = 0; i < max_idx; i++) {
>  		if (ath11k_ce_get_attr_flags(ab, i) & CE_ATTR_DIS_INTR)
>  			continue;
>  		irq_idx = ATH11K_IRQ_CE0_OFFSET + i;
>  		free_irq(ab->irq_num[irq_idx], &ab->ce.ce_pipe[i]);
>  	}
> +}
> +
> +static void ath11k_ahb_free_irq(struct ath11k_base *ab)
> +{
> +	if (ab->hw_params.hybrid_bus_type)
> +		return ath11k_pcic_free_irq(ab);
>  
> +	ath11k_ahb_free_ce_irqs(ab, ab->hw_params.ce_count);
>  	ath11k_ahb_free_ext_irq(ab);
>  }
>  
> @@ -524,20 +532,25 @@ static irqreturn_t ath11k_ahb_ext_interrupt_handler(int irq, void *arg)
>  static int ath11k_ahb_config_ext_irq(struct ath11k_base *ab)
>  {
>  	struct ath11k_hw_params *hw = &ab->hw_params;
> +	struct ath11k_ext_irq_grp *irq_grp;
>  	int i, j;
>  	int irq;
>  	int ret;
>  
>  	for (i = 0; i < ATH11K_EXT_IRQ_GRP_NUM_MAX; i++) {
> -		struct ath11k_ext_irq_grp *irq_grp = &ab->ext_irq_grp[i];
>  		u32 num_irq = 0;
>  
> +		irq_grp = &ab->ext_irq_grp[i];
> +
>  		irq_grp->ab = ab;
>  		irq_grp->grp_id = i;
>  
>  		irq_grp->napi_ndev = alloc_netdev_dummy(0);
> -		if (!irq_grp->napi_ndev)
> -			return -ENOMEM;
> +		if (!irq_grp->napi_ndev) {
> +			ret = -ENOMEM;
> +			irq_grp->num_irq = 0;
> +			goto err_request_irq;
> +		}
>  
>  		netif_napi_add(irq_grp->napi_ndev, &irq_grp->napi,
>  			       ath11k_ahb_ext_grp_napi_poll);
> @@ -585,14 +598,11 @@ static int ath11k_ahb_config_ext_irq(struct ath11k_base *ab)
>  				}
>  			}
>  		}
> -		irq_grp->num_irq = num_irq;
> -
> -		for (j = 0; j < irq_grp->num_irq; j++) {
> +		for (j = 0; j < num_irq; j++) {
>  			int irq_idx = irq_grp->irqs[j];
>  
>  			irq = platform_get_irq_byname(ab->pdev,
>  						      irq_name[irq_idx]);
> -			ab->irq_num[irq_idx] = irq;
>  			irq_set_status_flags(irq, IRQ_NOAUTOEN | IRQ_DISABLE_UNLAZY);
>  			ret = request_irq(irq, ath11k_ahb_ext_interrupt_handler,
>  					  IRQF_TRIGGER_RISING,
> @@ -600,11 +610,24 @@ static int ath11k_ahb_config_ext_irq(struct ath11k_base *ab)
>  			if (ret) {
>  				ath11k_err(ab, "failed request_irq for %d\n",
>  					   irq);
> +				irq_grp->num_irq = j;
> +				ath11k_ahb_free_ext_irq_grp(ab, irq_grp);
> +				goto err_request_irq;
>  			}
> +			ab->irq_num[irq_idx] = irq;
>  		}
> +
> +		irq_grp->num_irq = num_irq;
>  	}
>  
>  	return 0;
> +
> +err_request_irq:
> +	for (i--; i >= 0; i--) {
> +		irq_grp = &ab->ext_irq_grp[i];
> +		ath11k_ahb_free_ext_irq_grp(ab, irq_grp);
> +	}
> +	return ret;
>  }
>  
>  static int ath11k_ahb_config_irq(struct ath11k_base *ab)
> @@ -629,16 +652,24 @@ static int ath11k_ahb_config_irq(struct ath11k_base *ab)
>  		ret = request_irq(irq, ath11k_ahb_ce_interrupt_handler,
>  				  IRQF_TRIGGER_RISING, irq_name[irq_idx],
>  				  ce_pipe);
> -		if (ret)
> +		if (ret) {
> +			ath11k_err(ab, "failed request_irq for %d\n", irq);
> +			ath11k_ahb_free_ce_irqs(ab, i);
>  			return ret;
> +		}
>  
>  		ab->irq_num[irq_idx] = irq;
>  	}
>  
>  	/* Configure external interrupts */
>  	ret = ath11k_ahb_config_ext_irq(ab);
> +	if (ret) {
> +		ath11k_err(ab, "failed to configure ext irq: %d\n", ret);
> +		ath11k_ahb_free_ce_irqs(ab, ab->hw_params.ce_count);
> +		return ret;
> +	}
>  
> -	return ret;
> +	return 0;
>  }
>  
>  static int ath11k_ahb_map_service_to_pipe(struct ath11k_base *ab, u16 service_id,