FAILED: patch "[PATCH] net/sched: act_ct: fix sk_buff leak when the header checks" failed to apply to 6.1-stable tree

PATCH New [email protected] 2 participants 5 messages Aug 17 – Aug 19

Subsystems (3)

Networking Stable branch Traffic control (tc)

lore

Share & tools

lore

[email protected]

author maintainer

2026-08-17 12:44 UTC #0 lore raw

The patch below does not apply to the 6.1-stable tree.
If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <[email protected]>.

To reproduce the conflict and resubmit, you may use the following commands:

git fetch https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/ linux-6.1.y
git checkout FETCH_HEAD
git cherry-pick -x 8a7ed561671aa6a911a2de99e59ef670a4d0b1df
# <resolve conflicts, build, test, etc.>
git commit -s
git send-email --to '<[email protected]>' --in-reply-to '2026081753-suffix-dill-686c@gregkh' --subject-prefix 'PATCH 6.1.y' 'HEAD^..'

Possible dependencies:

thanks,

greg k-h

------------------ original commit in Linus's tree ------------------

From 8a7ed561671aa6a911a2de99e59ef670a4d0b1df Mon Sep 17 00:00:00 2001
From: Hyunjung Ko <[email protected]>
Date: Thu, 6 Aug 2026 19:12:34 +0900
Subject: [PATCH] net/sched: act_ct: fix sk_buff leak when the header checks
reject a packet

tcf_ct_handle_fragments() runs its header sanity checks before handing anything to the defragmentation engine:

	if (family == NFPROTO_IPV4)
		err = tcf_ct_ipv4_is_fragment(skb, &frag);
	else
		err = tcf_ct_ipv6_is_fragment(skb, &frag);
	if (err || !frag)
		return err;

tcf_ct_ipv4_is_fragment() returns -EINVAL or -ENOMEM; tcf_ct_ipv6_is_fragment() adds -EPROTO when ipv6_find_hdr() fails. None of them frees or queues the skb, so on that path the caller still owns it.

tcf_ct_act() however funnels every non-zero return into the ownership-transfer exit:

	err = tcf_ct_handle_fragments(net, skb, family, p->zone, &defrag);
	if (err)
		goto out_frag;
	...
out_frag:
	if (err != -EINPROGRESS)
		tcf_action_inc_drop_qstats(&c->common);
	return TC_ACT_CONSUMED;

TC_ACT_CONSUMED means the action took ownership of the skb, so no caller frees it - sch_handle_ingress(), sch_handle_egress() and tcf_qevent_handle() all deliberately skip the free for that verdict. The skb is therefore orphaned: one sk_buff plus its data buffer is leaked per malformed packet, unbounded. Note the drop counter is already incremented for these errors, so the statistics claim a drop that never happens.

Three different ownership states reach out_frag: today - the skb may be queued by the defrag engine (-EINPROGRESS), already freed by nf_ct_handle_fragments(), or still owned by us. Tell the caller which of those it is, and free the packet ourselves in the last case, which restores the TC_ACT_SHOT behaviour that predated the Fixes: commit.

Reproduced on v7.2-rc6 with a 54-byte frame carrying a 40-byte IPv6 header with nexthdr = 0 (hop-by-hop) and nothing after it, on a clsact ingress chain with "action ct". kmemleak reports one leaked 232-byte skbuff_head_cache object plus its 704-byte data buffer per packet; with this patch it reports none.

Fixes 3f14b377d01d ("net/sched: act_ct: fix skb leak and crash on ooo frags")

Cc [email protected] # v6.8+

Signed-off-by Hyunjung Ko <[email protected]>

Acked-by Jamal Hadi Salim <[email protected]>

Link https://patch.msgid.link/[email protected] open here

Signed-off-by Jakub Kicinski <[email protected]>

net/sched/act_ct.c +25 -4

index be535a261fa0..e250969c84ac 100644
@@ -840,8 +840,15 @@ static int tcf_ct_ipv6_is_fragment(struct sk_buff *skb, bool *frag)
 	return 0;
 }
 
+/* On error, tells the caller whether it still owns @skb and must free it
+ * itself.  @skb is ours only when the header checks below reject the packet
+ * before it is handed to the defragmentation engine; once nf_ct_handle_
+ * fragments() has been called the skb is either queued (-EINPROGRESS) or has
+ * already been freed by it.
+ */
 static int tcf_ct_handle_fragments(struct net *net, struct sk_buff *skb,
-				   u8 family, u16 zone, bool *defrag)
+				   u8 family, u16 zone, bool *defrag,
+				   bool *skb_is_ours)
 {
 	enum ip_conntrack_info ctinfo;
 	struct tc_skb_cb cb;

@@ -859,8 +866,12 @@ static int tcf_ct_handle_fragments(struct net *net, struct sk_buff *skb,
 		err = tcf_ct_ipv4_is_fragment(skb, &frag);
 	else
 		err = tcf_ct_ipv6_is_fragment(skb, &frag);
-	if (err || !frag)
+	if (err) {
+		*skb_is_ours = true;
 		return err;
+	}
+	if (!frag)
+		return 0;
 
 	cb = *tc_skb_cb(skb);
 	err = nf_ct_handle_fragments(net, skb, zone, family, &proto, &cb.mru);

@@ -977,6 +988,7 @@ TC_INDIRECT_SCOPE int tcf_ct_act(struct sk_buff *skb, const struct tc_action *a,
 	int nh_ofs, err, retval;
 	struct tcf_ct_params *p;
 	bool add_helper = false;
+	bool skb_is_ours = false;
 	bool skip_add = false;
 	bool defrag = false;
 	struct nf_conn *ct;

@@ -1012,9 +1024,18 @@ TC_INDIRECT_SCOPE int tcf_ct_act(struct sk_buff *skb, const struct tc_action *a,
 	 */
 	nh_ofs = skb_network_offset(skb);
 	skb_pull_rcsum(skb, nh_ofs);
-	err = tcf_ct_handle_fragments(net, skb, family, p->zone, &defrag);
-	if (err)
+	err = tcf_ct_handle_fragments(net, skb, family, p->zone, &defrag,
+				      &skb_is_ours);
+	if (err) {
+		/* The skb is still ours only when the header checks rejected
+		 * it; returning TC_ACT_CONSUMED for such a packet would leak
+		 * it, since no caller frees an skb it was told it no longer
+		 * owns.
+		 */
+		if (skb_is_ours)
+			goto drop;
 		goto out_frag;
+	}
 
 	err = nf_ct_skb_network_trim(skb, family);
 	if (err)

Sasha Levin

maintainer

2026-08-19 19:08 UTC #1 lore raw

PATCH 1/4 openvswitch: use skb_ip_totlen in conntrack

From: Xin Long <[email protected]>

[ Upstream commit ec84c955a0d06cef31664bae328d94be7a3e2f03 ]

IPv4 GSO packets may get processed in ovs_skb_network_trim(), and we need to use skb_ip_totlen() to get iph totlen.

Signed-off-by Xin Long <[email protected]>

Reviewed-by Aaron Conole <[email protected]>

Reviewed-by Eric Dumazet <[email protected]>

Signed-off-by Jakub Kicinski <[email protected]>

Stable-dep-of: 8a7ed561671a ("net/sched: act_ct: fix sk_buff leak when the header checks reject a packet")

Signed-off-by Sasha Levin <[email protected]>

---
 net/openvswitch/conntrack.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

net/openvswitch/conntrack.c +1 -1

index e0ff87aaf979f..3bb361e70cdd4 100644
@@ -1282,7 +1282,7 @@ static int ovs_skb_network_trim(struct sk_buff *skb)
 
 	switch (skb->protocol) {
 	case htons(ETH_P_IP):
-		len = ntohs(ip_hdr(skb)->tot_len);
+		len = skb_ip_totlen(skb);
 		break;
 	case htons(ETH_P_IPV6):
 		len = sizeof(struct ipv6hdr)

Signature · 1 line

2.53.0

Sasha Levin

maintainer

2026-08-19 19:08 UTC #2 lore raw

PATCH 2/4 net: sched: use skb_ip_totlen and iph_totlen

From: Xin Long <[email protected]>

[ Upstream commit 043e397e48c58b4442ea5124dc1bdc95367a0a33 ]

There are 1 action and 1 qdisc that may process IPv4 TCP GSO packets and access iph->tot_len, replace them with skb_ip_totlen() and iph_totlen() accordingly.

Note that we don't need to replace the one in tcf_csum_ipv4(), as it will return for TCP GSO packets in tcf_csum_ipv4_tcp().

Signed-off-by Xin Long <[email protected]>

Reviewed-by Eric Dumazet <[email protected]>

Signed-off-by Jakub Kicinski <[email protected]>

Stable-dep-of: 8a7ed561671a ("net/sched: act_ct: fix sk_buff leak when the header checks reject a packet")

Signed-off-by Sasha Levin <[email protected]>

---
 net/sched/act_ct.c   | 2 +-
 net/sched/sch_cake.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

net/sched/act_ct.c +1 -1

index 651701a186fec..81c023f8433cc 100644
@@ -789,7 +789,7 @@ static int tcf_ct_skb_network_trim(struct sk_buff *skb, int family)
 
 	switch (family) {
 	case NFPROTO_IPV4:
-		len = ntohs(ip_hdr(skb)->tot_len);
+		len = skb_ip_totlen(skb);
 		break;
 	case NFPROTO_IPV6:
 		len = sizeof(struct ipv6hdr)

net/sched/sch_cake.c +1 -1

index e210a676dc340..91927d44d2d7c 100644
@@ -1269,7 +1269,7 @@ static struct sk_buff *cake_ack_filter(struct cake_sched_data *q,
 			    iph_check->daddr != iph->daddr)
 				continue;
 
-			seglen = ntohs(iph_check->tot_len) -
+			seglen = iph_totlen(skb, iph_check) -
 				       (4 * iph_check->ihl);
 		} else if (iph_check->version == 6) {
 			ipv6h = (struct ipv6hdr *)iph;

Signature · 1 line

2.53.0

Sasha Levin

maintainer

2026-08-19 19:08 UTC #3 lore raw

PATCH 3/4 openvswitch: move key and ovs_cb update out of handle_fragments

In reply to Sasha Levin

From: Xin Long <[email protected]>

[ Upstream commit 1b83bf4489cbc47d88976291cc967a17adb8e118 ]

This patch has no functional changes and just moves key and ovs_cb update out of handle_fragments, and skb_clear_hash() and skb->ignore_df change into handle_fragments(), to make it easier to move the duplicate code from handle_fragments() into nf_conntrack_ovs later.

Note that it changes to pass info->family to handle_fragments() instead of key for the packet type check, as info->family is set according to key->eth.type in ovs_ct_copy_action() when creating the action.

Signed-off-by Xin Long <[email protected]>

Reviewed-by Simon Horman <[email protected]>

Reviewed-by Aaron Conole <[email protected]>

Acked-by Florian Westphal <[email protected]>

Signed-off-by Jakub Kicinski <[email protected]>

Stable-dep-of: 8a7ed561671a ("net/sched: act_ct: fix sk_buff leak when the header checks reject a packet")

Signed-off-by Sasha Levin <[email protected]>

---
 net/openvswitch/conntrack.c | 37 +++++++++++++++++++++++++------------
 1 file changed, 25 insertions(+), 12 deletions(-)

net/openvswitch/conntrack.c +25 ~~-12~~

index 3bb361e70cdd4..0085e87f10f63 100644
@@ -496,13 +496,12 @@ static int ovs_ct_helper(struct sk_buff *skb, u16 proto)
 /* Returns 0 on success, -EINPROGRESS if 'skb' is stolen, or other nonzero
  * value if 'skb' is freed.
  */
-static int handle_fragments(struct net *net, struct sw_flow_key *key,
-			    u16 zone, struct sk_buff *skb)
+static int handle_fragments(struct net *net, struct sk_buff *skb,
+			    u16 zone, u8 family, u8 *proto, u16 *mru)
 {
-	struct ovs_skb_cb ovs_cb = *OVS_CB(skb);
 	int err;
 
-	if (key->eth.type == htons(ETH_P_IP)) {
+	if (family == NFPROTO_IPV4) {
 		enum ip_defrag_users user = IP_DEFRAG_CONNTRACK_IN + zone;
 
 		memset(IPCB(skb), 0, sizeof(struct inet_skb_parm));

@@ -510,9 +509,9 @@ static int handle_fragments(struct net *net, struct sw_flow_key *key,
 		if (err)
 			return err;
 
-		ovs_cb.mru = IPCB(skb)->frag_max_size;
+		*mru = IPCB(skb)->frag_max_size;
 #if IS_ENABLED(CONFIG_NF_DEFRAG_IPV6)
-	} else if (key->eth.type == htons(ETH_P_IPV6)) {
+	} else if (family == NFPROTO_IPV6) {
 		enum ip6_defrag_users user = IP6_DEFRAG_CONNTRACK_IN + zone;
 
 		memset(IP6CB(skb), 0, sizeof(struct inet6_skb_parm));

@@ -523,22 +522,35 @@ static int handle_fragments(struct net *net, struct sw_flow_key *key,
 			return err;
 		}
 
-		key->ip.proto = ipv6_hdr(skb)->nexthdr;
-		ovs_cb.mru = IP6CB(skb)->frag_max_size;
+		*proto = ipv6_hdr(skb)->nexthdr;
+		*mru = IP6CB(skb)->frag_max_size;
 #endif
 	} else {
 		kfree_skb(skb);
 		return -EPFNOSUPPORT;
 	}
 
+	skb_clear_hash(skb);
+	skb->ignore_df = 1;
+
+	return 0;
+}
+
+static int ovs_ct_handle_fragments(struct net *net, struct sw_flow_key *key,
+				   u16 zone, int family, struct sk_buff *skb)
+{
+	struct ovs_skb_cb ovs_cb = *OVS_CB(skb);
+	int err;
+
+	err = handle_fragments(net, skb, zone, family, &key->ip.proto, &ovs_cb.mru);
+	if (err)
+		return err;
+
 	/* The key extracted from the fragment that completed this datagram
 	 * likely didn't have an L4 header, so regenerate it.
 	 */
 	ovs_flow_key_update_l3l4(skb, key);
-
 	key->ip.frag = OVS_FRAG_TYPE_NONE;
-	skb_clear_hash(skb);
-	skb->ignore_df = 1;
 	*OVS_CB(skb) = ovs_cb;
 
 	return 0;

@@ -1318,7 +1330,8 @@ int ovs_ct_execute(struct net *net, struct sk_buff *skb,
 		return err;
 
 	if (key->ip.frag != OVS_FRAG_TYPE_NONE) {
-		err = handle_fragments(net, key, info->zone.id, skb);
+		err = ovs_ct_handle_fragments(net, key, info->zone.id,
+					      info->family, skb);
 		if (err)
 			return err;
 	}

Signature · 1 line

2.53.0

Sasha Levin

maintainer

2026-08-19 19:08 UTC #4 lore raw

PATCH 4/4 net/sched: act_ct: fix sk_buff leak when the header checks reject a packet

In reply to Sasha Levin

From: Hyunjung Ko <[email protected]>

[ Upstream commit 8a7ed561671aa6a911a2de99e59ef670a4d0b1df ]

tcf_ct_handle_fragments() runs its header sanity checks before handing anything to the defragmentation engine:

	if (family == NFPROTO_IPV4)
		err = tcf_ct_ipv4_is_fragment(skb, &frag);
	else
		err = tcf_ct_ipv6_is_fragment(skb, &frag);
	if (err || !frag)
		return err;

tcf_ct_act() however funnels every non-zero return into the ownership-transfer exit:

	err = tcf_ct_handle_fragments(net, skb, family, p->zone, &defrag);
	if (err)
		goto out_frag;
	...
out_frag:
	if (err != -EINPROGRESS)
		tcf_action_inc_drop_qstats(&c->common);
	return TC_ACT_CONSUMED;

Fixes 3f14b377d01d ("net/sched: act_ct: fix skb leak and crash on ooo frags")

Cc [email protected] # v6.8+

Signed-off-by Hyunjung Ko <[email protected]>

Acked-by Jamal Hadi Salim <[email protected]>

Link https://patch.msgid.link/[email protected] open here

Signed-off-by Jakub Kicinski <[email protected]>

[ Dropped upstream's `bool add_helper = false;` context line, absent on 6.1, keeping only the new `bool skb_is_ours = false;` declaration. ]

Signed-off-by Sasha Levin <[email protected]>

---
 net/sched/act_ct.c | 29 +++++++++++++++++++++++++----
 1 file changed, 25 insertions(+), 4 deletions(-)

net/sched/act_ct.c +25 -4

index 81c023f8433cc..f6b2f693e21d5 100644
@@ -854,8 +854,15 @@ static int tcf_ct_ipv6_is_fragment(struct sk_buff *skb, bool *frag)
 	return 0;
 }
 
+/* On error, tells the caller whether it still owns @skb and must free it
+ * itself.  @skb is ours only when the header checks below reject the packet
+ * before it is handed to the defragmentation engine; once nf_ct_handle_
+ * fragments() has been called the skb is either queued (-EINPROGRESS) or has
+ * already been freed by it.
+ */
 static int tcf_ct_handle_fragments(struct net *net, struct sk_buff *skb,
-				   u8 family, u16 zone, bool *defrag)
+				   u8 family, u16 zone, bool *defrag,
+				   bool *skb_is_ours)
 {
 	enum ip_conntrack_info ctinfo;
 	struct nf_conn *ct;

@@ -872,8 +879,12 @@ static int tcf_ct_handle_fragments(struct net *net, struct sk_buff *skb,
 		err = tcf_ct_ipv4_is_fragment(skb, &frag);
 	else
 		err = tcf_ct_ipv6_is_fragment(skb, &frag);
-	if (err || !frag)
+	if (err) {
+		*skb_is_ours = true;
 		return err;
+	}
+	if (!frag)
+		return 0;
 
 	mru = tc_skb_cb(skb)->mru;

@@ -1124,6 +1135,7 @@ static int tcf_ct_act(struct sk_buff *skb, const struct tc_action *a,
 	struct nf_hook_state state;
 	int nh_ofs, err, retval;
 	struct tcf_ct_params *p;
+	bool skb_is_ours = false;
 	bool skip_add = false;
 	bool defrag = false;
 	struct nf_conn *ct;

@@ -1160,9 +1172,18 @@ static int tcf_ct_act(struct sk_buff *skb, const struct tc_action *a,
 	 */
 	nh_ofs = skb_network_offset(skb);
 	skb_pull_rcsum(skb, nh_ofs);
-	err = tcf_ct_handle_fragments(net, skb, family, p->zone, &defrag);
-	if (err)
+	err = tcf_ct_handle_fragments(net, skb, family, p->zone, &defrag,
+				      &skb_is_ours);
+	if (err) {
+		/* The skb is still ours only when the header checks rejected
+		 * it; returning TC_ACT_CONSUMED for such a packet would leak
+		 * it, since no caller frees an skb it was told it no longer
+		 * owns.
+		 */
+		if (skb_is_ours)
+			goto drop;
 		goto out_frag;
+	}
 
 	err = tcf_ct_skb_network_trim(skb, family);
 	if (err)

Signature · 1 line

2.53.0