From: Florian Westphal <fw@strlen.de>
To: <netdev@vger.kernel.org>
Cc: jhs@mojatatu.com, jiri@resnulli.us, victor@mojatatu.com,
Florian Westphal <fw@strlen.de>, Kyle Zeng <kylebot@openai.com>
Subject: [PATCH net 2/4] net/sched: act_pedit: drop conntrack on network/transport header changes
Date: Wed, 19 Aug 2026 22:42:08 +0200 [thread overview]
Message-ID: <20260819204210.23722-3-fw@strlen.de> (raw)
In-Reply-To: <20260819204210.23722-1-fw@strlen.de>
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
next prev parent reply other threads:[~2026-08-19 20:42 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-08-19 20:42 [PATCH net 0/4] net/sched: reset conntrack after packet munging Florian Westphal
2026-08-19 20:42 ` [PATCH net 1/4] selftests/tc-testing: pass mp_pm via initialiser Florian Westphal
2026-08-19 20:42 ` Florian Westphal [this message]
2026-08-19 20:42 ` [PATCH net 3/4] selftests: tc-testing: add act_ct test for ct reset handling Florian Westphal
2026-08-19 20:42 ` [PATCH net 4/4] net/sched: act_nat: discard any conntrack entry post modification Florian Westphal
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20260819204210.23722-3-fw@strlen.de \
--to=fw@strlen.de \
--cc=jhs@mojatatu.com \
--cc=jiri@resnulli.us \
--cc=kylebot@openai.com \
--cc=netdev@vger.kernel.org \
--cc=victor@mojatatu.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox