[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

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

[Jamal Hadi Salim]

Hi Florian,

On Wed, Aug 19, 2026 at 4:42 PM Florian Westphal <fw@strlen.de> wrote:
>
> 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.
>

I apologize for the delayed response in the private exchange, but let
me repeat my thoughts here...
As you said, the munging can be done by an ebpf program (at different
attachment points, not just cls/act using multiple helpers).
In your commit you say:
"Packet rewrites are still possible with BPF. However, unlike pedit,
that needs privileges in the initial namespace."
That's only true in the default setup, but with BPF "token delegation"
it no longer holds (will be reachable via unshare -Urn).

I have not tried to verify what i am claiming since we "last talked" -
but could try it out when i get time (weekend looks promising).
If what i am saying is true then you will have many places in ebpf to
fix, which is not ideal.
But even if not true, you can still do it with CAP_BPF - which leads
to the same outcome. Unless we are saying as a general rule we should
prioritize where/how we fix things based on whether they are
reproducible via 1)namespace -urn  vs 2) require root permission
(assume this includes CAP_BPF)

With that thought in mind, the question is: is there a single hook
entry point where this could be done?
Example: nf_conntrack_in() or nf_confirm() as the choke point which
catches all? i.e check the protocol consistency there and if there is
a mismatch you call nf_reset
I understand the dilemma: fixing this in both pedit and eBPF will
penalize only those hot paths as opposed to this approach which will
penalize all of contrack users with no interest in pedit/ebpf - but
otoh, I feel like this will be a good defensive mechanism for
netfilter for whichever new creative change in the kernel which could
be used to make similar changes.

I could experiment with the single choke point approach on the weekend
- let me know.

cheers,
jamal

> 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
>

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

[Jamal Hadi Salim]

On Thu, Aug 20, 2026 at 7:09 AM Jamal Hadi Salim <jhs@mojatatu.com> wrote:
>
> Hi Florian,
>
> On Wed, Aug 19, 2026 at 4:42 PM Florian Westphal <fw@strlen.de> wrote:
> >
> > 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.
> >
>
> I apologize for the delayed response in the private exchange, but let
> me repeat my thoughts here...
> As you said, the munging can be done by an ebpf program (at different
> attachment points, not just cls/act using multiple helpers).
> In your commit you say:
> "Packet rewrites are still possible with BPF. However, unlike pedit,
> that needs privileges in the initial namespace."
> That's only true in the default setup, but with BPF "token delegation"
> it no longer holds (will be reachable via unshare -Urn).
>
> I have not tried to verify what i am claiming since we "last talked" -
> but could try it out when i get time (weekend looks promising).
> If what i am saying is true then you will have many places in ebpf to
> fix, which is not ideal.
> But even if not true, you can still do it with CAP_BPF - which leads
> to the same outcome. Unless we are saying as a general rule we should
> prioritize where/how we fix things based on whether they are
> reproducible via 1)namespace -urn  vs 2) require root permission
> (assume this includes CAP_BPF)
>
> With that thought in mind, the question is: is there a single hook
> entry point where this could be done?
> Example: nf_conntrack_in() or nf_confirm() as the choke point which

sorry, my brain wouldnt let it go: nf_confirm() looks very promising.
I was tempted to create a patch and test it but i dont have time right now..

cheers,
jamal

> catches all? i.e check the protocol consistency there and if there is
> a mismatch you call nf_reset
> I understand the dilemma: fixing this in both pedit and eBPF will
> penalize only those hot paths as opposed to this approach which will
> penalize all of contrack users with no interest in pedit/ebpf - but
> otoh, I feel like this will be a good defensive mechanism for
> netfilter for whichever new creative change in the kernel which could
> be used to make similar changes.
>
> I could experiment with the single choke point approach on the weekend
> - let me know.
>
> cheers,
> jamal
>
> > 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
> >

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

[Florian Westphal]

Jamal Hadi Salim <jhs@mojatatu.com> wrote:
> I apologize for the delayed response in the private exchange, but let
> me repeat my thoughts here...
> As you said, the munging can be done by an ebpf program (at different
> attachment points, not just cls/act using multiple helpers).

Yes, and that is ALSO buggy.  For BPF its even worse because if you
permit arbitrary writes at any point in the stack you will also
invalidate e.g. addresses/offsets stored in skb->cb[].

> "Packet rewrites are still possible with BPF. However, unlike pedit,
> that needs privileges in the initial namespace."
> That's only true in the default setup, but with BPF "token delegation"
> it no longer holds (will be reachable via unshare -Urn).
> 
> I have not tried to verify what i am claiming since we "last talked" -
> but could try it out when i get time (weekend looks promising).
> If what i am saying is true then you will have many places in ebpf to
> fix, which is not ideal.

*shrug*

> But even if not true, you can still do it with CAP_BPF - which leads
> to the same outcome. Unless we are saying as a general rule we should
> prioritize where/how we fix things based on whether they are
> reproducible via 1)namespace -urn  vs 2) require root permission
> (assume this includes CAP_BPF)

I think 2) is very different from 1).

> With that thought in mind, the question is: is there a single hook
> entry point where this could be done?

No.

> Example: nf_conntrack_in() or nf_confirm() as the choke point which
> catches all?

Those are too late.  You would need a new, unconditional sanitizer hook
at INT_MIN, in all of IPV4/IPV6/Bridge, plus additional code in core.c
to refuse installation of custom hooks before the sanitizer hook.

And that doesn't solve BPF (can sit anywhere) or OVS (no idea here if
its affected or not).

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

[Florian Westphal]

Jamal Hadi Salim <jhs@mojatatu.com> wrote:
> sorry, my brain wouldnt let it go: nf_confirm() looks very promising.
> I was tempted to create a patch and test it but i dont have time right now..

nf_confirm is the last hook in the conntrack pipeline, so not useful for
this -- all of the exposed code runs before this.

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

[Jamal Hadi Salim]

On Thu, Aug 20, 2026 at 7:42 AM Florian Westphal <fw@strlen.de> wrote:
>
> Jamal Hadi Salim <jhs@mojatatu.com> wrote:
> > I apologize for the delayed response in the private exchange, but let
> > me repeat my thoughts here...
> > As you said, the munging can be done by an ebpf program (at different
> > attachment points, not just cls/act using multiple helpers).
>
> Yes, and that is ALSO buggy.  For BPF its even worse because if you
> permit arbitrary writes at any point in the stack you will also
> invalidate e.g. addresses/offsets stored in skb->cb[].
>

There are a lot of helpers that could cause a mess.

> > "Packet rewrites are still possible with BPF. However, unlike pedit,
> > that needs privileges in the initial namespace."
> > That's only true in the default setup, but with BPF "token delegation"
> > it no longer holds (will be reachable via unshare -Urn).
> >
> > I have not tried to verify what i am claiming since we "last talked" -
> > but could try it out when i get time (weekend looks promising).
> > If what i am saying is true then you will have many places in ebpf to
> > fix, which is not ideal.
>
> *shrug*
>
> > But even if not true, you can still do it with CAP_BPF - which leads
> > to the same outcome. Unless we are saying as a general rule we should
> > prioritize where/how we fix things based on whether they are
> > reproducible via 1)namespace -urn  vs 2) require root permission
> > (assume this includes CAP_BPF)
>
> I think 2) is very different from 1).
>

I agree. In fact, on the tc side we prioritize fixing bugs
reproducible using unshare -urn over those that require root.
The question is: Should that influence how and where we fix bugs? It
is something i am struggling with.
Example: If the ebpf one could be reproduced with unshare -urn would
it have been fine to make the fix sit in netfilter?

> > With that thought in mind, the question is: is there a single hook
> > entry point where this could be done?
>
> No.
>
> > Example: nf_conntrack_in() or nf_confirm() as the choke point which
> > catches all?
>
> Those are too late.  You would need a new, unconditional sanitizer hook
> at INT_MIN, in all of IPV4/IPV6/Bridge, plus additional code in core.c
> to refuse installation of custom hooks before the sanitizer hook.
>

Yeah, that is a bit much.

> And that doesn't solve BPF (can sit anywhere) or OVS (no idea here if
> its affected or not).

OVS is very likely affected.
I saw your other email and you are the expert, but I am itchy to try
the nf_confirm() approach.
At minimal it should fix the PoC imo.

cheers,
jamal

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

[Florian Westphal]

Jamal Hadi Salim <jhs@mojatatu.com> wrote:
> Example: If the ebpf one could be reproduced with unshare -urn would
> it have been fine to make the fix sit in netfilter?

I don't know how to 'fix netfilter'.  We would have to remove skb->nfct
and re-parse at every turn.  Is that a viable option...? Don't think so.

> > And that doesn't solve BPF (can sit anywhere) or OVS (no idea here if
> > its affected or not).
> 
> OVS is very likely affected.
> I saw your other email and you are the expert, but I am itchy to try
> the nf_confirm() approach.
> At minimal it should fix the PoC imo.

As I said, I don't see any existing function that could be used
to add the required re-validation.  nf_confirm() is the last step
in the pipeline, its way too late.  nf_conntrack_in() doesn't work
either because you can install matches earlier than that, e.g. via
-t raw -m conntrack in classic iptables, or via 'hook prerouting
type filter priority -2147483647' or something like that in
nftables.

The only other option is playing whack-a-mole, accepting the original
proposed patch for nft_ct, but I can already tell this will not be
enough and revalidation will have to be added everywhere.

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

[Jamal Hadi Salim]

[-- Attachment #1: Type: text/plain, Size: 1666 bytes --]

On Thu, Aug 20, 2026 at 8:09 AM Florian Westphal <fw@strlen.de> wrote:
>
> Jamal Hadi Salim <jhs@mojatatu.com> wrote:
> > Example: If the ebpf one could be reproduced with unshare -urn would
> > it have been fine to make the fix sit in netfilter?
>
> I don't know how to 'fix netfilter'.  We would have to remove skb->nfct
> and re-parse at every turn.  Is that a viable option...? Don't think so.
>
> > > And that doesn't solve BPF (can sit anywhere) or OVS (no idea here if
> > > its affected or not).
> >
> > OVS is very likely affected.
> > I saw your other email and you are the expert, but I am itchy to try
> > the nf_confirm() approach.
> > At minimal it should fix the PoC imo.
>
> As I said, I don't see any existing function that could be used
> to add the required re-validation.  nf_confirm() is the last step
> in the pipeline, its way too late.

Sigh - couldnt help myself. Attached  fixes the poc from kyle - i know
you are hinting there are other scenarios where it wont work.


>  nf_conntrack_in() doesn't work
> either because you can install matches earlier than that, e.g. via
> -t raw -m conntrack in classic iptables, or via 'hook prerouting
> type filter priority -2147483647' or something like that in
> nftables.
>
> The only other option is playing whack-a-mole, accepting the original
> proposed patch for nft_ct, but I can already tell this will not be
> enough and revalidation will have to be added everywhere.

whack-a-mole is what i was worrying about. Someone will come up with a
scenario for ebpf and likely ovs and you will have to fix multiple
spots until the next one.

cheers,
jamal

[-- Attachment #2: nf-confirm.patchlet --]
[-- Type: application/octet-stream, Size: 742 bytes --]

diff --git a/net/netfilter/nf_conntrack_proto.c b/net/netfilter/nf_conntrack_proto.c
index 7a40e4e0e33e..302b4aa9073e 100644
--- a/net/netfilter/nf_conntrack_proto.c
+++ b/net/netfilter/nf_conntrack_proto.c
@@ -151,6 +151,22 @@ unsigned int nf_confirm(void *priv,
 	if (!ct || in_vrf_postrouting(state))
 		return NF_ACCEPT;
 
+	switch (nf_ct_l3num(ct)) {
+	case NFPROTO_IPV4:
+		pnum = ip_hdr(skb)->protocol;
+		break;
+	case NFPROTO_IPV6:
+		pnum = ipv6_hdr(skb)->nexthdr;
+		break;
+	default:
+		pnum = nf_ct_protonum(ct);
+		break;
+	}
+	if (pnum != nf_ct_protonum(ct)) {
+		nf_reset_ct(skb);
+		return NF_ACCEPT;
+	}
+
 	help = nfct_help(ct);
 
 	seqadj_needed = test_bit(IPS_SEQ_ADJUST_BIT, &ct->status) && !nf_is_loopback_packet(skb);

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

[Florian Westphal]

Jamal Hadi Salim <jhs@mojatatu.com> wrote:
> Sigh - couldnt help myself. Attached  fixes the poc from kyle - i know
> you are hinting there are other scenarios where it wont work.

Yes, e.g. what if pedit inflated th->doff?
nf_conntrack_in() checked that this was fine, but now the pipeline
can re-neg on that.

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

[Jamal Hadi Salim]

On Thu, Aug 20, 2026 at 9:10 AM Florian Westphal <fw@strlen.de> wrote:
>
> Jamal Hadi Salim <jhs@mojatatu.com> wrote:
> > Sigh - couldnt help myself. Attached  fixes the poc from kyle - i know
> > you are hinting there are other scenarios where it wont work.
>
> Yes, e.g. what if pedit inflated th->doff?
> nf_conntrack_in() checked that this was fine, but now the pipeline
> can re-neg on that.

True.
So my question to you then is: what about BPF, OVS, future thingy? Are
you going to fix each individually?
Or is the answer there "these are different" because they require
root/CAP_BPF they are not worth fixing?

cheers,
jamal

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

[Florian Westphal]

Jamal Hadi Salim <jhs@mojatatu.com> wrote:
> > Yes, e.g. what if pedit inflated th->doff?
> > nf_conntrack_in() checked that this was fine, but now the pipeline
> > can re-neg on that.
> 
> True.
> So my question to you then is: what about BPF, OVS, future thingy? Are
> you going to fix each individually?

I see no alternative?  I mean, what are you suggesting?
We can apply the patch from Kyle, that will stop the reproducer.
But I can tell you that we'll need hundreds of followup patches
all over.

I mean, we always relied on IP stack having checked that iph->ihl is
fine, the ipv6 header is complete, etc.

And that isn't exclusive to netfilter.

My best suggestion is to remove skb->_nfct and reparse everywhere,
but I will NOT make such a patch, IMO conntrack and netfilter might
as well be axed then.  Hey, would solve a few bugs.

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

[Jamal Hadi Salim]

On Thu, Aug 20, 2026 at 1:37 PM Florian Westphal <fw@strlen.de> wrote:
>
> Jamal Hadi Salim <jhs@mojatatu.com> wrote:
> > > Yes, e.g. what if pedit inflated th->doff?
> > > nf_conntrack_in() checked that this was fine, but now the pipeline
> > > can re-neg on that.
> >
> > True.
> > So my question to you then is: what about BPF, OVS, future thingy? Are
> > you going to fix each individually?
>
> I see no alternative?  I mean, what are you suggesting?
>
> We can apply the patch from Kyle, that will stop the reproducer.
> But I can tell you that we'll need hundreds of followup patches
> all over.
>
> I mean, we always relied on IP stack having checked that iph->ihl is
> fine, the ipv6 header is complete, etc.
>
> And that isn't exclusive to netfilter.
>

Agreed.
My suggestion is: if you fix pedit then fix all other sources.
Kyle should be able to provide repros for the others (ebpf etc).

cheers,
jamal

> My best suggestion is to remove skb->_nfct and reparse everywhere,
> but I will NOT make such a patch, IMO conntrack and netfilter might
> as well be axed then.  Hey, would solve a few bugs.