aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorRoman Kapl <code@rkapl.cz>2017-11-24 06:27:58 -0500
committerDavid S. Miller <davem@davemloft.net>2017-11-25 09:57:20 -0500
commita60b3f515d30d0fe8537c64671926879a3548103 (patch)
tree704071b30c9bb4a5fa8658d05961653625bb3651
parent540c11159dcece5c4a8157a7b39336316085470f (diff)
net: sched: crash on blocks with goto chain action
tcf_block_put_ext has assumed that all filters (and thus their goto actions) are destroyed in RCU callback and thus can not race with our list iteration. However, that is not true during netns cleanup (see tcf_exts_get_net comment). Prevent the user after free by holding all chains (except 0, that one is already held). foreach_safe is not enough in this case. To reproduce, run the following in a netns and then delete the ns: ip link add dtest type dummy tc qdisc add dev dtest ingress tc filter add dev dtest chain 1 parent ffff: handle 1 prio 1 flower action goto chain 2 Fixes: 822e86d997 ("net_sched: remove tcf_block_put_deferred()") Signed-off-by: Roman Kapl <code@rkapl.cz> Acked-by: Jiri Pirko <jiri@mellanox.com> Signed-off-by: David S. Miller <davem@davemloft.net>
-rw-r--r--net/sched/cls_api.c17
1 files changed, 12 insertions, 5 deletions
diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
index 7d97f612c9b9..ddcf04b4ab43 100644
--- a/net/sched/cls_api.c
+++ b/net/sched/cls_api.c
@@ -336,7 +336,8 @@ static void tcf_block_put_final(struct work_struct *work)
336 struct tcf_chain *chain, *tmp; 336 struct tcf_chain *chain, *tmp;
337 337
338 rtnl_lock(); 338 rtnl_lock();
339 /* Only chain 0 should be still here. */ 339
340 /* At this point, all the chains should have refcnt == 1. */
340 list_for_each_entry_safe(chain, tmp, &block->chain_list, list) 341 list_for_each_entry_safe(chain, tmp, &block->chain_list, list)
341 tcf_chain_put(chain); 342 tcf_chain_put(chain);
342 rtnl_unlock(); 343 rtnl_unlock();
@@ -344,15 +345,21 @@ static void tcf_block_put_final(struct work_struct *work)
344} 345}
345 346
346/* XXX: Standalone actions are not allowed to jump to any chain, and bound 347/* XXX: Standalone actions are not allowed to jump to any chain, and bound
347 * actions should be all removed after flushing. However, filters are now 348 * actions should be all removed after flushing.
348 * destroyed in tc filter workqueue with RTNL lock, they can not race here.
349 */ 349 */
350void tcf_block_put_ext(struct tcf_block *block, struct Qdisc *q, 350void tcf_block_put_ext(struct tcf_block *block, struct Qdisc *q,
351 struct tcf_block_ext_info *ei) 351 struct tcf_block_ext_info *ei)
352{ 352{
353 struct tcf_chain *chain, *tmp; 353 struct tcf_chain *chain;
354 354
355 list_for_each_entry_safe(chain, tmp, &block->chain_list, list) 355 /* Hold a refcnt for all chains, except 0, so that they don't disappear
356 * while we are iterating.
357 */
358 list_for_each_entry(chain, &block->chain_list, list)
359 if (chain->index)
360 tcf_chain_hold(chain);
361
362 list_for_each_entry(chain, &block->chain_list, list)
356 tcf_chain_flush(chain); 363 tcf_chain_flush(chain);
357 364
358 tcf_block_offload_unbind(block, q, ei); 365 tcf_block_offload_unbind(block, q, ei);