public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] xfrm: fix memory leak in xfrm_add_policy()
@ 2026-04-12  2:08 Deepanshu Kartikey
  2026-04-13 13:32 ` Sabrina Dubroca
  0 siblings, 1 reply; 5+ messages in thread
From: Deepanshu Kartikey @ 2026-04-12  2:08 UTC (permalink / raw)
  To: steffen.klassert, herbert, davem, edumazet, kuba, pabeni, horms
  Cc: leon, netdev, linux-kernel, Deepanshu Kartikey,
	syzbot+901d48e0b95aed4a2548

When xfrm_policy_insert() fails, the error path performs manual
cleanup by calling xfrm_dev_policy_free(), security_xfrm_policy_free()
and kfree() directly. This is incorrect because xfrm_policy_destroy()
already handles all of these, causing a memory leak detected by
kmemleak.

Replace the open-coded cleanup with xfrm_policy_destroy(), consistent
with the error handling in xfrm_policy_construct(). The walk.dead
flag must be set before calling xfrm_policy_destroy() as it requires
it via BUG_ON(!policy->walk.dead).

Fixes: 94b95dfaa814 ("xfrm: release all offloaded policy memory")
Reported-by: syzbot+901d48e0b95aed4a2548@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=901d48e0b95aed4a2548
Tested-by: syzbot+901d48e0b95aed4a2548@syzkaller.appspotmail.com
Signed-off-by: Deepanshu Kartikey <kartikey406@gmail.com>
---
 net/xfrm/xfrm_user.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/net/xfrm/xfrm_user.c b/net/xfrm/xfrm_user.c
index d56450f61669..ae144d1e4a65 100644
--- a/net/xfrm/xfrm_user.c
+++ b/net/xfrm/xfrm_user.c
@@ -2267,9 +2267,8 @@ static int xfrm_add_policy(struct sk_buff *skb, struct nlmsghdr *nlh,
 
 	if (err) {
 		xfrm_dev_policy_delete(xp);
-		xfrm_dev_policy_free(xp);
-		security_xfrm_policy_free(xp->security);
-		kfree(xp);
+		xp->walk.dead = 1;
+		xfrm_policy_destroy(xp);
 		return err;
 	}
 
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] xfrm: fix memory leak in xfrm_add_policy()
  2026-04-12  2:08 [PATCH] xfrm: fix memory leak in xfrm_add_policy() Deepanshu Kartikey
@ 2026-04-13 13:32 ` Sabrina Dubroca
  2026-04-13 14:28   ` Deepanshu Kartikey
  0 siblings, 1 reply; 5+ messages in thread
From: Sabrina Dubroca @ 2026-04-13 13:32 UTC (permalink / raw)
  To: Deepanshu Kartikey
  Cc: steffen.klassert, herbert, davem, edumazet, kuba, pabeni, horms,
	leon, netdev, linux-kernel, syzbot+901d48e0b95aed4a2548

2026-04-12, 07:38:09 +0530, Deepanshu Kartikey wrote:
> When xfrm_policy_insert() fails, the error path performs manual
> cleanup by calling xfrm_dev_policy_free(), security_xfrm_policy_free()
> and kfree() directly. This is incorrect because xfrm_policy_destroy()
> already handles all of these, causing a memory leak detected by
> kmemleak.

What is missing in the current code? "we have a better way to do this"
is not a bugfix, it's a clean up. The kmemleak report says that we're
leaking the xfrm_policy struct on this codepath, which doesn't make
sense, that's covered by the existing kfree(xp).

Also, please use "PATCH ipsec" for fixes to net/xfrm and the rest of
the IPsec implementation.

-- 
Sabrina

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] xfrm: fix memory leak in xfrm_add_policy()
  2026-04-13 13:32 ` Sabrina Dubroca
@ 2026-04-13 14:28   ` Deepanshu Kartikey
  2026-04-13 14:34     ` Sabrina Dubroca
  0 siblings, 1 reply; 5+ messages in thread
From: Deepanshu Kartikey @ 2026-04-13 14:28 UTC (permalink / raw)
  To: Sabrina Dubroca
  Cc: steffen.klassert, herbert, davem, edumazet, kuba, pabeni, horms,
	leon, netdev, linux-kernel, syzbot+901d48e0b95aed4a2548

On Mon, Apr 13, 2026 at 7:03 PM Sabrina Dubroca <sd@queasysnail.net> wrote:
>

> What is missing in the current code? "we have a better way to do this"
> is not a bugfix, it's a clean up. The kmemleak report says that we're
> leaking the xfrm_policy struct on this codepath, which doesn't make
> sense, that's covered by the existing kfree(xp).
>
> Also, please use "PATCH ipsec" for fixes to net/xfrm and the rest of
> the IPsec implementation.
>
> --
> Sabrina

Hi Sabrina,

Thanks for the review!

You are right, the existing kfree(xp) already covers the struct
itself, so my commit message was incorrect in claiming a memory
leak fix. I will resend this as a cleanup patch to replace the
open-coded manual cleanup with xfrm_policy_destroy(), which is
more consistent with xfrm_policy_construct() error handling.

I am also separately investigating the root cause of the actual
kmemleak report and will send a proper fix once identified.

Sorry for the noise and thanks again for the guidance.

Deepanshu

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] xfrm: fix memory leak in xfrm_add_policy()
  2026-04-13 14:28   ` Deepanshu Kartikey
@ 2026-04-13 14:34     ` Sabrina Dubroca
  2026-04-14  2:12       ` Deepanshu Kartikey
  0 siblings, 1 reply; 5+ messages in thread
From: Sabrina Dubroca @ 2026-04-13 14:34 UTC (permalink / raw)
  To: Deepanshu Kartikey
  Cc: steffen.klassert, herbert, davem, edumazet, kuba, pabeni, horms,
	leon, netdev, linux-kernel, syzbot+901d48e0b95aed4a2548

2026-04-13, 19:58:53 +0530, Deepanshu Kartikey wrote:
> On Mon, Apr 13, 2026 at 7:03 PM Sabrina Dubroca <sd@queasysnail.net> wrote:
> >
> 
> > What is missing in the current code? "we have a better way to do this"
> > is not a bugfix, it's a clean up. The kmemleak report says that we're
> > leaking the xfrm_policy struct on this codepath, which doesn't make
> > sense, that's covered by the existing kfree(xp).
> >
> > Also, please use "PATCH ipsec" for fixes to net/xfrm and the rest of
> > the IPsec implementation.
> >
> > --
> > Sabrina
> 
> Hi Sabrina,
> 
> Thanks for the review!
> 
> You are right, the existing kfree(xp) already covers the struct
> itself, so my commit message was incorrect in claiming a memory
> leak fix. I will resend this as a cleanup patch to replace the
> open-coded manual cleanup with xfrm_policy_destroy(), which is
> more consistent with xfrm_policy_construct() error handling.

Ok. Then you should wait 2 weeks until the merge window is over:
https://lore.kernel.org/netdev/20260412142250.131bf997@kernel.org/

and use "[PATCH ipsec-next]" as prefix for the cleanup patch (+ drop
the syzbot references).

> I am also separately investigating the root cause of the actual
> kmemleak report and will send a proper fix once identified.

Ok, thanks.

-- 
Sabrina

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] xfrm: fix memory leak in xfrm_add_policy()
  2026-04-13 14:34     ` Sabrina Dubroca
@ 2026-04-14  2:12       ` Deepanshu Kartikey
  0 siblings, 0 replies; 5+ messages in thread
From: Deepanshu Kartikey @ 2026-04-14  2:12 UTC (permalink / raw)
  To: Sabrina Dubroca
  Cc: steffen.klassert, herbert, davem, edumazet, kuba, pabeni, horms,
	leon, netdev, linux-kernel, syzbot+901d48e0b95aed4a2548

On Mon, Apr 13, 2026 at 8:04 PM Sabrina Dubroca <sd@queasysnail.net> wrote:
>
> Ok. Then you should wait 2 weeks until the merge window is over:
> https://lore.kernel.org/netdev/20260412142250.131bf997@kernel.org/
>
> and use "[PATCH ipsec-next]" as prefix for the cleanup patch (+ drop
> the syzbot references).
>

Hi Sabrina,

Thanks for the guidance.  I have submitted patch v3.

Thanks

Deepanshu

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-04-14  2:12 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-12  2:08 [PATCH] xfrm: fix memory leak in xfrm_add_policy() Deepanshu Kartikey
2026-04-13 13:32 ` Sabrina Dubroca
2026-04-13 14:28   ` Deepanshu Kartikey
2026-04-13 14:34     ` Sabrina Dubroca
2026-04-14  2:12       ` Deepanshu Kartikey

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox