Commit de508f8b authored by David S. Miller's avatar David S. Miller

Merge branch 'net-sched-fix-NULL-dereference-in-goto-chain-control-action'

Davide Caratti says:

====================
net/sched: fix NULL dereference in 'goto chain' control action

in a couple of TC actions (i.e. csum and tunnel_key), the control action
is stored together with the action-specific configuration data.
This avoids a race condition (see [1]), but it causes a crash when 'goto
chain' is used with the above actions. Since this race condition is
tolerated on the other TC actions (it's present even on actions where the
spinlock is still used), storing the control action in the common area
should be acceptable for tunnel_key and csum as well.

[1] https://www.spinics.net/lists/netdev/msg472047.html
====================
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
parents 404cd086 38230a3e
...@@ -7,7 +7,6 @@ ...@@ -7,7 +7,6 @@
#include <linux/tc_act/tc_csum.h> #include <linux/tc_act/tc_csum.h>
struct tcf_csum_params { struct tcf_csum_params {
int action;
u32 update_flags; u32 update_flags;
struct rcu_head rcu; struct rcu_head rcu;
}; };
......
...@@ -18,7 +18,6 @@ ...@@ -18,7 +18,6 @@
struct tcf_tunnel_key_params { struct tcf_tunnel_key_params {
struct rcu_head rcu; struct rcu_head rcu;
int tcft_action; int tcft_action;
int action;
struct metadata_dst *tcft_enc_metadata; struct metadata_dst *tcft_enc_metadata;
}; };
......
...@@ -91,7 +91,7 @@ static int tcf_csum_init(struct net *net, struct nlattr *nla, ...@@ -91,7 +91,7 @@ static int tcf_csum_init(struct net *net, struct nlattr *nla,
} }
params_old = rtnl_dereference(p->params); params_old = rtnl_dereference(p->params);
params_new->action = parm->action; p->tcf_action = parm->action;
params_new->update_flags = parm->update_flags; params_new->update_flags = parm->update_flags;
rcu_assign_pointer(p->params, params_new); rcu_assign_pointer(p->params, params_new);
if (params_old) if (params_old)
...@@ -561,7 +561,7 @@ static int tcf_csum(struct sk_buff *skb, const struct tc_action *a, ...@@ -561,7 +561,7 @@ static int tcf_csum(struct sk_buff *skb, const struct tc_action *a,
tcf_lastuse_update(&p->tcf_tm); tcf_lastuse_update(&p->tcf_tm);
bstats_cpu_update(this_cpu_ptr(p->common.cpu_bstats), skb); bstats_cpu_update(this_cpu_ptr(p->common.cpu_bstats), skb);
action = params->action; action = READ_ONCE(p->tcf_action);
if (unlikely(action == TC_ACT_SHOT)) if (unlikely(action == TC_ACT_SHOT))
goto drop_stats; goto drop_stats;
...@@ -599,11 +599,11 @@ static int tcf_csum_dump(struct sk_buff *skb, struct tc_action *a, int bind, ...@@ -599,11 +599,11 @@ static int tcf_csum_dump(struct sk_buff *skb, struct tc_action *a, int bind,
.index = p->tcf_index, .index = p->tcf_index,
.refcnt = p->tcf_refcnt - ref, .refcnt = p->tcf_refcnt - ref,
.bindcnt = p->tcf_bindcnt - bind, .bindcnt = p->tcf_bindcnt - bind,
.action = p->tcf_action,
}; };
struct tcf_t t; struct tcf_t t;
params = rtnl_dereference(p->params); params = rtnl_dereference(p->params);
opt.action = params->action;
opt.update_flags = params->update_flags; opt.update_flags = params->update_flags;
if (nla_put(skb, TCA_CSUM_PARMS, sizeof(opt), &opt)) if (nla_put(skb, TCA_CSUM_PARMS, sizeof(opt), &opt))
......
...@@ -36,7 +36,7 @@ static int tunnel_key_act(struct sk_buff *skb, const struct tc_action *a, ...@@ -36,7 +36,7 @@ static int tunnel_key_act(struct sk_buff *skb, const struct tc_action *a,
tcf_lastuse_update(&t->tcf_tm); tcf_lastuse_update(&t->tcf_tm);
bstats_cpu_update(this_cpu_ptr(t->common.cpu_bstats), skb); bstats_cpu_update(this_cpu_ptr(t->common.cpu_bstats), skb);
action = params->action; action = READ_ONCE(t->tcf_action);
switch (params->tcft_action) { switch (params->tcft_action) {
case TCA_TUNNEL_KEY_ACT_RELEASE: case TCA_TUNNEL_KEY_ACT_RELEASE:
...@@ -182,7 +182,7 @@ static int tunnel_key_init(struct net *net, struct nlattr *nla, ...@@ -182,7 +182,7 @@ static int tunnel_key_init(struct net *net, struct nlattr *nla,
params_old = rtnl_dereference(t->params); params_old = rtnl_dereference(t->params);
params_new->action = parm->action; t->tcf_action = parm->action;
params_new->tcft_action = parm->t_action; params_new->tcft_action = parm->t_action;
params_new->tcft_enc_metadata = metadata; params_new->tcft_enc_metadata = metadata;
...@@ -254,13 +254,13 @@ static int tunnel_key_dump(struct sk_buff *skb, struct tc_action *a, ...@@ -254,13 +254,13 @@ static int tunnel_key_dump(struct sk_buff *skb, struct tc_action *a,
.index = t->tcf_index, .index = t->tcf_index,
.refcnt = t->tcf_refcnt - ref, .refcnt = t->tcf_refcnt - ref,
.bindcnt = t->tcf_bindcnt - bind, .bindcnt = t->tcf_bindcnt - bind,
.action = t->tcf_action,
}; };
struct tcf_t tm; struct tcf_t tm;
params = rtnl_dereference(t->params); params = rtnl_dereference(t->params);
opt.t_action = params->tcft_action; opt.t_action = params->tcft_action;
opt.action = params->action;
if (nla_put(skb, TCA_TUNNEL_KEY_PARMS, sizeof(opt), &opt)) if (nla_put(skb, TCA_TUNNEL_KEY_PARMS, sizeof(opt), &opt))
goto nla_put_failure; goto nla_put_failure;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment