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 Closes: https://lore.kernel.org/netfilter-devel/20260810221744.35007-1-kylebot@openai.com/ Signed-off-by: Florian Westphal --- 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