summaryrefslogtreecommitdiff
path: root/net
diff options
context:
space:
mode:
authorDavide Caratti <dcaratti@redhat.com>2019-03-20 17:00:16 +0300
committerDavid S. Miller <davem@davemloft.net>2019-03-21 23:26:42 +0300
commitee3bbfe806cdb46b02cda63626cb50a7a7b19fc5 (patch)
tree33700fcef44dffdc033bf13c90604e426e4fc47d /net
parentfe384e2fa36ca084a456fd30558cccc75b4b3fbd (diff)
downloadlinux-ee3bbfe806cdb46b02cda63626cb50a7a7b19fc5.tar.xz
net/sched: let actions use RCU to access 'goto_chain'
use RCU when accessing the action chain, to avoid use after free in the traffic path when 'goto chain' is replaced on existing TC actions (see script below). Since the control action is read in the traffic path without holding the action spinlock, we need to explicitly ensure that a->goto_chain is not NULL before dereferencing (i.e it's not sufficient to rely on the value of TC_ACT_GOTO_CHAIN bits). Not doing so caused NULL dereferences in tcf_action_goto_chain_exec() when the following script: # tc chain add dev dd0 chain 42 ingress protocol ip flower \ > ip_proto udp action pass index 4 # tc filter add dev dd0 ingress protocol ip flower \ > ip_proto udp action csum udp goto chain 42 index 66 # tc chain del dev dd0 chain 42 ingress (start UDP traffic towards dd0) # tc action replace action csum udp pass index 66 was run repeatedly for several hours. Suggested-by: Cong Wang <xiyou.wangcong@gmail.com> Suggested-by: Vlad Buslov <vladbu@mellanox.com> Signed-off-by: Davide Caratti <dcaratti@redhat.com> Signed-off-by: David S. Miller <davem@davemloft.net>
Diffstat (limited to 'net')
-rw-r--r--net/sched/act_api.c18
-rw-r--r--net/sched/cls_api.c2
2 files changed, 11 insertions, 9 deletions
diff --git a/net/sched/act_api.c b/net/sched/act_api.c
index fe67b98ac641..5a87e271d35a 100644
--- a/net/sched/act_api.c
+++ b/net/sched/act_api.c
@@ -31,7 +31,7 @@
static void tcf_action_goto_chain_exec(const struct tc_action *a,
struct tcf_result *res)
{
- const struct tcf_chain *chain = a->goto_chain;
+ const struct tcf_chain *chain = rcu_dereference_bh(a->goto_chain);
res->goto_tp = rcu_dereference_bh(chain->filter_chain);
}
@@ -91,13 +91,11 @@ end:
EXPORT_SYMBOL(tcf_action_check_ctrlact);
struct tcf_chain *tcf_action_set_ctrlact(struct tc_action *a, int action,
- struct tcf_chain *newchain)
+ struct tcf_chain *goto_chain)
{
- struct tcf_chain *oldchain = a->goto_chain;
-
a->tcfa_action = action;
- a->goto_chain = newchain;
- return oldchain;
+ rcu_swap_protected(a->goto_chain, goto_chain, 1);
+ return goto_chain;
}
EXPORT_SYMBOL(tcf_action_set_ctrlact);
@@ -108,7 +106,7 @@ EXPORT_SYMBOL(tcf_action_set_ctrlact);
*/
static void free_tcf(struct tc_action *p)
{
- struct tcf_chain *chain = p->goto_chain;
+ struct tcf_chain *chain = rcu_dereference_protected(p->goto_chain, 1);
free_percpu(p->cpu_bstats);
free_percpu(p->cpu_bstats_hw);
@@ -686,6 +684,10 @@ repeat:
return TC_ACT_OK;
}
} else if (TC_ACT_EXT_CMP(ret, TC_ACT_GOTO_CHAIN)) {
+ if (unlikely(!rcu_access_pointer(a->goto_chain))) {
+ net_warn_ratelimited("can't go to NULL chain!\n");
+ return TC_ACT_SHOT;
+ }
tcf_action_goto_chain_exec(a, res);
}
@@ -931,7 +933,7 @@ struct tc_action *tcf_action_init_1(struct net *net, struct tcf_proto *tp,
module_put(a_o->owner);
if (TC_ACT_EXT_CMP(a->tcfa_action, TC_ACT_GOTO_CHAIN) &&
- !a->goto_chain) {
+ !rcu_access_pointer(a->goto_chain)) {
tcf_action_destroy_1(a, bind);
NL_SET_ERR_MSG(extack, "can't use goto chain with NULL chain");
return ERR_PTR(-EINVAL);
diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
index dc10525e90e7..99ae30c177c7 100644
--- a/net/sched/cls_api.c
+++ b/net/sched/cls_api.c
@@ -367,7 +367,7 @@ static void tcf_chain_destroy(struct tcf_chain *chain, bool free_block)
struct tcf_block *block = chain->block;
mutex_destroy(&chain->filter_chain_lock);
- kfree(chain);
+ kfree_rcu(chain, rcu);
if (free_block)
tcf_block_destroy(block);
}