Re: [PATCH v8] wifi: ath11k: fix resource leak on error in ext IRQ setup
Rameshkumar Sundaram <[email protected]> Thu, 30 Jul 2026 15:10:10 +0530
| Newsgroups | org.infradead.lists.ath11k,org.kernel.vger.linux-kernel,org.kernel.vger.linux-wireless |
|---|---|
| Message-ID | <[email protected]> |
On 7/30/2026 6:54 AM, Jeff Johnson wrote: > On 7/28/2026 7:00 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. >> > > Is there a reason you didn't carry forward Baochen's Reviewed-by: tag? > > He first gave the the tag for v4 > > In v5 the code was unchanged, and Baochen commented: > 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 <[email protected]> > > v6 was a trivial change in code. If you had applied his Reviewed-by then I > would have maintained it for v6. if it had been a bigger diff then it might be > ok to drop the Reviewed-by, but you'd then want to mention that in the commit > history > > v7 was unchanged from v6 so it should have had Baochen's RB. > code in v8 was unchanged from v7 so it should have had Baochen's RB. > > This is another good reason to use b4 for patch maintenance. > "b4 trailers -u" will accumulate any tags and update your patch. > > That is all advice for next time. > > You don't need to post another version to pick up the RB tag. > Running b4 in maintainer mode will pick up the one I just added above. > > Baochen, no need to re-review. > Ramesh, would like you to review. Looks like my RB was already present on v6. Nevertheless, I've added it to v8 as well. -- Ramesh