Skip to content

Commit c9637db

Browse files
Rajat Guptaroxanan1996
authored andcommitted
net/sched: fix pedit partial COW leading to page cache corruption
jira VULN-188478 cve CVE-2026-46331 commit-author Rajat Gupta <rajat.gupta@oss.qualcomm.com> commit 899ee91 upstream-diff | rename include file from linux/unaligned.h to asm/unaligned.h 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: 8b79647 ("net/sched: act_pedit: really ensure the skb is writable") Reported-by: Yiming Qian <yimingqian591@gmail.com> Reported-by: Keenan Dong <keenanat2000@gmail.com> Reported-by: Han Guidong <2045gemini@gmail.com> Reported-by: Zhang Cen <rollkingzzc@gmail.com> Reviewed-by: Han Guidong <2045gemini@gmail.com> Tested-by: Han Guidong <2045gemini@gmail.com> Reviewed-by: Davide Caratti <dcaratti@redhat.com> Tested-by: Davide Caratti <dcaratti@redhat.com> Reviewed-by: Toke Høiland-Jørgensen <toke@redhat.com> Tested-by: Toke Høiland-Jørgensen <toke@redhat.com> Reviewed-by: Victor Nogueira <victor@mojatatu.com> Tested-by: Victor Nogueira <victor@mojatatu.com> Acked-by: Jamal Hadi Salim <jhs@mojatatu.com> Signed-off-by: Rajat Gupta <rajat.gupta@oss.qualcomm.com> Link: https://patch.msgid.link/20260531123221.48732-1-jhs@mojatatu.com Signed-off-by: Jakub Kicinski <kuba@kernel.org> (cherry picked from commit 899ee91) Signed-off-by: Roxana Nicolescu <rnicolescu@ciq.com>
1 parent b0f6966 commit c9637db

2 files changed

Lines changed: 41 additions & 37 deletions

File tree

include/net/tc_act/tc_pedit.h

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@ struct tcf_pedit_key_ex {
1414
struct tcf_pedit_parms {
1515
struct tc_pedit_key *tcfp_keys;
1616
struct tcf_pedit_key_ex *tcfp_keys_ex;
17-
u32 tcfp_off_max_hint;
1817
unsigned char tcfp_nkeys;
1918
unsigned char tcfp_flags;
2019
struct rcu_head rcu;

net/sched/act_pedit.c

Lines changed: 41 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,8 @@
2020
#include <linux/ip.h>
2121
#include <linux/ipv6.h>
2222
#include <linux/slab.h>
23+
#include <linux/overflow.h>
24+
#include <asm/unaligned.h>
2325
#include <net/ipv6.h>
2426
#include <net/netlink.h>
2527
#include <net/pkt_sched.h>
@@ -236,7 +238,6 @@ static int tcf_pedit_init(struct net *net, struct nlattr *nla,
236238
goto out_free_ex;
237239
}
238240

239-
nparms->tcfp_off_max_hint = 0;
240241
nparms->tcfp_flags = parm->flags;
241242
nparms->tcfp_nkeys = parm->nkeys;
242243

@@ -264,14 +265,6 @@ static int tcf_pedit_init(struct net *net, struct nlattr *nla,
264265
BITS_PER_TYPE(int) - 1,
265266
nparms->tcfp_keys[i].shift);
266267

267-
/* The AT option can read a single byte, we can bound the actual
268-
* value with uchar max.
269-
*/
270-
cur += (0xff & offmask) >> nparms->tcfp_keys[i].shift;
271-
272-
/* Each key touches 4 bytes starting from the computed offset */
273-
nparms->tcfp_off_max_hint =
274-
max(nparms->tcfp_off_max_hint, cur + 4);
275268
}
276269

277270
p = to_pedit(*a);
@@ -314,15 +307,12 @@ static void tcf_pedit_cleanup(struct tc_action *a)
314307
call_rcu(&parms->rcu, tcf_pedit_cleanup_rcu);
315308
}
316309

317-
static bool offset_valid(struct sk_buff *skb, int offset)
310+
static bool offset_valid(struct sk_buff *skb, int offset, int len)
318311
{
319-
if (offset > 0 && offset > skb->len)
320-
return false;
321-
322-
if (offset < 0 && -offset > skb_headroom(skb))
312+
if (offset < -(int)skb_headroom(skb))
323313
return false;
324314

325-
return true;
315+
return offset <= (int)skb->len - len;
326316
}
327317

328318
static int pedit_l4_skb_offset(struct sk_buff *skb, int *hoffset, const int header_type)
@@ -388,29 +378,22 @@ static int tcf_pedit_act(struct sk_buff *skb, const struct tc_action *a,
388378
struct tcf_pedit_key_ex *tkey_ex;
389379
struct tcf_pedit_parms *parms;
390380
struct tc_pedit_key *tkey;
391-
u32 max_offset;
392381
int i;
393382

394383
parms = rcu_dereference_bh(p->parms);
395384

396-
max_offset = (skb_transport_header_was_set(skb) ?
397-
skb_transport_offset(skb) :
398-
skb_network_offset(skb)) +
399-
parms->tcfp_off_max_hint;
400-
if (skb_ensure_writable(skb, min(skb->len, max_offset)))
401-
goto done;
402-
403385
tcf_lastuse_update(&p->tcf_tm);
404386
tcf_action_update_bstats(&p->common, skb);
405387

406388
tkey = parms->tcfp_keys;
407389
tkey_ex = parms->tcfp_keys_ex;
408390

409391
for (i = parms->tcfp_nkeys; i > 0; i--, tkey++) {
392+
int write_offset, write_len;
410393
int offset = tkey->off;
411394
int hoffset = 0;
412-
u32 *ptr, hdata;
413-
u32 val;
395+
u32 cur_val, val;
396+
u32 *ptr;
414397
int rc;
415398

416399
if (tkey_ex) {
@@ -428,13 +411,15 @@ static int tcf_pedit_act(struct sk_buff *skb, const struct tc_action *a,
428411

429412
if (tkey->offmask) {
430413
u8 *d, _d;
414+
int at_offset;
431415

432-
if (!offset_valid(skb, hoffset + tkey->at)) {
416+
if (check_add_overflow(hoffset, (int)tkey->at, &at_offset) ||
417+
!offset_valid(skb, at_offset, sizeof(_d))) {
433418
pr_info_ratelimited("tc action pedit 'at' offset %d out of bounds\n",
434419
hoffset + tkey->at);
435420
goto bad;
436421
}
437-
d = skb_header_pointer(skb, hoffset + tkey->at,
422+
d = skb_header_pointer(skb, at_offset,
438423
sizeof(_d), &_d);
439424
if (!d)
440425
goto bad;
@@ -446,31 +431,51 @@ static int tcf_pedit_act(struct sk_buff *skb, const struct tc_action *a,
446431
}
447432
}
448433

449-
if (!offset_valid(skb, hoffset + offset)) {
450-
pr_info_ratelimited("tc action pedit offset %d out of bounds\n", hoffset + offset);
434+
if (check_add_overflow(hoffset, offset, &write_offset)) {
435+
pr_info_ratelimited("tc action pedit offset overflow\n");
451436
goto bad;
452437
}
453438

454-
ptr = skb_header_pointer(skb, hoffset + offset,
455-
sizeof(hdata), &hdata);
456-
if (!ptr)
439+
if (!offset_valid(skb, write_offset, sizeof(*ptr))) {
440+
pr_info_ratelimited("tc action pedit offset %d out of bounds\n",
441+
write_offset);
457442
goto bad;
443+
}
444+
445+
if (write_offset < 0) {
446+
if (skb_cow(skb, -write_offset))
447+
goto bad;
448+
if (write_offset + (int)sizeof(*ptr) > 0) {
449+
if (skb_ensure_writable(skb,
450+
min_t(int, skb->len,
451+
write_offset + (int)sizeof(*ptr))))
452+
goto bad;
453+
}
454+
} else {
455+
if (check_add_overflow(write_offset, (int)sizeof(*ptr),
456+
&write_len))
457+
goto bad;
458+
if (skb_ensure_writable(skb, min_t(int, skb->len,
459+
write_len)))
460+
goto bad;
461+
}
462+
463+
ptr = (u32 *)(skb->data + write_offset);
464+
cur_val = get_unaligned(ptr);
458465
/* just do it, baby */
459466
switch (cmd) {
460467
case TCA_PEDIT_KEY_EX_CMD_SET:
461468
val = tkey->val;
462469
break;
463470
case TCA_PEDIT_KEY_EX_CMD_ADD:
464-
val = (*ptr + tkey->val) & ~tkey->mask;
471+
val = (cur_val + tkey->val) & ~tkey->mask;
465472
break;
466473
default:
467474
pr_info_ratelimited("tc action pedit bad command (%d)\n", cmd);
468475
goto bad;
469476
}
470477

471-
*ptr = ((*ptr & tkey->mask) ^ val);
472-
if (ptr == &hdata)
473-
skb_store_bits(skb, hoffset + offset, ptr, 4);
478+
put_unaligned((cur_val & tkey->mask) ^ val, ptr);
474479
}
475480

476481
goto done;

0 commit comments

Comments
 (0)