* [PATCH v5] wifi: ath11k: fix resource leak on error in ext IRQ setup
@ 2026-07-23 1:38 ZhaoJinming
2026-07-23 1:55 ` Baochen Qiang
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: ZhaoJinming @ 2026-07-23 1:38 UTC (permalink / raw)
To: Baochen Qiang, Jeff Johnson
Cc: linux-wireless, ath11k, linux-kernel, ZhaoJinming
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.
Signed-off-by: ZhaoJinming <zhaojinming@uniontech.com>
---
Changes in v5:
- Version bump only. Same content as v4, resent as a fresh thread per
maintainer request to avoid subject collision with the v4 reply.
- Reviewed-by: Baochen Qiang <baochen.qiang@oss.qualcomm.com>
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 f566d699d074..bdf53aa7ddf1 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,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)
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;
}
}
+
+ 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,
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH v5] wifi: ath11k: fix resource leak on error in ext IRQ setup
2026-07-23 1:38 [PATCH v5] wifi: ath11k: fix resource leak on error in ext IRQ setup ZhaoJinming
@ 2026-07-23 1:55 ` Baochen Qiang
2026-07-25 1:44 ` Jeff Johnson
2026-07-25 2:13 ` Jeff Johnson
2 siblings, 0 replies; 8+ messages in thread
From: Baochen Qiang @ 2026-07-23 1:55 UTC (permalink / raw)
To: ZhaoJinming, Jeff Johnson; +Cc: linux-wireless, ath11k, linux-kernel
On 7/23/2026 9:38 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.
>
> Signed-off-by: ZhaoJinming <zhaojinming@uniontech.com>
I have given my Reviewed-by: tag in the v4 review. Since v5 is identical to v4 you should
pick it.
once again:
Reviewed-by: Baochen Qiang <baochen.qiang@oss.qualcomm.com>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v5] wifi: ath11k: fix resource leak on error in ext IRQ setup
2026-07-23 1:38 [PATCH v5] wifi: ath11k: fix resource leak on error in ext IRQ setup ZhaoJinming
2026-07-23 1:55 ` Baochen Qiang
@ 2026-07-25 1:44 ` Jeff Johnson
2026-07-25 2:13 ` Jeff Johnson
2 siblings, 0 replies; 8+ messages in thread
From: Jeff Johnson @ 2026-07-25 1:44 UTC (permalink / raw)
To: ZhaoJinming, Baochen Qiang, Jeff Johnson
Cc: linux-wireless, ath11k, linux-kernel
On 7/22/2026 6:38 PM, ZhaoJinming wrote:
> +static void ath11k_ahb_free_ext_irq_grp(struct ath11k_base *ab,
> + struct ath11k_ext_irq_grp *irq_grp)
checkpatch says:
CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
No need to resend for this -- I can fix this when I pick up the patch
(but if you need to post a v6 then please fix)
/jeff
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v5] wifi: ath11k: fix resource leak on error in ext IRQ setup
2026-07-23 1:38 [PATCH v5] wifi: ath11k: fix resource leak on error in ext IRQ setup ZhaoJinming
2026-07-23 1:55 ` Baochen Qiang
2026-07-25 1:44 ` Jeff Johnson
@ 2026-07-25 2:13 ` Jeff Johnson
2026-07-27 6:55 ` [PATCH v6] " ZhaoJinming
2 siblings, 1 reply; 8+ messages in thread
From: Jeff Johnson @ 2026-07-25 2:13 UTC (permalink / raw)
To: ZhaoJinming, Baochen Qiang, Jeff Johnson
Cc: linux-wireless, ath11k, linux-kernel
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,
^ permalink raw reply [flat|nested] 8+ messages in thread* [PATCH v6] wifi: ath11k: fix resource leak on error in ext IRQ setup
2026-07-25 2:13 ` Jeff Johnson
@ 2026-07-27 6:55 ` ZhaoJinming
2026-07-27 11:03 ` Rameshkumar Sundaram
2026-07-27 15:47 ` Jeff Johnson
0 siblings, 2 replies; 8+ messages in thread
From: ZhaoJinming @ 2026-07-27 6:55 UTC (permalink / raw)
To: jeff.johnson, baochen.qiang
Cc: ath11k, jjohnson, linux-kernel, linux-wireless, zhaojinming
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.
Signed-off-by: ZhaoJinming <zhaojinming@uniontech.com>
---
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,
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH v6] wifi: ath11k: fix resource leak on error in ext IRQ setup
2026-07-27 6:55 ` [PATCH v6] " ZhaoJinming
@ 2026-07-27 11:03 ` Rameshkumar Sundaram
2026-07-27 15:47 ` Jeff Johnson
1 sibling, 0 replies; 8+ messages in thread
From: Rameshkumar Sundaram @ 2026-07-27 11:03 UTC (permalink / raw)
To: ZhaoJinming, jeff.johnson, baochen.qiang
Cc: ath11k, jjohnson, linux-kernel, linux-wireless
On 7/27/2026 12:25 PM, 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.
>
> Signed-off-by: ZhaoJinming <zhaojinming@uniontech.com>
Reviewed-by: Rameshkumar Sundaram <rameshkumar.sundaram@oss.qualcomm.com>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v6] wifi: ath11k: fix resource leak on error in ext IRQ setup
2026-07-27 6:55 ` [PATCH v6] " ZhaoJinming
2026-07-27 11:03 ` Rameshkumar Sundaram
@ 2026-07-27 15:47 ` Jeff Johnson
2026-07-28 1:45 ` 赵金明
1 sibling, 1 reply; 8+ messages in thread
From: Jeff Johnson @ 2026-07-27 15:47 UTC (permalink / raw)
To: ZhaoJinming, baochen.qiang; +Cc: ath11k, jjohnson, linux-kernel, linux-wireless
After v4 I reminded you:
>In the future please post new versions of patches as a separate thread.
Yet once again you sent a new patch as a threaded reply instead of as a new
thread.
:(
You are ruining your reputation.
/jeff
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v6] wifi: ath11k: fix resource leak on error in ext IRQ setup
2026-07-27 15:47 ` Jeff Johnson
@ 2026-07-28 1:45 ` 赵金明
0 siblings, 0 replies; 8+ messages in thread
From: 赵金明 @ 2026-07-28 1:45 UTC (permalink / raw)
To: Jeff Johnson, baochen.qiang
Cc: ath11k, jjohnson, linux-kernel, linux-wireless
Hi Jeff,
Thank you for the reminder, and I apologize for the continued misstep.
I do remember your previous instruction about starting new patch series as separate threads. The reason I sent this v6 as a threaded reply this time was to keep the review context easily accessible in my mail client ― I find it helpful to have the feedback and the new version grouped together for side?by?side reference during the review process.
That said, I fully understand that this convenience for myself creates inconvenience for others, especially for those who use threaded views to track patch flow. My intent was to send the final, ready?to?merge version as a new thread once the review on this round had settled ― but I should have followed the established process from the start.
To correct this, I will send v7 as a completely new thread, with the link to this discussion added at the end of the commit message for traceability.
Again, sorry for the noise, and thanks for your patience.
Best regards,
Zhao Jinming
>After v4 I reminded you:
>>In the future please post new versions of patches as a separate thread.
>
>Yet once again you sent a new patch as a threaded reply instead of as a new
>thread.
>
>:(
>
>You are ruining your reputation.
>
>/jeff
>
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2026-07-28 1:45 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-07-23 1:38 [PATCH v5] wifi: ath11k: fix resource leak on error in ext IRQ setup ZhaoJinming
2026-07-23 1:55 ` Baochen Qiang
2026-07-25 1:44 ` Jeff Johnson
2026-07-25 2:13 ` Jeff Johnson
2026-07-27 6:55 ` [PATCH v6] " ZhaoJinming
2026-07-27 11:03 ` Rameshkumar Sundaram
2026-07-27 15:47 ` Jeff Johnson
2026-07-28 1:45 ` 赵金明
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®