[PATCH v2 1/1] xfrm: nat_keepalive: avoid double free on send error

[PATCH v2 1/1] xfrm: nat_keepalive: avoid double free on send error

[Ren Wei]

From: Qianyu Luo <qianyuluo3@gmail.com>

nat_keepalive_send() frees the keepalive skb whenever the IPv4 or IPv6
send helper reports an error.

That cleanup is only correct before the skb is handed to the output
path. Once ip_build_and_send_pkt() or ip6_xmit() takes ownership, the
networking stack may already have consumed the skb before returning an
error, so freeing it again is unsafe.

Handle the pre-handoff failure cases inside nat_keepalive_send_ipv4()
and nat_keepalive_send_ipv6(), where the caller still owns the skb, and
keep nat_keepalive_send() responsible only for family dispatch and the
unsupported-family cleanup path.

Fixes: f531d13bdfe3 ("xfrm: support sending NAT keepalives in ESP in UDP states")
Cc: stable@vger.kernel.org
Reported-by: Yuan Tan <yuantan098@gmail.com>
Reported-by: Xin Liu <bird@lzu.edu.cn>
Signed-off-by: Qianyu Luo <qianyuluo3@gmail.com>
Signed-off-by: Ren Wei <n05ec@lzu.edu.cn>
---
Changes in v2:
- move kfree_skb() after local_unlock_nested_bh() in the IPv6 dst-lookup
  failure path as suggested in review
- rebase onto latest netdev/net

Link: https://lore.kernel.org/all/46eb334399ce0e25e0897b42f21020541d159300.1781788385.git.qianyuluo3@gmail.com/

 net/xfrm/xfrm_nat_keepalive.c | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/net/xfrm/xfrm_nat_keepalive.c b/net/xfrm/xfrm_nat_keepalive.c
index 458931062a04..eb1b6f67739e 100644
--- a/net/xfrm/xfrm_nat_keepalive.c
+++ b/net/xfrm/xfrm_nat_keepalive.c
@@ -55,8 +55,10 @@ static int nat_keepalive_send_ipv4(struct sk_buff *skb,
 			   ka->encap_sport, sock_net_uid(net, NULL));
 
 	rt = ip_route_output_key(net, &fl4);
-	if (IS_ERR(rt))
+	if (IS_ERR(rt)) {
+		kfree_skb(skb);
 		return PTR_ERR(rt);
+	}
 
 	skb_dst_set(skb, &rt->dst);
 
@@ -101,6 +103,7 @@ static int nat_keepalive_send_ipv6(struct sk_buff *skb,
 	dst = ip6_dst_lookup_flow(net, sk, &fl6, NULL);
 	if (IS_ERR(dst)) {
 		local_unlock_nested_bh(&nat_keepalive_sk_ipv6.bh_lock);
+		kfree_skb(skb);
 		return PTR_ERR(dst);
 	}
 
@@ -118,7 +121,6 @@ static void nat_keepalive_send(struct nat_keepalive *ka)
 					sizeof(struct ipv6hdr)) +
 				    sizeof(struct udphdr);
 	const u8 nat_ka_payload = 0xFF;
-	int err = -EAFNOSUPPORT;
 	struct sk_buff *skb;
 	struct udphdr *uh;
 
@@ -140,16 +142,17 @@ static void nat_keepalive_send(struct nat_keepalive *ka)
 
 	switch (ka->family) {
 	case AF_INET:
-		err = nat_keepalive_send_ipv4(skb, ka);
+		nat_keepalive_send_ipv4(skb, ka);
 		break;
 #if IS_ENABLED(CONFIG_IPV6)
 	case AF_INET6:
-		err = nat_keepalive_send_ipv6(skb, ka, uh);
+		nat_keepalive_send_ipv6(skb, ka, uh);
 		break;
 #endif
-	}
-	if (err)
+	default:
 		kfree_skb(skb);
+		break;
+	}
 }
 
 struct nat_keepalive_work_ctx {
-- 
2.43.7

Re: [PATCH v2 1/1] xfrm: nat_keepalive: avoid double free on send error

[Eyal Birger]

On Wed, Jun 24, 2026 at 10:55 PM Ren Wei <n05ec@lzu.edu.cn> wrote:
>
> From: Qianyu Luo <qianyuluo3@gmail.com>
>
> nat_keepalive_send() frees the keepalive skb whenever the IPv4 or IPv6
> send helper reports an error.
>
> That cleanup is only correct before the skb is handed to the output
> path. Once ip_build_and_send_pkt() or ip6_xmit() takes ownership, the
> networking stack may already have consumed the skb before returning an
> error, so freeing it again is unsafe.
>
> Handle the pre-handoff failure cases inside nat_keepalive_send_ipv4()
> and nat_keepalive_send_ipv6(), where the caller still owns the skb, and
> keep nat_keepalive_send() responsible only for family dispatch and the
> unsupported-family cleanup path.
>
> Fixes: f531d13bdfe3 ("xfrm: support sending NAT keepalives in ESP in UDP states")

Thanks for the fix!

Reviewed-by: Eyal Birger <eyal.birger@gmail.com>

Re: [PATCH v2 1/1] xfrm: nat_keepalive: avoid double free on send error

[Steffen Klassert]

On Sun, Jun 28, 2026 at 06:42:44AM -0700, Eyal Birger wrote:
> On Wed, Jun 24, 2026 at 10:55 PM Ren Wei <n05ec@lzu.edu.cn> wrote:
> >
> > From: Qianyu Luo <qianyuluo3@gmail.com>
> >
> > nat_keepalive_send() frees the keepalive skb whenever the IPv4 or IPv6
> > send helper reports an error.
> >
> > That cleanup is only correct before the skb is handed to the output
> > path. Once ip_build_and_send_pkt() or ip6_xmit() takes ownership, the
> > networking stack may already have consumed the skb before returning an
> > error, so freeing it again is unsafe.
> >
> > Handle the pre-handoff failure cases inside nat_keepalive_send_ipv4()
> > and nat_keepalive_send_ipv6(), where the caller still owns the skb, and
> > keep nat_keepalive_send() responsible only for family dispatch and the
> > unsupported-family cleanup path.
> >
> > Fixes: f531d13bdfe3 ("xfrm: support sending NAT keepalives in ESP in UDP states")
> 
> Thanks for the fix!
> 
> Reviewed-by: Eyal Birger <eyal.birger@gmail.com>

Applied, thanks everyone!