Skip to content

Commit

Permalink
net/sched: act_pedit: really ensure the skb is writable
Browse files Browse the repository at this point in the history
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: 1da177e ("Linux-2.6.12-rc2")
Acked-by: Mat Martineau <mathew.j.martineau@linux.intel.com>
Tested-by: Geliang Tang <geliang.tang@suse.com>
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
  • Loading branch information
Paolo Abeni authored and matttbe committed May 2, 2022
1 parent b140ffd commit b841c3f
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 2 deletions.
1 change: 1 addition & 0 deletions include/net/tc_act/tc_pedit.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
};
Expand Down
23 changes: 21 additions & 2 deletions net/sched/act_pedit.c
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down

0 comments on commit b841c3f

Please sign in to comment.