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

From: Jeff Johnson

Date: Fri Jul 24 2026 - 22:13:35 EST


On 7/22/2026 6:38 PM, ZhaoJinming wrote:
> @@ -585,9 +598,7 @@ 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,
> @@ -600,11 +611,23 @@ static int ath11k_ahb_config_ext_irq(struct ath11k_base *ab)

my review agent noted in inconsistency between this logic and the CE IRQ logic

just above here, after the platform_get_irq_byname() in the prior chunk, is
the assignment:
ab->irq_num[irq_idx] = irq;

that assignment takes place before the request_irq() function is called and
error checked

in the CE IRQ logic (see below) the assignment doesn't take place until after
the request_irq() function is called and error checked. so consider relocating
that assignment to after the if block below
> 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;
> }

so assign here?

> }
> +
> + 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;

note here the assignment takes place only when request_irq() is successful

> }
>
> /* 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,