diff --git a/include/net/tc_act/tc_pedit.h b/include/net/tc_act/tc_pedit.h index 3e02709a1df65..748cf87a4d7ea 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 823ee643371c2..eab88929298db 100644 --- a/net/sched/act_pedit.c +++ b/net/sched/act_pedit.c @@ -14,6 +14,8 @@ #include #include #include +#include +#include #include #include #include @@ -228,21 +230,10 @@ 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; @@ -276,15 +267,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, @@ -324,18 +312,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) { @@ -346,10 +326,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; + int hoffset = 0; + u32 cur_val, val; + u32 *ptr; int rc; if (tkey_ex) { @@ -361,20 +342,22 @@ static int tcf_pedit_act(struct sk_buff *skb, const struct tc_action *a, rc = pedit_skb_hdr_offset(skb, htype, &hoffset); if (rc) { - pr_info("tc action pedit bad header type specified (0x%x)\n", - htype); + pr_info_ratelimited("tc action pedit unable to extract header offset for header type (0x%x)\n", + htype); goto bad; } 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; @@ -382,37 +365,56 @@ static int tcf_pedit_act(struct sk_buff *skb, const struct tc_action *a, } if (offset % 4) { - pr_info("tc action pedit offset must be on 32 bit boundaries\n"); + pr_info_ratelimited("tc action pedit offset must be on 32 bit boundaries\n"); 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", - cmd); + 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; @@ -424,7 +426,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; }