github-mirror/linux
1
0
Fork 0
mirror of https://github.com/torvalds/linux.git synced 2026-07-27 09:36:22 +02:00
Code Issues Actions Packages Projects Releases Wiki Activity Graph

net/sched: serialize qdisc_rtab_list against concurrent get/put

Browse Source 
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: 470502de5b ("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>
This commit is contained in:
Aldo Ariel Panzardo 2026-07-15 08:41:14 -03:00 committed by Jakub Kicinski
parent 92d3817649
commit f43ee0c073
1 changed files with 19 additions and 5 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

24
net/sched/sch_api.c
View File

@ -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);