Skip to content

Commit bb1b0a8

Browse files
pvts-matgvrose8192
authored andcommitted
net/sched: sch_hfsc: upgrade 'rt' to 'sc' when it becomes a inner curve
jira VULN-6713 cve CVE-2023-4623 commit-author Pedro Tammela <[email protected]> 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/[email protected]/ - Correct 'Fixes' tag and merge with revert (Jakub) Cc: Christian Theune <[email protected]> Cc: Budimir Markovic <[email protected]> Fixes: b3d26c5 ("net/sched: sch_hfsc: Ensure inner classes have fsc curve") Signed-off-by: Pedro Tammela <[email protected]> Acked-by: Jamal Hadi Salim <[email protected]> Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Jakub Kicinski <[email protected]> (cherry picked from commit a13b67c) Signed-off-by: Marcin Wcisło <[email protected]>
1 parent 0de7b10 commit bb1b0a8

File tree

1 file changed

+14
-4
lines changed

1 file changed

+14
-4
lines changed

net/sched/sch_hfsc.c

+14-4
Original file line numberDiff line numberDiff line change
@@ -903,6 +903,14 @@ hfsc_change_usc(struct hfsc_class *cl, struct tc_service_curve *usc,
903903
cl->cl_flags |= HFSC_USC;
904904
}
905905

906+
static void
907+
hfsc_upgrade_rt(struct hfsc_class *cl)
908+
{
909+
cl->cl_fsc = cl->cl_rsc;
910+
rtsc_init(&cl->cl_virtual, &cl->cl_fsc, cl->cl_vt, cl->cl_total);
911+
cl->cl_flags |= HFSC_FSC;
912+
}
913+
906914
static const struct nla_policy hfsc_policy[TCA_HFSC_MAX + 1] = {
907915
[TCA_HFSC_RSC] = { .len = sizeof(struct tc_service_curve) },
908916
[TCA_HFSC_FSC] = { .len = sizeof(struct tc_service_curve) },
@@ -1012,10 +1020,6 @@ hfsc_change_class(struct Qdisc *sch, u32 classid, u32 parentid,
10121020
if (parent == NULL)
10131021
return -ENOENT;
10141022
}
1015-
if (!(parent->cl_flags & HFSC_FSC) && parent != &q->root) {
1016-
NL_SET_ERR_MSG(extack, "Invalid parent - parent class must have FSC");
1017-
return -EINVAL;
1018-
}
10191023

10201024
if (classid == 0 || TC_H_MAJ(classid ^ sch->handle) != 0)
10211025
return -EINVAL;
@@ -1066,6 +1070,12 @@ hfsc_change_class(struct Qdisc *sch, u32 classid, u32 parentid,
10661070
cl->cf_tree = RB_ROOT;
10671071

10681072
sch_tree_lock(sch);
1073+
/* Check if the inner class is a misconfigured 'rt' */
1074+
if (!(parent->cl_flags & HFSC_FSC) && parent != &q->root) {
1075+
NL_SET_ERR_MSG(extack,
1076+
"Forced curve change on parent 'rt' to 'sc'");
1077+
hfsc_upgrade_rt(parent);
1078+
}
10691079
qdisc_class_hash_insert(&q->clhash, &cl->cl_common);
10701080
list_add_tail(&cl->siblings, &parent->children);
10711081
if (parent->level == 0)

0 commit comments

Comments
 (0)