Re: [PATCH net v2] net/sched: serialize qdisc_rtab_list against concurrent get/put

[Eric Dumazet]

On Wed, Jul 22, 2026 at 1:47 AM Jamal Hadi Salim <jhs@xxxxxxxxxxxx> wrote:
>
> On Wed, Jul 15, 2026 at 7:41 AM Aldo Ariel Panzardo <qwe.aldo@xxxxxxxxx> wrote:
> >
> > qdisc_get_rtab() and qdisc_put_rtab() mutate the process-global singly
> > linked list qdisc_rtab_list and a plain non-atomic 'int refcnt' with no
> > lock. This was only safe because every caller historically held the RTNL
> > mutex, which serialized all rate-table lookups, inserts and frees.
> >
> > That invariant no longer holds. cls_flower sets
> > TCF_PROTO_OPS_DOIT_UNLOCKED, so tc_new_tfilter() keeps rtnl_held == false
> > for it and sets TCA_ACT_FLAGS_NO_RTNL. That flag propagates through
> > tcf_exts_validate_ex() -> tcf_action_init() -> tcf_action_init_1() ->
> > tcf_police_init(), which calls qdisc_get_rtab()/qdisc_put_rtab() with the
> > RTNL mutex NOT held. Two RTM_NEWTFILTER requests on different CPUs, each
> > adding a flower filter with a police action carrying the same rate, then
> > race on qdisc_rtab_list and on the non-atomic refcnt, leading to a
> > use-after-free / double-free of the kmalloc-2k struct qdisc_rate_table.
> > qdisc_rtab_list is a single global (not per-netns), so the corrupted
> > object is shared system-wide.
> >
> >   BUG: KASAN: slab-use-after-free in qdisc_put_rtab+0x12f/0x160
> >    qdisc_put_rtab+0x12f/0x160
> >    tcf_police_init+0xda9/0x1590
> >    tcf_action_init_1+0x460/0x6b0
> >    tcf_action_init+0x439/0xa40
> >    tcf_exts_validate_ex+0x42d/0x550
> >    fl_change+0xddd/0x7da0
> >    tc_new_tfilter+0xaa7/0x2420
> >    rtnetlink_rcv_msg+0x95e/0xe90
> >   which belongs to the cache kmalloc-2k of size 2048
> >
> > Protect qdisc_rtab_list and the refcount with a dedicated spinlock. The
> > (sleeping, GFP_KERNEL) allocation in qdisc_get_rtab() is performed before
> > taking the lock; if a concurrent inserter added an identical table in the
> > meantime the freshly allocated one is freed under the lock, so no
> > duplicate is leaked. qdisc_put_rtab() now decrements the refcount and
> > unlinks under the same lock.
> >
> > Fixes: 470502de5bdb ("net: sched: unlock rules update API")
> > Suggested-by: Eric Dumazet <edumazet@xxxxxxxxxx>
> > Signed-off-by: Aldo Ariel Panzardo <qwe.aldo@xxxxxxxxx>
>
> The fix looks reasonable to me. I also tested the poc
>  So:
> Acked-by: Jamal Hadi Salim <jhs@xxxxxxxxxxxx>

Same here, thanks!

Reviewed-by: Eric Dumazet <edumazet@xxxxxxxxxx>