[PATCH net] net/sched: keep tcf block alive during offload unbind

[Chengfeng Ye]

An RTNL-unlocked filter request can race with qdisc teardown on a shared
tcf block.  The filter request takes a temporary block reference before
teardown drops the qdisc owner's reference.  Teardown then enters
tcf_block_offload_unbind() without retaining a reference of its own.

The following interleaving can occur:

  filter request                   qdisc teardown
  tcf_block_refcnt_get(): 1 -> 2
                                   __tcf_block_put(): 2 -> 1
                                   tcf_block_offload_unbind()
  tcf_block_refcnt_put(): 1 -> 0
  tcf_block_destroy()
                                   access block->nooffloaddevcnt

The unbind path may sleep in flow_indr_dev_setup_offload(), allowing the
RCU grace period to expire and the block to be reclaimed.  It then
continues using block->cb_lock and block->nooffloaddevcnt.

KASAN reported:

  BUG: KASAN: slab-use-after-free in tcf_block_offload_unbind
  Read of size 4 at addr ffff88811ae590a4
  Call Trace:
   tcf_block_offload_unbind+0x158/0x190
   clsact_destroy+0xd6/0x690
   __qdisc_destroy+0xdf/0x290
   qdisc_graft+0x5d7/0x1560
   tc_get_qdisc+0x415/0xd50
   rtnetlink_rcv_msg+0x367/0xa40
  Allocated by task 85:
   tcf_block_get_ext+0x803/0x1570
   clsact_init+0x1ff/0xb50
   qdisc_create+0x392/0xde0
  Freed by task 88:
   __kasan_slab_free+0x43/0x70
   __rcu_free_sheaf_prepare+0x5c/0x220
   rcu_free_sheaf+0x1a/0x100

Keep the qdisc owner's reference until offload unbind completes by moving
the unbind before the refcount decrement.  A concurrent temporary
reference release can no longer destroy the block while unbind uses it.

Fixes: 92149190067d ("net: sched: flower: set unlocked flag for flower proto ops")
Cc: stable@xxxxxxxxxxxxxxx
Signed-off-by: Chengfeng Ye <nicoyip.dev@xxxxxxxxx>
---
 net/sched/cls_api.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
index fee4524adc98..d3b2506d87cf 100644
--- a/net/sched/cls_api.c
+++ b/net/sched/cls_api.c
@@ -1312,6 +1312,9 @@ static struct tcf_block *__tcf_block_find(struct net *net, struct Qdisc *q,
 static void __tcf_block_put(struct tcf_block *block, struct Qdisc *q,
 			    struct tcf_block_ext_info *ei, bool rtnl_held)
 {
+	if (q)
+		tcf_block_offload_unbind(block, q, ei);
+
 	if (refcount_dec_and_mutex_lock(&block->refcnt, &block->lock)) {
 		/* Flushing/putting all chains will cause the block to be
 		 * deallocated when last chain is freed. However, if chain_list
@@ -1325,15 +1328,10 @@ static void __tcf_block_put(struct tcf_block *block, struct Qdisc *q,
 		if (tcf_block_shared(block))
 			tcf_block_remove(block, block->net);
 
-		if (q)
-			tcf_block_offload_unbind(block, q, ei);
-
 		if (free_block)
 			tcf_block_destroy(block);
 		else
 			tcf_block_flush_all_chains(block, rtnl_held);
-	} else if (q) {
-		tcf_block_offload_unbind(block, q, ei);
 	}
 }
 
-- 
2.43.0