[PATCH net 0/4] net/sched: reset conntrack after packet munging

[PATCH net 0/4] net/sched: reset conntrack after packet munging

[Florian Westphal]

This series removes skb <-> conntrack mapping after pedit
altered l3 and/or l4 headers.  See patch 2 for all the details.

First patch makes tdc.py work for me, second patch is the actual
fix, third patch has test cases. Last patch also removes the mapping
in act_nat.

Florian Westphal (4):
  selftests/tc-testing: pass mp_pm via initialiser
  net/sched: act_pedit: drop conntrack on network/transport header changes
  selftests: tc-testing: add act_ct test for ct reset handling
  net/sched: act_nat: discard any conntrack entry post modification

 net/sched/act_nat.c                           |   1 +
 net/sched/act_pedit.c                         |  21 ++-
 .../tc-testing/tc-tests/actions/ct.json       | 168 ++++++++++++++++++
 tools/testing/selftests/tc-testing/tdc.py     |  27 ++-
 4 files changed, 208 insertions(+), 9 deletions(-)

-- 
2.54.0

[PATCH net 1/4] selftests/tc-testing: pass mp_pm via initialiser

[Florian Westphal]

The script doesn't work for me:
 -- ns/SubPlugin.__init__
Executing 1205 tests in parallel and 89 in serial
Using 39 batches and 4 workers
multiprocessing.pool.RemoteTraceback:
"""
Traceback (most recent call last):
  File "/usr/lib/python3.14/multiprocessing/pool.py", line 125, in worker
    result = (True, func(*args, **kwds))
                    ~~~~^^^^^^^^^^^^^^^
  File "/usr/lib/python3.14/multiprocessing/pool.py", line 48, in mapstar
    return list(map(*args))
  File "tools/testing/selftests/tc-testing/tdc.py", line 604, in __mp_runner
    (_, tsr) = test_runner(mp_pm, mp_args, tests)
                           ^^^^^
NameError: name 'mp_pm' is not defined

Assisted-by: ollama:gemma4:26b
Signed-off-by: Florian Westphal <fw@strlen.de>
---
 tools/testing/selftests/tc-testing/tdc.py | 27 ++++++++++++++++-------
 1 file changed, 19 insertions(+), 8 deletions(-)

diff --git a/tools/testing/selftests/tc-testing/tdc.py b/tools/testing/selftests/tc-testing/tdc.py
index 511d66c36a2a..19b8a1fd1c71 100755
--- a/tools/testing/selftests/tc-testing/tdc.py
+++ b/tools/testing/selftests/tc-testing/tdc.py
@@ -600,6 +600,18 @@ def mp_bins(alltests):
 
     return (serial, parallel)
 
+mp_pm = None
+mp_args = None
+
+def __mp_init__(pm, args):
+    """
+    This function is called once when each worker process starts.
+    It sets the global variables in the child process's memory space.
+    """
+    global mp_pm, mp_args
+    mp_pm = pm
+    mp_args = args
+
 def __mp_runner(tests):
     (_, tsr) = test_runner(mp_pm, mp_args, tests)
     return tsr._testsuite
@@ -615,14 +627,13 @@ def test_runner_mp(pm, args, alltests):
     print("Executing {} tests in parallel and {} in serial".format(len(parallel), len(serial)))
     print("Using {} batches and {} workers".format(len(batches), args.mp))
 
-    # We can't pickle these objects so workaround them
-    global mp_pm
-    mp_pm = pm
-
-    global mp_args
-    mp_args = args
-
-    with Pool(args.mp) as p:
+    # Use the 'initializer' to pass the unpickleable/shared objects
+    # to each worker process exactly once upon startup.
+    with Pool(
+        processes=args.mp,
+        initializer=__mp_init__,
+        initargs=(pm, args)
+    ) as p:
         pres = p.map(__mp_runner, batches)
 
     tsr = TestSuiteReport()
-- 
2.54.0

[PATCH net 2/4] net/sched: act_pedit: drop conntrack on network/transport header changes

[Florian Westphal]

PEDIT can rewrite packet headers after act_ct attached a conntrack entry.
conntrack (and conntrack helpers) rely on validation done by conntrack or
network stack, i.e. ip header lengths, th->doff and the like are assumed
to be valid.

One common gadget to cause OOB access or worse is "action ct", followed
by "action pedit munge ip protocol set 1"

... and then waiting for netfilter to trip because it trusts packet and
conntrack entry refer to the same protocols.

Reset the skb conntrack state if one was attached and we might have updated
relevant header fields.

This patch is a simpler version of related commits:

968cc2c96390 ("netfilter: disable payload mangling in userns")
df07998dfd40 ("netfilter: nftables: restrict linklayer and network header writes")
54f34607d184 ("netfilter: nfnetlink_queue: restrict writes to network header")

that restricted post-conntrack-pickup mangling in netfilter.

The opposite approach -- revalidation at every turn -- is hardly
feasible, even examples like:

        if (nf_ct_protonum(ct) == IPPROTO_TCP) {
                th = (struct tcphdr *)(skb->data + protoff);
                baseoff = protoff + th->doff * 4;

or
        if (ip_hdr(skb)->protocol == IPPROTO_TCP) {
                if (!nf_nat_mangle_tcp_packet(skb, ct, ctinfo, ..

... would be buggy: nf_ct_protonum(ct) and ip_hdr->protocol could be
off-sync and th->doff could point past skb writeable area.

Fixes tag points to 'action ct'.  Packet rewrites are still possible with
BPF. However, unlike pedit, that needs privileges in the initial namespace.

Fixes: b57dc7c13ea9 ("net/sched: Introduce action ct")
Assisted-by: Claude:claude-sonnet-5
Reported-by: Kyle Zeng <kylebot@openai.com>
Closes: https://lore.kernel.org/netfilter-devel/20260810221744.35007-1-kylebot@openai.com/
Signed-off-by: Florian Westphal <fw@strlen.de>
---
 passes tc tests, old POC for originally reported issue
 no longer causes a splat.

 net/sched/act_pedit.c | 21 ++++++++++++++++++++-
 1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/net/sched/act_pedit.c b/net/sched/act_pedit.c
index d4d47a9921f4..b079257c2b7b 100644
--- a/net/sched/act_pedit.c
+++ b/net/sched/act_pedit.c
@@ -386,6 +386,7 @@ TC_INDIRECT_SCOPE int tcf_pedit_act(struct sk_buff *skb,
 	struct tcf_pedit *p = to_pedit(a);
 	struct tcf_pedit_key_ex *tkey_ex;
 	struct tcf_pedit_parms *parms;
+	bool l3_l4_changed = false;
 	struct tc_pedit_key *tkey;
 	int i;
 
@@ -398,7 +399,7 @@ TC_INDIRECT_SCOPE int tcf_pedit_act(struct sk_buff *skb,
 	tkey_ex = parms->tcfp_keys_ex;
 
 	for (i = parms->tcfp_nkeys; i > 0; i--, tkey++) {
-		int write_offset, write_len;
+		int write_offset, write_len, nw_offset;
 		int offset = tkey->off;
 		int hoffset = 0;
 		u32 cur_val, val;
@@ -418,6 +419,8 @@ TC_INDIRECT_SCOPE int tcf_pedit_act(struct sk_buff *skb,
 			goto bad;
 		}
 
+		nw_offset = skb_network_offset(skb);
+
 		if (tkey->offmask) {
 			u8 *d, _d;
 			int at_offset;
@@ -485,6 +488,19 @@ TC_INDIRECT_SCOPE int tcf_pedit_act(struct sk_buff *skb,
 		}
 
 		put_unaligned((cur_val & tkey->mask) ^ val, ptr);
+
+		/* Track if L3 or L4 headers were modified:
+		 * - Direct L3/L4 header types
+		 * - ETH header type with offset/length reaching into L3/L4
+		 */
+		if (htype == TCA_PEDIT_KEY_EX_HDR_TYPE_NETWORK ||
+		    htype == TCA_PEDIT_KEY_EX_HDR_TYPE_IP4 ||
+		    htype == TCA_PEDIT_KEY_EX_HDR_TYPE_IP6 ||
+		    htype == TCA_PEDIT_KEY_EX_HDR_TYPE_TCP ||
+		    htype == TCA_PEDIT_KEY_EX_HDR_TYPE_UDP ||
+		    (htype == TCA_PEDIT_KEY_EX_HDR_TYPE_ETH &&
+		     write_offset + (int)sizeof(*ptr) > nw_offset))
+			l3_l4_changed = true;
 	}
 
 	goto done;
@@ -492,6 +508,9 @@ TC_INDIRECT_SCOPE int tcf_pedit_act(struct sk_buff *skb,
 bad:
 	tcf_action_inc_overlimit_qstats(&p->common);
 done:
+	if (l3_l4_changed)
+		nf_reset_ct(skb);
+
 	return parms->action;
 }
 
-- 
2.54.0

[PATCH net 3/4] selftests: tc-testing: add act_ct test for ct reset handling

[Florian Westphal]

Check that skb->nfct is re-set when pedit munges network or transport
header data, else we feed packets to stack where skb might carry
different or invalid headers compared to what conntrack input has
validated earlier.

Also check MAC modifications keep the ct entry as-is.

Assisted-by: Claude:claude-sonnet-5
Signed-off-by: Florian Westphal <fw@strlen.de>
---
 .../tc-testing/tc-tests/actions/ct.json       | 168 ++++++++++++++++++
 1 file changed, 168 insertions(+)

diff --git a/tools/testing/selftests/tc-testing/tc-tests/actions/ct.json b/tools/testing/selftests/tc-testing/tc-tests/actions/ct.json
index 8ab48def89b6..18a773748358 100644
--- a/tools/testing/selftests/tc-testing/tc-tests/actions/ct.json
+++ b/tools/testing/selftests/tc-testing/tc-tests/actions/ct.json
@@ -742,5 +742,173 @@
         "teardown": [
             "$TC qdisc del dev $DEV1 clsact"
         ]
+    },
+    {
+        "id": "c2a9",
+        "name": "Verify conntrack reset after pedit modifies IP protocol",
+        "category": [
+            "actions",
+            "ct",
+            "pedit",
+            "scapy"
+        ],
+        "plugins": {
+            "requires": [
+                "nsPlugin",
+                "scapyPlugin"
+            ]
+        },
+        "setup": [
+            [
+                "$TC qdisc del dev $DEV1 ingress",
+                0,
+                1,
+                2,
+                255
+            ],
+            "iptables -t raw -F",
+            "iptables -t raw -A PREROUTING -i $DEV1 -p icmp -m conntrack --ctstate INVALID -j DROP",
+            "$TC qdisc add dev $DEV1 ingress"
+        ],
+        "cmdUnderTest": "$TC filter add dev $DEV1 ingress protocol ip prio 1 flower action ct commit pipe action pedit ex munge ip protocol set 1 pipe action csum ip4h continue",
+        "scapy": {
+            "iface": "$DEV0",
+            "count": 1,
+            "packet": "Ether(type=0x800)/IP(src='10.0.0.10',dst='10.0.0.20',proto=6)/TCP(sport=1234,dport=80,flags='S')"
+        },
+        "expExitCode": "0",
+        "verifyCmd": "iptables-save -c -t raw | grep 'ctstate INVALID'",
+        "matchPattern": "^\\[1:40\\]",
+        "matchCount": "1",
+        "teardown": [
+            "$TC qdisc del dev $DEV1 ingress",
+            "iptables -t raw -F"
+        ]
+    },
+    {
+        "id": "d3b8",
+        "name": "Verify conntrack reset after TCP port modification",
+        "category": [
+            "actions",
+            "ct",
+            "pedit",
+            "scapy"
+        ],
+        "plugins": {
+            "requires": [
+                "nsPlugin",
+                "scapyPlugin"
+            ]
+        },
+        "setup": [
+            [
+                "$TC qdisc del dev $DEV1 ingress",
+                0,
+                1,
+                2,
+                255
+            ],
+            "iptables -t raw -F",
+            "iptables -t raw -A PREROUTING -i $DEV1 -p tcp -m conntrack --ctstate INVALID -j DROP",
+            "$TC qdisc add dev $DEV1 ingress"
+        ],
+        "cmdUnderTest": "$TC filter add dev $DEV1 ingress protocol ip prio 1 flower ip_proto tcp action ct commit pipe action pedit ex munge tcp sport set 9999 pipe",
+        "scapy": {
+            "iface": "$DEV0",
+            "count": 1,
+            "packet": "Ether(type=0x800)/IP(src='10.0.0.10',dst='10.0.0.20')/TCP(sport=1234,dport=80,flags='S')"
+        },
+        "expExitCode": "0",
+        "verifyCmd": "iptables-save -c -t raw | grep 'ctstate INVALID'",
+        "matchCount": "1",
+        "matchPattern": "^\\[1:40\\]",
+        "teardown": [
+            "$TC qdisc del dev $DEV1 ingress",
+            "iptables -t raw -F"
+        ]
+    },
+    {
+        "id": "a1b2",
+        "name": "Verify conntrack NOT reset when pedit modifies only MAC",
+        "category": [
+            "actions",
+            "ct",
+            "pedit",
+            "scapy"
+        ],
+        "plugins": {
+            "requires": [
+                "nsPlugin",
+                "scapyPlugin"
+            ]
+        },
+        "setup": [
+            [
+                "$TC qdisc del dev $DEV1 ingress",
+                0,
+                1,
+                2,
+                255
+            ],
+            "iptables -t raw -F",
+            "iptables -t raw -A PREROUTING -i $DEV1 -p icmp -m conntrack --ctstate NEW -j DROP",
+            "$TC qdisc add dev $DEV1 ingress"
+        ],
+        "cmdUnderTest": "$TC filter add dev $DEV1 ingress protocol ip prio 1 flower action ct commit pipe action pedit ex munge eth src set 11:22:33:44:55:66 pipe",
+        "scapy": {
+            "iface": "$DEV0",
+            "count": 1,
+            "packet": "Ether(src='aa:bb:cc:dd:ee:ff',type=0x800)/IP(src='10.0.0.11',dst='10.0.0.21')/ICMP()"
+        },
+        "expExitCode": "0",
+        "verifyCmd": "iptables-save -c -t raw | grep 'ctstate NEW'",
+        "matchPattern": "^\\[1:28\\]",
+        "matchCount": "1",
+        "teardown": [
+            "$TC qdisc del dev $DEV1 ingress",
+            "iptables -t raw -F"
+        ]
+    },
+    {
+        "id": "b7f4",
+        "name": "Verify conntrack reset when ETH write extends into IP layer",
+        "category": [
+            "actions",
+            "ct",
+            "pedit",
+            "scapy"
+        ],
+        "plugins": {
+            "requires": [
+                "nsPlugin",
+                "scapyPlugin"
+            ]
+        },
+        "setup": [
+            [
+                "$TC qdisc del dev $DEV1 ingress",
+                0,
+                1,
+                2,
+                255
+            ],
+            "iptables -t raw -F",
+            "iptables -t raw -A PREROUTING -i $DEV1 -m conntrack --ctstate INVALID -j DROP",
+            "$TC qdisc add dev $DEV1 ingress"
+        ],
+        "cmdUnderTest": "$TC filter add dev $DEV1 ingress protocol ip prio 1 flower action ct commit pipe action pedit ex munge offset 12 u32 set 0x08004500 pipe csum ip4h continue",
+        "scapy": {
+            "iface": "$DEV0",
+            "count": 1,
+            "packet": "Ether(type=0x800)/IP(src='10.0.0.13',dst='10.0.0.23')/ICMP()"
+        },
+        "expExitCode": "0",
+        "verifyCmd": "iptables-save -t raw -c | grep 'ctstate INVALID'",
+        "matchPattern": "^\\[1:28\\]",
+        "matchCount": "1",
+        "teardown": [
+            "$TC qdisc del dev $DEV1 ingress",
+            "iptables -t raw -F"
+        ]
     }
 ]
-- 
2.54.0

[PATCH net 4/4] net/sched: act_nat: discard any conntrack entry post modification

[Florian Westphal]

act_nat is stateless nat, so the expectation is that skb->_nfct isn't
set in the first place.

But if it is, then addresses stored in the conntrack entry no longer
match what is in the packet payload, which isn't expected.

As act_nat can't mangle arbitrary data, this shouldn't be an issue
but better play it safe and don't retain such a conntrack entry.

Fixes: b57dc7c13ea9 ("net/sched: Introduce action ct")
Signed-off-by: Florian Westphal <fw@strlen.de>
---
 net/sched/act_nat.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/net/sched/act_nat.c b/net/sched/act_nat.c
index abb332dee836..0c3d3b89d349 100644
--- a/net/sched/act_nat.c
+++ b/net/sched/act_nat.c
@@ -257,6 +257,7 @@ TC_INDIRECT_SCOPE int tcf_nat_act(struct sk_buff *skb,
 	}
 
 out:
+	nf_reset_ct(skb);
 	return action;
 
 drop:
-- 
2.54.0