Skip to content

Commit 01ce15b

Browse files
committed
net/sched: sch_hfsc: upgrade 'rt' to 'sc' when it becomes a inner curve
jira LE-1907 cve CVE-2023-4623 Rebuild_History Non-Buildable kernel-3.10.0-1160.118.1.el7 commit-author Pedro Tammela <pctammela@mojatatu.com> commit a13b67c Christian Theune says: I upgraded from 6.1.38 to 6.1.55 this morning and it broke my traffic shaping script, leaving me with a non-functional uplink on a remote router. A 'rt' curve cannot be used as a inner curve (parent class), but we were allowing such configurations since the qdisc was introduced. Such configurations would trigger a UAF as Budimir explains: The parent will have vttree_insert() called on it in init_vf(), but will not have vttree_remove() called on it in update_vf() because it does not have the HFSC_FSC flag set. The qdisc always assumes that inner classes have the HFSC_FSC flag set. This is by design as it doesn't make sense 'qdisc wise' for an 'rt' curve to be an inner curve. Budimir's original patch disallows users to add classes with a 'rt' parent, but this is too strict as it breaks users that have been using 'rt' as a inner class. Another approach, taken by this patch, is to upgrade the inner 'rt' into a 'sc', warning the user in the process. It avoids the UAF reported by Budimir while also being more permissive to bad scripts/users/code using 'rt' as a inner class. Users checking the `tc class ls [...]` or `tc class get [...]` dumps would observe the curve change and are potentially breaking with this change. v1->v2: https://lore.kernel.org/all/20231013151057.2611860-1-pctammela@mojatatu.com/ - Correct 'Fixes' tag and merge with revert (Jakub) Cc: Christian Theune <ct@flyingcircus.io> Cc: Budimir Markovic <markovicbudimir@gmail.com> Fixes: b3d26c5 ("net/sched: sch_hfsc: Ensure inner classes have fsc curve") Signed-off-by: Pedro Tammela <pctammela@mojatatu.com> Acked-by: Jamal Hadi Salim <jhs@mojatatu.com> Link: https://lore.kernel.org/r/20231017143602.3191556-1-pctammela@mojatatu.com Signed-off-by: Jakub Kicinski <kuba@kernel.org> (cherry picked from commit a13b67c) Signed-off-by: Jonathan Maple <jmaple@ciq.com>
1 parent 39170ba commit 01ce15b

File tree

1 file changed

+14
-4
lines changed

1 file changed

+14
-4
lines changed

net/sched/sch_hfsc.c

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -913,6 +913,14 @@ hfsc_change_usc(struct hfsc_class *cl, struct tc_service_curve *usc,
913913
cl->cl_flags |= HFSC_USC;
914914
}
915915

916+
static void
917+
hfsc_upgrade_rt(struct hfsc_class *cl)
918+
{
919+
cl->cl_fsc = cl->cl_rsc;
920+
rtsc_init(&cl->cl_virtual, &cl->cl_fsc, cl->cl_vt, cl->cl_total);
921+
cl->cl_flags |= HFSC_FSC;
922+
}
923+
916924
static const struct nla_policy hfsc_policy[TCA_HFSC_MAX + 1] = {
917925
[TCA_HFSC_RSC] = { .len = sizeof(struct tc_service_curve) },
918926
[TCA_HFSC_FSC] = { .len = sizeof(struct tc_service_curve) },
@@ -1020,10 +1028,6 @@ hfsc_change_class(struct Qdisc *sch, u32 classid, u32 parentid,
10201028
if (parent == NULL)
10211029
return -ENOENT;
10221030
}
1023-
if (!(parent->cl_flags & HFSC_FSC) && parent != &q->root) {
1024-
NL_SET_ERR_MSG(extack, "Invalid parent - parent class must have FSC");
1025-
return -EINVAL;
1026-
}
10271031

10281032
if (classid == 0 || TC_H_MAJ(classid ^ sch->handle) != 0)
10291033
return -EINVAL;
@@ -1076,6 +1080,12 @@ hfsc_change_class(struct Qdisc *sch, u32 classid, u32 parentid,
10761080
cl->cf_tree = RB_ROOT;
10771081

10781082
sch_tree_lock(sch);
1083+
/* Check if the inner class is a misconfigured 'rt' */
1084+
if (!(parent->cl_flags & HFSC_FSC) && parent != &q->root) {
1085+
NL_SET_ERR_MSG(extack,
1086+
"Forced curve change on parent 'rt' to 'sc'");
1087+
hfsc_upgrade_rt(parent);
1088+
}
10791089
qdisc_class_hash_insert(&q->clhash, &cl->cl_common);
10801090
list_add_tail(&cl->siblings, &parent->children);
10811091
if (parent->level == 0)

0 commit comments

Comments
 (0)