[ciqlts9_2] net/sched: fix pedit partial COW leading to page cache corruption#1390
Closed
ciq-kernel-automation[bot] wants to merge 1 commit into
Closed
[ciqlts9_2] net/sched: fix pedit partial COW leading to page cache corruption#1390ciq-kernel-automation[bot] wants to merge 1 commit into
ciq-kernel-automation[bot] wants to merge 1 commit into
Conversation
jira VULN-188480 cve CVE-2026-46331 commit-author Rajat Gupta <rajat.gupta@oss.qualcomm.com> commit 899ee91 upstream-diff | This tree predates the act_pedit RCU refactor (struct tcf_pedit_parms and the RCU 'parms' pointer), so the upstream commit does not apply cleanly. The fix is adapted to the flat struct tcf_pedit layout: parms->tcfp_* accesses become p->tcfp_*, and only the tcfp_off_max_hint field is removed from the struct. The COW-correctness change is otherwise identical to upstream -- the pre-loop skb_ensure_writable() keyed on tcfp_off_max_hint is removed, and per-key COW (skb_ensure_writable(), or skb_cow() for negative offsets) is performed inside the loop with check_add_overflow() guards and the len-aware offset_valid(). The datapath read/write switches from skb_header_pointer()/ skb_store_bits() to a direct skb->data pointer with get_unaligned()/put_unaligned(), matching upstream. Additionally, <linux/unaligned.h> does not exist in this tree (it was created by upstream commit 5f60d5f "move asm/unaligned.h to linux/unaligned.h" in v6.12, which postdates 5.14), so the include is changed to <asm/unaligned.h>, which provides the same helpers here. 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: Shreeya Patel <spatel@ciq.com> Signed-off-by: Shreeya Patel <spatel@ciq.com>
ciq-kernel-automation
Bot
added
the
created-by-kernelci
|
🤖 Validation Checks In Progress Workflow run: https://github.com/ctrliq/kernel-src-tree/actions/runs/28381998699 |
🔍 Interdiff Analysis
================================================================================
* DELTA DIFFERENCES - code changes that differ between the patches *
================================================================================
--- b/include/net/tc_act/tc_pedit.h
+++ b/include/net/tc_act/tc_pedit.h
@@ -14,6 +14,7 @@
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;
};
--- b/net/sched/act_pedit.c
+++ b/net/sched/act_pedit.c
@@ -14,8 +14,6 @@
#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>
@@ -230,10 +228,21 @@
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;
@@ -312,10 +321,18 @@
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) {
@@ -326,11 +343,10 @@
enum pedit_cmd cmd = TCA_PEDIT_KEY_EX_CMD_SET;
for (i = p->tcfp_nkeys; i > 0; i--, tkey++) {
- int write_offset, write_len;
+ u32 *ptr, hdata;
int offset = tkey->off;
- int hoffset = 0;
- u32 cur_val, val;
- u32 *ptr;
+ int hoffset;
+ u32 val;
int rc;
if (tkey_ex) {
@@ -342,22 +358,20 @@
rc = pedit_skb_hdr_offset(skb, htype, &hoffset);
if (rc) {
- pr_info_ratelimited("tc action pedit unable to extract header offset for header type (0x%x)\n",
- htype);
+ pr_info("tc action pedit bad header type specified (0x%x)\n",
+ htype);
goto bad;
}
if (tkey->offmask) {
u8 *d, _d;
- int at_offset;
- 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);
+ if (!offset_valid(skb, hoffset + tkey->at)) {
+ pr_info("tc action pedit 'at' offset %d out of bounds\n",
+ hoffset + tkey->at);
goto bad;
}
- d = skb_header_pointer(skb, at_offset,
+ d = skb_header_pointer(skb, hoffset + tkey->at,
sizeof(_d), &_d);
if (!d)
goto bad;
@@ -365,56 +379,37 @@
}
if (offset % 4) {
- pr_info_ratelimited("tc action pedit offset must be on 32 bit boundaries\n");
+ pr_info("tc action pedit offset must be on 32 bit boundaries\n");
goto bad;
}
- if (check_add_overflow(hoffset, offset, &write_offset)) {
- pr_info_ratelimited("tc action pedit offset overflow\n");
+ if (!offset_valid(skb, hoffset + offset)) {
+ pr_info("tc action pedit offset %d out of bounds\n",
+ hoffset + offset);
goto bad;
}
- if (!offset_valid(skb, write_offset, sizeof(*ptr))) {
- pr_info_ratelimited("tc action pedit offset %d out of bounds\n",
- write_offset);
+ ptr = skb_header_pointer(skb, hoffset + offset,
+ sizeof(hdata), &hdata);
+ if (!ptr)
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 = (cur_val + tkey->val) & ~tkey->mask;
+ val = (*ptr + tkey->val) & ~tkey->mask;
break;
default:
- pr_info_ratelimited("tc action pedit bad command (%d)\n",
- cmd);
+ pr_info("tc action pedit bad command (%d)\n",
+ cmd);
goto bad;
}
- put_unaligned((cur_val & tkey->mask) ^ val, ptr);
+ *ptr = ((*ptr & tkey->mask) ^ val);
+ if (ptr == &hdata)
+ skb_store_bits(skb, hoffset + offset, ptr, 4);
}
goto done;
@@ -426,6 +421,7 @@
p->tcf_qstats.overlimits++;
done:
bstats_update(&p->tcf_bstats, skb);
+unlock:
spin_unlock(&p->tcf_lock);
return p->tcf_action;
}
################################################################################
! REJECTED PATCH2 HUNKS - could not be compared; manual review needed !
################################################################################
--- b/include/net/tc_act/tc_pedit.h
+++ b/include/net/tc_act/tc_pedit.h
@@ -15,7 +15,6 @@
struct tc_pedit_key *tcfp_keys;
struct tcf_pedit_key_ex *tcfp_keys_ex;
int action;
- u32 tcfp_off_max_hint;
unsigned char tcfp_nkeys;
unsigned char tcfp_flags;
struct rcu_head rcu;
--- b/net/sched/act_pedit.c
+++ b/net/sched/act_pedit.c
@@ -16,6 +16,8 @@
#include <linux/ip.h>
#include <linux/ipv6.h>
#include <linux/slab.h>
+#include <linux/overflow.h>
+#include <linux/unaligned.h>
#include <net/ipv6.h>
#include <net/netlink.h>
#include <net/pkt_sched.h>
@@ -242,7 +244,6 @@
goto out_free_ex;
}
- nparms->tcfp_off_max_hint = 0;
nparms->tcfp_flags = parm->flags;
nparms->tcfp_nkeys = parm->nkeys;
@@ -268,14 +269,6 @@
BITS_PER_TYPE(int) - 1,
nparms->tcfp_keys[i].shift);
- /* The AT option can read a single byte, we can bound the actual
- * value with uchar max.
- */
- cur += (0xff & offmask) >> nparms->tcfp_keys[i].shift;
-
- /* Each key touches 4 bytes starting from the computed offset */
- nparms->tcfp_off_max_hint =
- max(nparms->tcfp_off_max_hint, cur + 4);
}
p = to_pedit(*a);
@@ -390,7 +380,6 @@
struct tcf_pedit_key_ex *tkey_ex;
struct tcf_pedit_parms *parms;
struct tc_pedit_key *tkey;
- u32 max_offset;
int i;
parms = rcu_dereference_bh(p->parms);
@@ -394,13 +383,6 @@
parms = rcu_dereference_bh(p->parms);
- max_offset = (skb_transport_header_was_set(skb) ?
- skb_transport_offset(skb) :
- skb_network_offset(skb)) +
- parms->tcfp_off_max_hint;
- if (skb_ensure_writable(skb, min(skb->len, max_offset)))
- goto done;
-
tcf_lastuse_update(&p->tcf_tm);
tcf_action_update_bstats(&p->common, skb);
@@ -409,6 +391,7 @@
tkey_ex = parms->tcfp_keys_ex;
for (i = parms->tcfp_nkeys; i > 0; i--, tkey++) {
+ int write_offset, write_len;
int offset = tkey->off;
int hoffset = 0;
u32 *ptr, hdata;
@@ -411,8 +394,8 @@
for (i = parms->tcfp_nkeys; i > 0; i--, tkey++) {
int offset = tkey->off;
int hoffset = 0;
- u32 *ptr, hdata;
- u32 val;
+ u32 cur_val, val;
+ u32 *ptr;
int rc;
if (tkey_ex) {
@@ -430,6 +413,7 @@
if (tkey->offmask) {
u8 *d, _d;
+ int at_offset;
if (!offset_valid(skb, hoffset + tkey->at)) {
pr_info_ratelimited("tc action pedit 'at' offset %d out of bounds\n",
@@ -431,7 +415,8 @@
if (tkey->offmask) {
u8 *d, _d;
- if (!offset_valid(skb, 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;
@@ -435,7 +420,7 @@
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;
@@ -448,8 +433,8 @@
}
}
- if (!offset_valid(skb, hoffset + offset)) {
- pr_info_ratelimited("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;
}
@@ -453,9 +438,9 @@
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;
/* just do it, baby */
switch (cmd) {
@@ -457,6 +442,28 @@
sizeof(hdata), &hdata);
if (!ptr)
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:
@@ -460,7 +467,7 @@
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_ratelimited("tc action pedit bad command (%d)\n", cmd);
@@ -464,9 +471,7 @@
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;
================================================================================
* CONTEXT DIFFERENCES - surrounding code differences between the patches *
================================================================================
--- b/include/net/tc_act/tc_pedit.h
+++ b/include/net/tc_act/tc_pedit.h
@@ -15,3 +11,7 @@
struct tc_pedit_key *tcfp_keys;
struct tcf_pedit_key_ex *tcfp_keys_ex;
-};
+ int action;
+ u32 tcfp_off_max_hint;
+ unsigned char tcfp_nkeys;
+ unsigned char tcfp_flags;
+ struct rcu_head rcu;
--- b/net/sched/act_pedit.c
+++ b/net/sched/act_pedit.c
@@ -11,5 +11,6 @@
-#include <linux/module.h>
-#include <linux/init.h>
+#include <linux/ip.h>
+#include <linux/ipv6.h>
#include <linux/slab.h>
+#include <net/ipv6.h>
#include <net/netlink.h>
#include <net/pkt_sched.h>
@@ -225,9 +228,6 @@
}
- 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);
+
+ nparms->tcfp_off_max_hint = 0;
+ nparms->tcfp_flags = parm->flags;
+ nparms->tcfp_nkeys = parm->nkeys;
+
@@ -234,11 +236,12 @@
/* 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;
+ cur += (0xff & offmask) >> nparms->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);
+ nparms->tcfp_off_max_hint =
+ max(nparms->tcfp_off_max_hint, cur + 4);
}
- p->tcfp_flags = parm->flags;
+ p = to_pedit(*a);
@@ -273,4 +310,4 @@
- kfree(p->tcfp_keys_ex);
+ call_rcu(&parms->rcu, tcf_pedit_cleanup_rcu);
}
static bool offset_valid(struct sk_buff *skb, int offset)
@@ -287,4 +329,4 @@
return true;
}
-static int pedit_skb_hdr_offset(struct sk_buff *skb,
+static int pedit_l4_skb_offset(struct sk_buff *skb, int *hoffset, const int header_type)
@@ -321,15 +366,25 @@
u32 max_offset;
int i;
- spin_lock(&p->tcf_lock);
+ parms = rcu_dereference_bh(p->parms);
max_offset = (skb_transport_header_was_set(skb) ?
skb_transport_offset(skb) :
skb_network_offset(skb)) +
- p->tcfp_off_max_hint;
+ parms->tcfp_off_max_hint;
if (skb_ensure_writable(skb, min(skb->len, max_offset)))
- goto unlock;
+ goto done;
tcf_lastuse_update(&p->tcf_tm);
+ tcf_action_update_bstats(&p->common, skb);
- if (p->tcfp_nkeys > 0) {
+ tkey_ex = parms->tcfp_keys_ex;
+
+ for (i = parms->tcfp_nkeys; i > 0; i--, tkey++) {
+ int offset = tkey->off;
+ int hoffset = 0;
+ u32 *ptr, hdata;
+ u32 val;
+ int rc;
+
+ if (tkey_ex) {
@@ -367,35 +391,21 @@
- if (offset % 4) {
- pr_info("tc action pedit offset must be on 32 bit boundaries\n");
- goto bad;
- }
+ if (tkey->offmask) {
+ u8 *d, _d;
- if (!offset_valid(skb, hoffset + offset)) {
- pr_info("tc action pedit offset %d out of bounds\n",
- hoffset + offset);
+ if (!offset_valid(skb, hoffset + tkey->at)) {
+ pr_info_ratelimited("tc action pedit 'at' offset %d out of bounds\n",
+ hoffset + tkey->at);
goto bad;
}
-
- ptr = skb_header_pointer(skb, hoffset + offset,
- sizeof(hdata), &hdata);
- if (!ptr)
- goto bad;
- /* 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;
- break;
- default:
- pr_info("tc action pedit bad command (%d)\n",
- cmd);
+ d = skb_header_pointer(skb, hoffset + tkey->at,
+ sizeof(_d), &_d);
+ if (!d)
goto bad;
}
+ }
- *ptr = ((*ptr & tkey->mask) ^ val);
- if (ptr == &hdata)
- skb_store_bits(skb, hoffset + offset, ptr, 4);
+ if (!offset_valid(skb, hoffset + offset)) {
+ pr_info_ratelimited("tc action pedit offset %d out of bounds\n", hoffset + offset);
+ goto bad;
}
@@ -403,7 +412,23 @@
- p->tcf_qstats.overlimits++;
-done:
- bstats_update(&p->tcf_bstats, skb);
-unlock:
- spin_unlock(&p->tcf_lock);
- return p->tcf_action;
-}
+ ptr = skb_header_pointer(skb, hoffset + offset,
+ sizeof(hdata), &hdata);
+ if (!ptr)
+ goto bad;
+ /* 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;
+ break;
+ default:
+ pr_info_ratelimited("tc action pedit bad command (%d)\n", cmd);
+ goto bad;
+ }
+
+ *ptr = ((*ptr & tkey->mask) ^ val);
+ if (ptr == &hdata)
+ skb_store_bits(skb, hoffset + offset, ptr, 4);
+ }
+
+ goto done;This is an automated interdiff check for backported commits. |
|
✅ Validation checks completed successfully View full results: https://github.com/ctrliq/kernel-src-tree/actions/runs/28381998699 |
Collaborator
|
Closing with priority on |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR has been automatically created after successful completion of all CI stages.
Commit Message(s)
Test Results
✅ Build Stage
✅ Boot Verification
✅ Kernel Selftests
✅ LTP Results
aarch64 regressions:
🤖 This PR was automatically generated by GitHub Actions
Run ID: 28364172149