From b841c3f765af1e7af669ce7c1b406a60668d4b4e Mon Sep 17 00:00:00 2001 From: Paolo Abeni Date: Fri, 29 Apr 2022 19:29:17 +0200 Subject: [PATCH] net/sched: act_pedit: really ensure the skb is writable Currently pedit tries to ensure that the accessed skb offset is writeble via skb_unclone(). The action potentially allows touching any skb bytes, so it may end-up modifying shared data. The above causes some sporadic MPTCP self-test failures. Address the issue keeping track of a rough over-estimate highest skb offset accessed by the action and ensure such offset is really writable. Note that this may cause performance regressions in some scenario, but hopefully pedit is not critical path. Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2") Acked-by: Mat Martineau Tested-by: Geliang Tang Signed-off-by: Paolo Abeni --- include/net/tc_act/tc_pedit.h | 1 + net/sched/act_pedit.c | 23 +++++++++++++++++++++-- 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/include/net/tc_act/tc_pedit.h b/include/net/tc_act/tc_pedit.h index 748cf87a4d7ea..3e02709a1df65 100644 --- a/include/net/tc_act/tc_pedit.h +++ b/include/net/tc_act/tc_pedit.h @@ -14,6 +14,7 @@ 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 31fcd279c1776..a8ab6c3f1ea27 100644 --- a/net/sched/act_pedit.c +++ b/net/sched/act_pedit.c @@ -149,7 +149,7 @@ static int tcf_pedit_init(struct net *net, struct nlattr *nla, struct nlattr *pattr; struct tcf_pedit *p; int ret = 0, err; - int ksize; + int i, ksize; u32 index; if (!nla) { @@ -228,6 +228,20 @@ 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; + + /* The AT option can read a single byte, we can bound the actual + * value with uchar max. Each key touches 4 bytes starting from + * the computed offset + */ + if (p->tcfp_keys[i].offmask) { + cur += 255 >> p->tcfp_keys[i].shift; + cur = max(p->tcfp_keys[i].at, cur); + } + p->tcfp_off_max_hint = max(p->tcfp_off_max_hint, cur + 4); + } p->tcfp_flags = parm->flags; goto_ch = tcf_action_set_ctrlact(*a, parm->action, goto_ch); @@ -308,9 +322,14 @@ 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; - if (skb_unclone(skb, GFP_ATOMIC)) + 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))) return p->tcf_action; spin_lock(&p->tcf_lock);