[PATCH 5.10.y] net/sched: fix pedit partial COW leading to page cache corruption
From: Dominique Martinet <hidden>
Date: 2026-06-30 08:27:31
Also in:
lkml, stable
Subsystem:
networking [general], tc subsystem, the rest · Maintainers:
"David S. Miller", Eric Dumazet, Jakub Kicinski, Paolo Abeni, Jamal Hadi Salim, Jiri Pirko, Linus Torvalds
From: Rajat Gupta <redacted>
[ Upstream commit 899ee91156e57784090c5565e4f31bd7dbffbc5a ]
tcf_pedit_act() computes the COW range for skb_ensure_writable()
once before the key loop using tcfp_off_max_hint, but the hint does
not account for the runtime header offset added by typed keys. This
can leave part of the write region un-COW'd.
Fix by moving skb_ensure_writable() inside the per-key loop where
the actual write offset is known, and add overflow checking on the
offset arithmetic. For negative offsets (e.g. Ethernet header edits
at ingress), use skb_cow() to COW the headroom instead. Guard
offset_valid() against INT_MIN, where negation is undefined.
Fixes: 8b796475fd78 ("net/sched: act_pedit: really ensure the skb is writable")
Reported-by: Yiming Qian <redacted>
Reported-by: Keenan Dong <redacted>
Reported-by: Han Guidong <redacted>
Reported-by: Zhang Cen <redacted>
Reviewed-by: Han Guidong <redacted>
Tested-by: Han Guidong <redacted>
Reviewed-by: Davide Caratti <redacted>
Tested-by: Davide Caratti <redacted>
Reviewed-by: Toke Høiland-Jørgensen <redacted>
Tested-by: Toke Høiland-Jørgensen <redacted>
Reviewed-by: Victor Nogueira <redacted>
Tested-by: Victor Nogueira <redacted>
Acked-by: Jamal Hadi Salim <jhs@mojatatu.com>
Signed-off-by: Rajat Gupta <redacted>
Link: https://patch.msgid.link/20260531123221.48732-1-jhs@mojatatu.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
[Dominique: plenty of context conflict but the code itself could still
mostly be used]
Signed-off-by: Dominique Martinet <redacted>
---
(Sorry for double-send to stable@vger, I hadn't intended to send the
Cc-free version...)
CVE-2026-46331 got attention and I've had to backport this to 5.10, so
here we are, this should fix it.
Unfortunately there are more conflicts with 5.15 so I didn't do this
one (but if anyone cares older 5.4/4.19 cherry-picks cleanly from this,
they're no longer stable kernels so I won't bother sending)
I've tested in egress with a trivial rule redirecting port inspired from
the man page (and checking on target with tcpdump because return path is
broken), so I hopefully didn't botch this too badly:
tc qdisc replace dev eth0 root handle 1: htb
tc filter add dev eth0 parent 1: u32 match ip dport 23 0xffff action pedit pedit munge ip dport set 22
---
include/net/tc_act/tc_pedit.h | 1 -
net/sched/act_pedit.c | 84 ++++++++++++++++++++++---------------------
2 files changed, 43 insertions(+), 42 deletions(-)
diff --git a/include/net/tc_act/tc_pedit.h b/include/net/tc_act/tc_pedit.h
index 3e02709a1df656931942be4851a115dd6bef8b4c..748cf87a4d7ea5c92b4fd48dd3302b8ad64944fe 100644
--- a/include/net/tc_act/tc_pedit.h
+++ b/include/net/tc_act/tc_pedit.h@@ -14,7 +14,6 @@ struct tcf_pedit { struct tc_action common; unsigned char tcfp_nkeys; unsigned char tcfp_flags; - u32 tcfp_off_max_hint; struct tc_pedit_key *tcfp_keys; struct tcf_pedit_key_ex *tcfp_keys_ex; };
diff --git a/net/sched/act_pedit.c b/net/sched/act_pedit.c
index a44101b2f441919b7d4b0be177aa735a3c547350..cebeed6214558b1e1dd717b411c9ffa108a2293d 100644
--- a/net/sched/act_pedit.c
+++ b/net/sched/act_pedit.c@@ -14,6 +14,8 @@ #include <linux/module.h> #include <linux/init.h> #include <linux/slab.h> +#include <linux/overflow.h> +#include <asm/unaligned.h> #include <net/netlink.h> #include <net/pkt_sched.h> #include <linux/tc_act/tc_pedit.h>
@@ -229,21 +231,11 @@ static int tcf_pedit_init(struct net *net, struct nlattr *nla, p->tcfp_nkeys = parm->nkeys; } memcpy(p->tcfp_keys, parm->keys, ksize); - p->tcfp_off_max_hint = 0; for (i = 0; i < p->tcfp_nkeys; ++i) { - u32 cur = p->tcfp_keys[i].off; - /* sanitize the shift value for any later use */ p->tcfp_keys[i].shift = min_t(size_t, BITS_PER_TYPE(int) - 1, p->tcfp_keys[i].shift); - /* The AT option can read a single byte, we can bound the actual - * value with uchar max. - */ - cur += (0xff & p->tcfp_keys[i].offmask) >> p->tcfp_keys[i].shift; - - /* Each key touches 4 bytes starting from the computed offset */ - p->tcfp_off_max_hint = max(p->tcfp_off_max_hint, cur + 4); } p->tcfp_flags = parm->flags;
@@ -277,15 +269,12 @@ static void tcf_pedit_cleanup(struct tc_action *a) kfree(p->tcfp_keys_ex); } -static bool offset_valid(struct sk_buff *skb, int offset) +static bool offset_valid(struct sk_buff *skb, int offset, int len) { - if (offset > 0 && offset > skb->len) + if (offset < -(int)skb_headroom(skb)) return false; - if (offset < 0 && -offset > skb_headroom(skb)) - return false; - - return true; + return offset <= (int)skb->len - len; } static int pedit_skb_hdr_offset(struct sk_buff *skb,
@@ -325,18 +314,10 @@ static int tcf_pedit_act(struct sk_buff *skb, const struct tc_action *a, struct tcf_result *res) { struct tcf_pedit *p = to_pedit(a); - u32 max_offset; int i; spin_lock(&p->tcf_lock); - max_offset = (skb_transport_header_was_set(skb) ? - skb_transport_offset(skb) : - skb_network_offset(skb)) + - p->tcfp_off_max_hint; - if (skb_ensure_writable(skb, min(skb->len, max_offset))) - goto unlock; - tcf_lastuse_update(&p->tcf_tm); if (p->tcfp_nkeys > 0) {
@@ -347,10 +328,11 @@ static int tcf_pedit_act(struct sk_buff *skb, const struct tc_action *a, enum pedit_cmd cmd = TCA_PEDIT_KEY_EX_CMD_SET; for (i = p->tcfp_nkeys; i > 0; i--, tkey++) { - u32 *ptr, hdata; + int write_offset, write_len; int offset = tkey->off; int hoffset; - u32 val; + u32 cur_val, val; + u32 *ptr; int rc; if (tkey_ex) {
@@ -369,13 +351,15 @@ static int tcf_pedit_act(struct sk_buff *skb, const struct tc_action *a, if (tkey->offmask) { u8 *d, _d; + int at_offset; - if (!offset_valid(skb, hoffset + tkey->at)) { - pr_info("tc action pedit 'at' offset %d out of bounds\n", - hoffset + tkey->at); + if (check_add_overflow(hoffset, (int)tkey->at, &at_offset) || + !offset_valid(skb, at_offset, sizeof(_d))) { + pr_info_ratelimited("tc action pedit 'at' offset %d out of bounds\n", + hoffset + tkey->at); goto bad; } - d = skb_header_pointer(skb, hoffset + tkey->at, + d = skb_header_pointer(skb, at_offset, sizeof(_d), &_d); if (!d) goto bad;
@@ -387,23 +371,44 @@ static int tcf_pedit_act(struct sk_buff *skb, const struct tc_action *a, goto bad; } - if (!offset_valid(skb, hoffset + offset)) { - pr_info("tc action pedit offset %d out of bounds\n", - hoffset + offset); + if (check_add_overflow(hoffset, offset, &write_offset)) { + pr_info_ratelimited("tc action pedit offset overflow\n"); goto bad; } - ptr = skb_header_pointer(skb, hoffset + offset, - sizeof(hdata), &hdata); - if (!ptr) + if (!offset_valid(skb, write_offset, sizeof(*ptr))) { + pr_info_ratelimited("tc action pedit offset %d out of bounds\n", + write_offset); goto bad; + } + + if (write_offset < 0) { + if (skb_cow(skb, -write_offset)) + goto bad; + if (write_offset + (int)sizeof(*ptr) > 0) { + if (skb_ensure_writable(skb, + min_t(int, skb->len, + write_offset + (int)sizeof(*ptr)))) + goto bad; + } + } else { + if (check_add_overflow(write_offset, (int)sizeof(*ptr), + &write_len)) + goto bad; + if (skb_ensure_writable(skb, min_t(int, skb->len, + write_len))) + goto bad; + } + + ptr = (u32 *)(skb->data + write_offset); + cur_val = get_unaligned(ptr); /* just do it, baby */ switch (cmd) { case TCA_PEDIT_KEY_EX_CMD_SET: val = tkey->val; break; case TCA_PEDIT_KEY_EX_CMD_ADD: - val = (*ptr + tkey->val) & ~tkey->mask; + val = (cur_val + tkey->val) & ~tkey->mask; break; default: pr_info("tc action pedit bad command (%d)\n",
@@ -411,9 +416,7 @@ static int tcf_pedit_act(struct sk_buff *skb, const struct tc_action *a, goto bad; } - *ptr = ((*ptr & tkey->mask) ^ val); - if (ptr == &hdata) - skb_store_bits(skb, hoffset + offset, ptr, 4); + put_unaligned((cur_val & tkey->mask) ^ val, ptr); } goto done;
@@ -425,7 +428,6 @@ static int tcf_pedit_act(struct sk_buff *skb, const struct tc_action *a, p->tcf_qstats.overlimits++; done: bstats_update(&p->tcf_bstats, skb); -unlock: spin_unlock(&p->tcf_lock); return p->tcf_action; }
--- base-commit: d9666dca97c01de1c7395ac53041634eb75120e2 change-id: 20260630-cve-2026-46331-4f851ccf8244 Best regards, -- Dominique Martinet [off-list ref]