Skip to content

Commit 4ad01ae

Browse files
Rajat Guptaroxanan1996
authored andcommitted
net/sched: fix pedit partial COW leading to page cache corruption
jira VULN-188480 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 5cf0a90 commit 4ad01ae

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
@@ -16,6 +16,8 @@
1616
#include <linux/ip.h>
1717
#include <linux/ipv6.h>
1818
#include <linux/slab.h>
19+
#include <linux/overflow.h>
20+
#include <asm/unaligned.h>
1921
#include <net/ipv6.h>
2022
#include <net/netlink.h>
2123
#include <net/pkt_sched.h>
@@ -232,7 +234,6 @@ static int tcf_pedit_init(struct net *net, struct nlattr *nla,
232234
goto out_free_ex;
233235
}
234236

235-
nparms->tcfp_off_max_hint = 0;
236237
nparms->tcfp_flags = parm->flags;
237238
nparms->tcfp_nkeys = parm->nkeys;
238239

@@ -260,14 +261,6 @@ static int tcf_pedit_init(struct net *net, struct nlattr *nla,
260261
BITS_PER_TYPE(int) - 1,
261262
nparms->tcfp_keys[i].shift);
262263

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

273266
p = to_pedit(*a);
@@ -310,15 +303,12 @@ static void tcf_pedit_cleanup(struct tc_action *a)
310303
call_rcu(&parms->rcu, tcf_pedit_cleanup_rcu);
311304
}
312305

313-
static bool offset_valid(struct sk_buff *skb, int offset)
306+
static bool offset_valid(struct sk_buff *skb, int offset, int len)
314307
{
315-
if (offset > 0 && offset > skb->len)
316-
return false;
317-
318-
if (offset < 0 && -offset > skb_headroom(skb))
308+
if (offset < -(int)skb_headroom(skb))
319309
return false;
320310

321-
return true;
311+
return offset <= (int)skb->len - len;
322312
}
323313

324314
static int pedit_l4_skb_offset(struct sk_buff *skb, int *hoffset, const int header_type)
@@ -384,29 +374,22 @@ static int tcf_pedit_act(struct sk_buff *skb, const struct tc_action *a,
384374
struct tcf_pedit_key_ex *tkey_ex;
385375
struct tcf_pedit_parms *parms;
386376
struct tc_pedit_key *tkey;
387-
u32 max_offset;
388377
int i;
389378

390379
parms = rcu_dereference_bh(p->parms);
391380

392-
max_offset = (skb_transport_header_was_set(skb) ?
393-
skb_transport_offset(skb) :
394-
skb_network_offset(skb)) +
395-
parms->tcfp_off_max_hint;
396-
if (skb_ensure_writable(skb, min(skb->len, max_offset)))
397-
goto done;
398-
399381
tcf_lastuse_update(&p->tcf_tm);
400382
tcf_action_update_bstats(&p->common, skb);
401383

402384
tkey = parms->tcfp_keys;
403385
tkey_ex = parms->tcfp_keys_ex;
404386

405387
for (i = parms->tcfp_nkeys; i > 0; i--, tkey++) {
388+
int write_offset, write_len;
406389
int offset = tkey->off;
407390
int hoffset = 0;
408-
u32 *ptr, hdata;
409-
u32 val;
391+
u32 cur_val, val;
392+
u32 *ptr;
410393
int rc;
411394

412395
if (tkey_ex) {
@@ -424,13 +407,15 @@ static int tcf_pedit_act(struct sk_buff *skb, const struct tc_action *a,
424407

425408
if (tkey->offmask) {
426409
u8 *d, _d;
410+
int at_offset;
427411

428-
if (!offset_valid(skb, hoffset + tkey->at)) {
412+
if (check_add_overflow(hoffset, (int)tkey->at, &at_offset) ||
413+
!offset_valid(skb, at_offset, sizeof(_d))) {
429414
pr_info_ratelimited("tc action pedit 'at' offset %d out of bounds\n",
430415
hoffset + tkey->at);
431416
goto bad;
432417
}
433-
d = skb_header_pointer(skb, hoffset + tkey->at,
418+
d = skb_header_pointer(skb, at_offset,
434419
sizeof(_d), &_d);
435420
if (!d)
436421
goto bad;
@@ -442,31 +427,51 @@ static int tcf_pedit_act(struct sk_buff *skb, const struct tc_action *a,
442427
}
443428
}
444429

445-
if (!offset_valid(skb, hoffset + offset)) {
446-
pr_info_ratelimited("tc action pedit offset %d out of bounds\n", hoffset + offset);
430+
if (check_add_overflow(hoffset, offset, &write_offset)) {
431+
pr_info_ratelimited("tc action pedit offset overflow\n");
447432
goto bad;
448433
}
449434

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

467-
*ptr = ((*ptr & tkey->mask) ^ val);
468-
if (ptr == &hdata)
469-
skb_store_bits(skb, hoffset + offset, ptr, 4);
474+
put_unaligned((cur_val & tkey->mask) ^ val, ptr);
470475
}
471476

472477
goto done;

0 commit comments

Comments
 (0)