mirror of
https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git
synced 2026-09-22 09:34:56 +02:00
net/sched: serialize qdisc_rtab_list against concurrent get/put
[ Upstream commitf43ee0c073] 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> Signed-off-by: Sasha Levin <sashal@kernel.org> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
This commit is contained in:
committed by
Greg Kroah-Hartman
parent
db3e82da61
commit
4131dd0b6f
+19
-5
@@ -413,12 +413,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 ||
|
||||
@@ -427,15 +428,20 @@ struct qdisc_rate_table *qdisc_get_rtab(struct tc_ratespec *r,
|
||||
return NULL;
|
||||
}
|
||||
|
||||
new_rtab = kmalloc(sizeof(*new_rtab), GFP_KERNEL);
|
||||
|
||||
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), 1024)) {
|
||||
rtab->refcnt++;
|
||||
spin_unlock(&qdisc_rtab_lock);
|
||||
kfree(new_rtab);
|
||||
return rtab;
|
||||
}
|
||||
}
|
||||
|
||||
rtab = kmalloc(sizeof(*rtab), GFP_KERNEL);
|
||||
rtab = new_rtab;
|
||||
if (rtab) {
|
||||
rtab->rate = *r;
|
||||
rtab->refcnt = 1;
|
||||
@@ -447,6 +453,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);
|
||||
@@ -455,18 +462,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);
|
||||
|
||||
|
||||
Reference in New Issue
Block a user