diff options
author | Jiri Pirko <jiri@mellanox.com> | 2017-09-13 11:32:37 -0400 |
---|---|---|
committer | David S. Miller <davem@davemloft.net> | 2017-09-13 12:34:08 -0400 |
commit | 255cd50f207ae8ec7b22663246c833407744e634 (patch) | |
tree | 8c98c50e192e780bfa9d845b4c7e416cb3ae9d3a | |
parent | 822f8565c93949fb2d31502d595c8bc45629c9b7 (diff) |
net: sched: fix use-after-free in tcf_action_destroy and tcf_del_walker
Recent commit d7fb60b9cafb ("net_sched: get rid of tcfa_rcu") removed
freeing in call_rcu, which changed already existing hard-to-hit
race condition into 100% hit:
[ 598.599825] BUG: unable to handle kernel NULL pointer dereference at 0000000000000030
[ 598.607782] IP: tcf_action_destroy+0xc0/0x140
Or:
[ 40.858924] BUG: unable to handle kernel NULL pointer dereference at 0000000000000030
[ 40.862840] IP: tcf_generic_walker+0x534/0x820
Fix this by storing the ops and use them directly for module_put call.
Fixes: a85a970af265 ("net_sched: move tc_action into tcf_common")
Signed-off-by: Jiri Pirko <jiri@mellanox.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
-rw-r--r-- | net/sched/act_api.c | 6 |
1 files changed, 4 insertions, 2 deletions
diff --git a/net/sched/act_api.c b/net/sched/act_api.c index fcd7dc7b807a..da6fa82c98a8 100644 --- a/net/sched/act_api.c +++ b/net/sched/act_api.c | |||
@@ -180,7 +180,7 @@ static int tcf_del_walker(struct tcf_idrinfo *idrinfo, struct sk_buff *skb, | |||
180 | idr_for_each_entry_ext(idr, p, id) { | 180 | idr_for_each_entry_ext(idr, p, id) { |
181 | ret = __tcf_idr_release(p, false, true); | 181 | ret = __tcf_idr_release(p, false, true); |
182 | if (ret == ACT_P_DELETED) { | 182 | if (ret == ACT_P_DELETED) { |
183 | module_put(p->ops->owner); | 183 | module_put(ops->owner); |
184 | n_i++; | 184 | n_i++; |
185 | } else if (ret < 0) { | 185 | } else if (ret < 0) { |
186 | goto nla_put_failure; | 186 | goto nla_put_failure; |
@@ -514,13 +514,15 @@ EXPORT_SYMBOL(tcf_action_exec); | |||
514 | 514 | ||
515 | int tcf_action_destroy(struct list_head *actions, int bind) | 515 | int tcf_action_destroy(struct list_head *actions, int bind) |
516 | { | 516 | { |
517 | const struct tc_action_ops *ops; | ||
517 | struct tc_action *a, *tmp; | 518 | struct tc_action *a, *tmp; |
518 | int ret = 0; | 519 | int ret = 0; |
519 | 520 | ||
520 | list_for_each_entry_safe(a, tmp, actions, list) { | 521 | list_for_each_entry_safe(a, tmp, actions, list) { |
522 | ops = a->ops; | ||
521 | ret = __tcf_idr_release(a, bind, true); | 523 | ret = __tcf_idr_release(a, bind, true); |
522 | if (ret == ACT_P_DELETED) | 524 | if (ret == ACT_P_DELETED) |
523 | module_put(a->ops->owner); | 525 | module_put(ops->owner); |
524 | else if (ret < 0) | 526 | else if (ret < 0) |
525 | return ret; | 527 | return ret; |
526 | } | 528 | } |