net/sched: fix pedit partial COW leading to page cache corruption

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 <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>

diff --git a/include/net/tc_act/tc_pedit.h b/include/net/tc_act/tc_pedit.h
index f58ee15cd858cf0291565c92a73e35c1f590431b..cb7b82f2cbc7fd8778a262edf4813a2734cb6127 100644 (file)
--- a/include/net/tc_act/tc_pedit.h
+++ b/include/net/tc_act/tc_pedit.h
@@ -15,7 +15,6 @@ struct tcf_pedit_parms {
        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;

diff --git a/net/sched/act_pedit.c b/net/sched/act_pedit.c
index bc20f08a27890167ed223faef241952532051284..bd3b1da3cd63b5b446abaf249b46f5bfcd0e876c 100644 (file)
--- a/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 @@ static int tcf_pedit_init(struct net *net, struct nlattr *nla,
                goto out_free_ex;
        }
 
-       nparms->tcfp_off_max_hint = 0;
        nparms->tcfp_flags = parm->flags;
        nparms->tcfp_nkeys = parm->nkeys;
 
@@ -268,14 +269,6 @@ static int tcf_pedit_init(struct net *net, struct nlattr *nla,
                                                   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);
@@ -318,15 +311,12 @@ static void tcf_pedit_cleanup(struct tc_action *a)
                call_rcu(&parms->rcu, tcf_pedit_cleanup_rcu);
 }
 
-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)
-               return false;
-
-       if  (offset < 0 && -offset > skb_headroom(skb))
+       if (offset < -(int)skb_headroom(skb))
                return false;
 
-       return true;
+       return offset <= (int)skb->len - len;
 }
 
 static int pedit_l4_skb_offset(struct sk_buff *skb, int *hoffset, const int header_type)
@@ -393,18 +383,10 @@ TC_INDIRECT_SCOPE int tcf_pedit_act(struct sk_buff *skb,
        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);
 
-       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);
 
@@ -412,10 +394,11 @@ 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 offset = tkey->off;
                int hoffset = 0;
-               u32 *ptr, hdata;
-               u32 val;
+               u32 cur_val, val;
+               u32 *ptr;
                int rc;
 
                if (tkey_ex) {
@@ -433,13 +416,15 @@ TC_INDIRECT_SCOPE int tcf_pedit_act(struct sk_buff *skb,
 
                if (tkey->offmask) {
                        u8 *d, _d;
+                       int at_offset;
 
-                       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;
                        }
-                       d = skb_header_pointer(skb, hoffset + tkey->at,
+                       d = skb_header_pointer(skb, at_offset,
                                               sizeof(_d), &_d);
                        if (!d)
                                goto bad;
@@ -451,31 +436,51 @@ TC_INDIRECT_SCOPE int tcf_pedit_act(struct sk_buff *skb,
                        }
                }
 
-               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;
                }
 
-               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_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);
+               put_unaligned((cur_val & tkey->mask) ^ val, ptr);
        }
 
        goto done;