Re: [PATCH] net: ipa: Fix double free of the interrupt structure on config error
flat view
From: Alex Elder <hidden>
Date: 2026-10-28 16:50:49
Also in:
lkml
On 10/28/26 5:23 AM, Thomas Fourier wrote:
Since ipa_interrupt_init() allocates struct ipa_interrupt in ipa_probe(),
the structure is owned by ipa_probe() and released with
ipa_interrupt_exit() on its error paths and in ipa_remove(). The error
path of ipa_interrupt_config() still frees it with kfree(). If
ipa_interrupt_config() fails (called from ipa_config() via ipa_setup()),
ipa_probe() unwinds through err_interrupt_exit, which frees the same
structure again.
Drop the kfree() from ipa_interrupt_config(); it should only undo what it
set up itself.
Fixes: ad1be80d7582 ("net: ipa: introduce ipa_interrupt_init()")
Signed-off-by: Thomas Fourier <redacted>
This is a good catch, something that really should have been caught
when that commit was reviewed (even by me...).
There is another problem I now spotted, and I'd like to have it
corrected when you submit version 2, because it's really related
to the original bug.
At the end of ipa_interrupt_config(), we (still) see this line:
ipa->interrupt = interrupt;
That now happens in ipa_probe() (which calls ipa_interrupt_init()),
so shouldn't be repeated in ipa_interrupt_config(). Similarly,
ipa_interrupt_deconfig() does this:
ipa->interrupt = NULL;
That's meant to be the inverse of what was being done in the
config function, but it should no longer be done there.
(There is no corresponding ipa->interrupt = NULL assignment
in ipa_interrupt_exit(), nor in ipa_remove(). There is no
real need, because the ipa structure will never be referenced
again after that.)
Thanks.
-Alex
quoted hunk ↗ jump to hunk
--- drivers/net/ipa/ipa_interrupt.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-)diff --git a/drivers/net/ipa/ipa_interrupt.c b/drivers/net/ipa/ipa_interrupt.c index 131b9cc94201..17cefd6a0b18 100644 --- a/drivers/net/ipa/ipa_interrupt.c +++ b/drivers/net/ipa/ipa_interrupt.c@@ -260,10 +260,8 @@ int ipa_interrupt_config(struct ipa *ipa) interrupt->enabled = 0; interrupt->suspend_enabled = bitmap_zalloc(ipa->endpoint_count, GFP_KERNEL); - if (!interrupt->suspend_enabled) { - ret = -ENOMEM; - goto err_kfree; - } + if (!interrupt->suspend_enabled) + return -ENOMEM; /* Disable IPA interrupt types */ reg = ipa_reg(ipa, IPA_IRQ_EN);@@ -299,8 +297,6 @@ int ipa_interrupt_config(struct ipa *ipa) free_irq(interrupt->irq, interrupt); err_free_bitmap: bitmap_free(interrupt->suspend_enabled); -err_kfree: - kfree(interrupt); return ret; }