net/sched: serialize qdisc_rtab_list against concurrent get/put

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@google.com>
Signed-off-by: Aldo Ariel Panzardo <qwe.aldo@gmail.com>
Cc: stable@vger.kernel.org
Acked-by: Jamal Hadi Salim <jhs@mojatatu.com>
Reviewed-by: Eric Dumazet <edumazet@google.com>
Link: https://patch.msgid.link/20260715114114.446841-1-qwe.aldo@gmail.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>

diff --git a/net/sched/sch_api.c b/net/sched/sch_api.c
index 8a3236456db4734e88404ea85504838c8c8dd4ad..668bcd60d183e0c6826e78309c5a83b3cf8057fe 100644 (file)
--- a/net/sched/sch_api.c
+++ b/net/sched/sch_api.c
@@ -415,12 +415,13 @@ static __u8 __detect_linklayer(struct tc_ratespec *r, __u32 *rtab)
 }
 
 static struct qdisc_rate_table *qdisc_rtab_list;
+static DEFINE_SPINLOCK(qdisc_rtab_lock);
 
 struct qdisc_rate_table *qdisc_get_rtab(struct tc_ratespec *r,
                                        struct nlattr *tab,
                                        struct netlink_ext_ack *extack)
 {
-       struct qdisc_rate_table *rtab;
+       struct qdisc_rate_table *rtab, *new_rtab;
 
        if (tab == NULL || r->rate == 0 ||
            r->cell_log == 0 || r->cell_log >= 32 ||
@@ -429,15 +430,20 @@ struct qdisc_rate_table *qdisc_get_rtab(struct tc_ratespec *r,
                return NULL;
        }
 
+       new_rtab = kmalloc_obj(*new_rtab);
+
+       spin_lock(&qdisc_rtab_lock);
        for (rtab = qdisc_rtab_list; rtab; rtab = rtab->next) {
                if (!memcmp(&rtab->rate, r, sizeof(struct tc_ratespec)) &&
                    !memcmp(&rtab->data, nla_data(tab), TC_RTAB_SIZE)) {
                        rtab->refcnt++;
+                       spin_unlock(&qdisc_rtab_lock);
+                       kfree(new_rtab);
                        return rtab;
                }
        }
 
-       rtab = kmalloc_obj(*rtab);
+       rtab = new_rtab;
        if (rtab) {
                rtab->rate = *r;
                rtab->refcnt = 1;
@@ -449,6 +455,7 @@ struct qdisc_rate_table *qdisc_get_rtab(struct tc_ratespec *r,
        } else {
                NL_SET_ERR_MSG(extack, "Failed to allocate new qdisc rate table");
        }
+       spin_unlock(&qdisc_rtab_lock);
        return rtab;
 }
 EXPORT_SYMBOL(qdisc_get_rtab);
@@ -457,18 +464,25 @@ void qdisc_put_rtab(struct qdisc_rate_table *tab)
 {
        struct qdisc_rate_table *rtab, **rtabp;
 
-       if (!tab || --tab->refcnt)
+       if (!tab)
                return;
 
+       spin_lock(&qdisc_rtab_lock);
+       if (--tab->refcnt) {
+               spin_unlock(&qdisc_rtab_lock);
+               return;
+       }
+
        for (rtabp = &qdisc_rtab_list;
             (rtab = *rtabp) != NULL;
             rtabp = &rtab->next) {
                if (rtab == tab) {
                        *rtabp = rtab->next;
-                       kfree(rtab);
-                       return;
+                       break;
                }
        }
+       spin_unlock(&qdisc_rtab_lock);
+       kfree(tab);
 }
 EXPORT_SYMBOL(qdisc_put_rtab);