Skip to contentSkip to conversation

FAILED: patch "[PATCH] net/sched: act_ct: fix helper UAF due to extensions realloc" failed to apply to 6.18-stable tree

PATCH New [email protected] 2 participants 5 messages Sep 29 – Sep 30

Subsystems (3)
lore
Share & tools
lore

[email protected]

author maintainer
#0 lore raw

The patch below does not apply to the 6.18-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.18.y
git checkout FETCH_HEAD
git cherry-pick -x dad19b59da050cb60d3f7023dac2a042a84bf0bd
# <resolve conflicts, build, test, etc.>
git commit -s
git send-email --to '<[email protected]>' --in-reply-to '2026092923-during-buffalo-09b7@gregkh' --subject-prefix 'PATCH 6.18.y' 'HEAD^..'

Possible dependencies:

thanks,

greg k-h

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

From dad19b59da050cb60d3f7023dac2a042a84bf0bd Mon Sep 17 00:00:00 2001
From: Ilya Maximets <[email protected]>
Date: Mon, 21 Sep 2026 16:55:48 +0200
Subject: [PATCH] net/sched: act_ct: fix helper UAF due to extensions realloc

While calling the helpers, a raw pointer to the extensions area is wired into expectations list:

-> nf_ct_helper()
-> helper->help()

    -> nf_ct_expect_related_report()
     -> nf_ct_expect_insert()
      -> hlist_add_head_rcu(&exp->lnode, &master_help->expectations)

In case the connection is not confirmed yet, more extensions can be added afterwards with *_ext_add() calls reallocating the extension space and leaving the now invalid pointer in the expectations list that is later accessed while removing the expectation.

Make sure that helpers are called at the end after all the other extensions are already added.

Note that the helper rejection now leaves the mark and labels set, but that's not different from how the NAT was handled before or how the mark and the labels were handled on confirmation failure. And there are no atomicity guarantees provided by the API anyway.

net/sched/act_ct.c +12 -6
index f62051ec9d57..411e3dd92d07 100644
@@ -1102,6 +1102,18 @@ TC_INDIRECT_SCOPE int tcf_ct_act(struct sk_buff *skb, const struct tc_action *a,
 		}
 	}
 
+	if (commit) {
+		tcf_ct_act_set_mark(ct, p->mark, p->mark_mask);
+		tcf_ct_act_set_labels(ct, p->labels, p->labels_mask);
+
+		if (!nf_ct_is_confirmed(ct))
+			nf_conn_act_ct_ext_add(skb, ct, ctinfo);
+	}
+
+	/* Run helpers for the connection if nf_conntrack_in() was executed
+	 * or if we're about to commit.  This has to be done after all the
+	 * extensions are already added.
+	 */
 	if (nf_ct_is_confirmed(ct) ? (!cached && !skip_add) : commit) {
 		err = nf_ct_helper(skb, ct, ctinfo, family);
 		if (err != NF_ACCEPT)
@@ -1109,12 +1121,6 @@ TC_INDIRECT_SCOPE int tcf_ct_act(struct sk_buff *skb, const struct tc_action *a,
 	}
 
 	if (commit) {
-		tcf_ct_act_set_mark(ct, p->mark, p->mark_mask);
-		tcf_ct_act_set_labels(ct, p->labels, p->labels_mask);
-
-		if (!nf_ct_is_confirmed(ct))
-			nf_conn_act_ct_ext_add(skb, ct, ctinfo);
-
 		/* This will take care of sending queued events
 		 * even if the connection is already confirmed.
 		 */
maintainer
#1 lore raw

PATCH 1/2 net: openvswitch: conntrack: avoid modifying shared unconfirmed ct entry

From: Ilya Maximets <[email protected]>

[ Upstream commit 26b2bd70d22457556e2fa01cbf1192cb1a94d619 ]

In a case where skb with an unconfirmed ct entry gets cloned, we may end up committing both but with different sets of extensions.

The series of events:

1. The first clone wants to commit and runs the helpers wiring up the extension pointer into the expectation list.
2. Then it looses the confirmation keeping the entry unconfirmed.
3. Second clone now wants to commit labels and adds the new extension for that breaking the pointer in the expectation list causing UAF on the destruction path later.

While this is possible to trigger, there should be no practical network pipeline where committing both clones without modifications into the same zone is needed. So, let's just reset the entry in case for some reason we got an skb with a shared one during commit. This doesn't affect any known use cases, but avoids any potential problems with sharing and modification of the unconfirmed ct entry.

The fixes tag points to the introduction of helpers, since that's the main UAF trigger for the sharing.

Stable-dep-of: dad19b59da05 ("net/sched: act_ct: fix helper UAF due to extensions realloc")

---
 include/net/netfilter/nf_conntrack.h |  5 +++++
 net/openvswitch/conntrack.c          | 12 ++++++++++++
 2 files changed, 17 insertions(+)
include/net/netfilter/nf_conntrack.h +5 -0
index aa0a7c82199ec..7e26611eb3c4a 100644
@@ -184,6 +184,11 @@ static inline void nf_ct_put(struct nf_conn *ct)
 		nf_ct_destroy(&ct->ct_general);
 }
 
+static inline bool nf_ct_shared(const struct nf_conn *ct)
+{
+	return refcount_read(&ct->ct_general.use) > 1;
+}
+
 /* load module; enable/disable conntrack in this namespace */
 int nf_ct_netns_get(struct net *net, u8 nfproto);
 void nf_ct_netns_put(struct net *net, u8 nfproto);
net/openvswitch/conntrack.c +12 -0
index 89ef350ca91c4..a3d839099fdb3 100644
@@ -734,6 +734,18 @@ static int __ovs_ct_lookup(struct net *net, struct sw_flow_key *key,
 	enum ip_conntrack_info ctinfo;
 	struct nf_conn *ct;
 
+	/* If the ct entry is not confirmed and shared with some other skb,
+	 * e.g., a cloned one, we can't just modify it with the commit as we
+	 * must not modify the extension set.  Reset.
+	 */
+	if (cached && info->commit) {
+		ct = nf_ct_get(skb, &ctinfo);
+		if (ct && !nf_ct_is_confirmed(ct) && nf_ct_shared(ct)) {
+			nf_reset_ct(skb);
+			cached = false;
+		}
+	}
+
 	if (!cached) {
 		struct nf_hook_state state = {
 			.hook = NF_INET_PRE_ROUTING,
Signature · 1 line
2.53.0
maintainer
#2 lore raw

PATCH 2/2 net/sched: act_ct: fix helper UAF due to extensions realloc

From: Ilya Maximets <[email protected]>

[ Upstream commit dad19b59da050cb60d3f7023dac2a042a84bf0bd ]

While calling the helpers, a raw pointer to the extensions area is wired into expectations list:

-> nf_ct_helper()
-> helper->help()

    -> nf_ct_expect_related_report()
     -> nf_ct_expect_insert()
      -> hlist_add_head_rcu(&exp->lnode, &master_help->expectations)

In case the connection is not confirmed yet, more extensions can be added afterwards with *_ext_add() calls reallocating the extension space and leaving the now invalid pointer in the expectations list that is later accessed while removing the expectation.

Make sure that helpers are called at the end after all the other extensions are already added.

Note that the helper rejection now leaves the mark and labels set, but that's not different from how the NAT was handled before or how the mark and the labels were handled on confirmation failure. And there are no atomicity guarantees provided by the API anyway.

Fixes a21b06e73191 ("net: sched: add helper support in act_ct")
[ Preserved the existing add_helper condition that upstream had removed. ]
---
 net/sched/act_ct.c | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)
net/sched/act_ct.c +12 -6
index cbac27ba76afc..c6085b06ec279 100644
@@ -1088,19 +1088,25 @@ TC_INDIRECT_SCOPE int tcf_ct_act(struct sk_buff *skb, const struct tc_action *a,
 		}
 	}
 
-	if (nf_ct_is_confirmed(ct) ? ((!cached && !skip_add) || add_helper) : commit) {
-		err = nf_ct_helper(skb, ct, ctinfo, family);
-		if (err != NF_ACCEPT)
-			goto nf_error;
-	}
-
 	if (commit) {
 		tcf_ct_act_set_mark(ct, p->mark, p->mark_mask);
 		tcf_ct_act_set_labels(ct, p->labels, p->labels_mask);
 
 		if (!nf_ct_is_confirmed(ct))
 			nf_conn_act_ct_ext_add(skb, ct, ctinfo);
+	}
 
+	/* Run helpers for the connection if nf_conntrack_in() was executed
+	 * or if we're about to commit.  This has to be done after all the
+	 * extensions are already added.
+	 */
+	if (nf_ct_is_confirmed(ct) ? ((!cached && !skip_add) || add_helper) : commit) {
+		err = nf_ct_helper(skb, ct, ctinfo, family);
+		if (err != NF_ACCEPT)
+			goto nf_error;
+	}
+
+	if (commit) {
 		/* This will take care of sending queued events
 		 * even if the connection is already confirmed.
 		 */
Signature · 1 line
2.53.0
maintainer
#3 lore raw

PATCH 1/2 net: openvswitch: conntrack: avoid modifying shared unconfirmed ct entry

In reply to [email protected]

From: Ilya Maximets <[email protected]>

[ Upstream commit 26b2bd70d22457556e2fa01cbf1192cb1a94d619 ]

In a case where skb with an unconfirmed ct entry gets cloned, we may end up committing both but with different sets of extensions.

The series of events:

1. The first clone wants to commit and runs the helpers wiring up the extension pointer into the expectation list.
2. Then it looses the confirmation keeping the entry unconfirmed.
3. Second clone now wants to commit labels and adds the new extension for that breaking the pointer in the expectation list causing UAF on the destruction path later.

While this is possible to trigger, there should be no practical network pipeline where committing both clones without modifications into the same zone is needed. So, let's just reset the entry in case for some reason we got an skb with a shared one during commit. This doesn't affect any known use cases, but avoids any potential problems with sharing and modification of the unconfirmed ct entry.

The fixes tag points to the introduction of helpers, since that's the main UAF trigger for the sharing.

Stable-dep-of: dad19b59da05 ("net/sched: act_ct: fix helper UAF due to extensions realloc")

---
 include/net/netfilter/nf_conntrack.h |  5 +++++
 net/openvswitch/conntrack.c          | 12 ++++++++++++
 2 files changed, 17 insertions(+)
include/net/netfilter/nf_conntrack.h +5 -0
index aa0a7c82199ec..7e26611eb3c4a 100644
@@ -184,6 +184,11 @@ static inline void nf_ct_put(struct nf_conn *ct)
 		nf_ct_destroy(&ct->ct_general);
 }
 
+static inline bool nf_ct_shared(const struct nf_conn *ct)
+{
+	return refcount_read(&ct->ct_general.use) > 1;
+}
+
 /* load module; enable/disable conntrack in this namespace */
 int nf_ct_netns_get(struct net *net, u8 nfproto);
 void nf_ct_netns_put(struct net *net, u8 nfproto);
net/openvswitch/conntrack.c +12 -0
index 89ef350ca91c4..a3d839099fdb3 100644
@@ -734,6 +734,18 @@ static int __ovs_ct_lookup(struct net *net, struct sw_flow_key *key,
 	enum ip_conntrack_info ctinfo;
 	struct nf_conn *ct;
 
+	/* If the ct entry is not confirmed and shared with some other skb,
+	 * e.g., a cloned one, we can't just modify it with the commit as we
+	 * must not modify the extension set.  Reset.
+	 */
+	if (cached && info->commit) {
+		ct = nf_ct_get(skb, &ctinfo);
+		if (ct && !nf_ct_is_confirmed(ct) && nf_ct_shared(ct)) {
+			nf_reset_ct(skb);
+			cached = false;
+		}
+	}
+
 	if (!cached) {
 		struct nf_hook_state state = {
 			.hook = NF_INET_PRE_ROUTING,
Signature · 1 line
2.53.0
maintainer
#4 lore raw

PATCH 2/2 net/sched: act_ct: fix helper UAF due to extensions realloc

From: Ilya Maximets <[email protected]>

[ Upstream commit dad19b59da050cb60d3f7023dac2a042a84bf0bd ]

While calling the helpers, a raw pointer to the extensions area is wired into expectations list:

-> nf_ct_helper()
-> helper->help()

    -> nf_ct_expect_related_report()
     -> nf_ct_expect_insert()
      -> hlist_add_head_rcu(&exp->lnode, &master_help->expectations)

In case the connection is not confirmed yet, more extensions can be added afterwards with *_ext_add() calls reallocating the extension space and leaving the now invalid pointer in the expectations list that is later accessed while removing the expectation.

Make sure that helpers are called at the end after all the other extensions are already added.

Note that the helper rejection now leaves the mark and labels set, but that's not different from how the NAT was handled before or how the mark and the labels were handled on confirmation failure. And there are no atomicity guarantees provided by the API anyway.

Fixes a21b06e73191 ("net: sched: add helper support in act_ct")
[ Preserved the existing add_helper condition that upstream had removed. ]
---
 net/sched/act_ct.c | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)
net/sched/act_ct.c +12 -6
index cbac27ba76afc..c6085b06ec279 100644
@@ -1088,19 +1088,25 @@ TC_INDIRECT_SCOPE int tcf_ct_act(struct sk_buff *skb, const struct tc_action *a,
 		}
 	}
 
-	if (nf_ct_is_confirmed(ct) ? ((!cached && !skip_add) || add_helper) : commit) {
-		err = nf_ct_helper(skb, ct, ctinfo, family);
-		if (err != NF_ACCEPT)
-			goto nf_error;
-	}
-
 	if (commit) {
 		tcf_ct_act_set_mark(ct, p->mark, p->mark_mask);
 		tcf_ct_act_set_labels(ct, p->labels, p->labels_mask);
 
 		if (!nf_ct_is_confirmed(ct))
 			nf_conn_act_ct_ext_add(skb, ct, ctinfo);
+	}
 
+	/* Run helpers for the connection if nf_conntrack_in() was executed
+	 * or if we're about to commit.  This has to be done after all the
+	 * extensions are already added.
+	 */
+	if (nf_ct_is_confirmed(ct) ? ((!cached && !skip_add) || add_helper) : commit) {
+		err = nf_ct_helper(skb, ct, ctinfo, family);
+		if (err != NF_ACCEPT)
+			goto nf_error;
+	}
+
+	if (commit) {
 		/* This will take care of sending queued events
 		 * even if the connection is already confirmed.
 		 */
Signature · 1 line
2.53.0