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

[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@vger.kernel.org
Signed-off-by: Chengfeng Ye <nicoyip.dev@gmail.com>
---
 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

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

[Jamal Hadi Salim]

On Fri, Jul 31, 2026 at 11:30 AM Chengfeng Ye <nicoyip.dev@gmail.com> wrote:
>
> 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.
>

Please always send a reproducer - either as a tdc test case or if it
is sensitive send it privately to me and Cc the other maintainers.
I will not look at it without a repro.

cheers,
jamal
> Fixes: 92149190067d ("net: sched: flower: set unlocked flag for flower proto ops")
> Cc: stable@vger.kernel.org
> Signed-off-by: Chengfeng Ye <nicoyip.dev@gmail.com>
> ---
>  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
>