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@xxxxxxxxx> 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@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
>