net_sched: remove tcf_block_put_deferred()
authorCong Wang <>
Mon, 30 Oct 2017 18:10:09 +0000 (11:10 -0700)
committerDavid S. Miller <>
Tue, 31 Oct 2017 02:06:01 +0000 (11:06 +0900)
In commit 7aa0045dadb6 ("net_sched: introduce a workqueue for RCU callbacks of tc filter")
I defer tcf_chain_flush() to a workqueue, this causes a use-after-free
because qdisc is already destroyed after we queue this work.

The tcf_block_put_deferred() is no longer necessary after we get RTNL
for each tc filter destroy work, no others could jump in at this point.
Same for tcf_chain_hold(), we are fully serialized now.

This also reduces one indirection therefore makes the code more
readable. Note this brings back a rcu_barrier(), however comparing
to the code prior to commit 7aa0045dadb6 we still reduced one
rcu_barrier(). For net-next, we can consider to refcnt tcf block to
avoid it.

Fixes: 7aa0045dadb6 ("net_sched: introduce a workqueue for RCU callbacks of tc filter")
Cc: Daniel Borkmann <>
Cc: Jiri Pirko <>
Cc: John Fastabend <>
Cc: Jamal Hadi Salim <>
Cc: "Paul E. McKenney" <>
Cc: Eric Dumazet <>
Signed-off-by: Cong Wang <>
Signed-off-by: David S. Miller <>

index 231181c602edbae39fa7aecc345a12a25b35a6bb..b2d31074548724a3b59defbcd827d6538ac9bea6 100644 (file)
@@ -280,8 +280,8 @@ static void tcf_block_put_final(struct work_struct *work)
        struct tcf_block *block = container_of(work, struct tcf_block, work);
        struct tcf_chain *chain, *tmp;
-       /* At this point, all the chains should have refcnt == 1. */
+       /* Only chain 0 should be still here. */
        list_for_each_entry_safe(chain, tmp, &block->chain_list, list)
@@ -289,23 +289,17 @@ static void tcf_block_put_final(struct work_struct *work)
 /* XXX: Standalone actions are not allowed to jump to any chain, and bound
- * actions should be all removed after flushing. However, filters are destroyed
- * in RCU callbacks, we have to hold the chains first, otherwise we would
- * always race with RCU callbacks on this list without proper locking.
+ * actions should be all removed after flushing. However, filters are now
+ * destroyed in tc filter workqueue with RTNL lock, they can not race here.
-static void tcf_block_put_deferred(struct work_struct *work)
+void tcf_block_put(struct tcf_block *block)
-       struct tcf_block *block = container_of(work, struct tcf_block, work);
-       struct tcf_chain *chain;
+       struct tcf_chain *chain, *tmp;
-       rtnl_lock();
-       /* Hold a refcnt for all chains, except 0, in case they are gone. */
-       list_for_each_entry(chain, &block->chain_list, list)
-               if (chain->index)
-                       tcf_chain_hold(chain);
+       if (!block)
+               return;
-       /* No race on the list, because no chain could be destroyed. */
-       list_for_each_entry(chain, &block->chain_list, list)
+       list_for_each_entry_safe(chain, tmp, &block->chain_list, list)
        INIT_WORK(&block->work, tcf_block_put_final);
@@ -314,21 +308,6 @@ static void tcf_block_put_deferred(struct work_struct *work)
-       rtnl_unlock();
-void tcf_block_put(struct tcf_block *block)
-       if (!block)
-               return;
-       INIT_WORK(&block->work, tcf_block_put_deferred);
-       /* Wait for existing RCU callbacks to cool down, make sure their works
-        * have been queued before this. We can not flush pending works here
-        * because we are holding the RTNL lock.
-        */
-       rcu_barrier();
-       tcf_queue_work(&block->work);