Skip to content

Commit 5c697e0

Browse files
committed
sch_htb: make htb_deactivate() idempotent
jira LE-3845 cve CVE-2025-38350 Rebuild_History Non-Buildable kernel-4.18.0-553.69.1.el8_10 commit-author Cong Wang <[email protected]> commit 3769478 Alan reported a NULL pointer dereference in htb_next_rb_node() after we made htb_qlen_notify() idempotent. It turns out in the following case it introduced some regression: htb_dequeue_tree(): |-> fq_codel_dequeue() |-> qdisc_tree_reduce_backlog() |-> htb_qlen_notify() |-> htb_deactivate() |-> htb_next_rb_node() |-> htb_deactivate() For htb_next_rb_node(), after calling the 1st htb_deactivate(), the clprio[prio]->ptr could be already set to NULL, which means htb_next_rb_node() is vulnerable here. For htb_deactivate(), although we checked qlen before calling it, in case of qlen==0 after qdisc_tree_reduce_backlog(), we may call it again which triggers the warning inside. To fix the issues here, we need to: 1) Make htb_deactivate() idempotent, that is, simply return if we already call it before. 2) Make htb_next_rb_node() safe against ptr==NULL. Many thanks to Alan for testing and for the reproducer. Fixes: 5ba8b83 ("sch_htb: make htb_qlen_notify() idempotent") Reported-by: Alan J. Wylie <[email protected]> Signed-off-by: Cong Wang <[email protected]> Link: https://patch.msgid.link/[email protected] Signed-off-by: Jakub Kicinski <[email protected]> (cherry picked from commit 3769478) Signed-off-by: Jonathan Maple <[email protected]>
1 parent 08ce88f commit 5c697e0

File tree

1 file changed

+6
-7
lines changed

1 file changed

+6
-7
lines changed

net/sched/sch_htb.c

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -353,7 +353,8 @@ static void htb_add_to_wait_tree(struct htb_sched *q,
353353
*/
354354
static inline void htb_next_rb_node(struct rb_node **n)
355355
{
356-
*n = rb_next(*n);
356+
if (*n)
357+
*n = rb_next(*n);
357358
}
358359

359360
/**
@@ -614,8 +615,8 @@ static inline void htb_activate(struct htb_sched *q, struct htb_class *cl)
614615
*/
615616
static inline void htb_deactivate(struct htb_sched *q, struct htb_class *cl)
616617
{
617-
WARN_ON(!cl->prio_activity);
618-
618+
if (!cl->prio_activity)
619+
return;
619620
htb_deactivate_prios(q, cl);
620621
cl->prio_activity = 0;
621622
}
@@ -1753,8 +1754,7 @@ static int htb_delete(struct Qdisc *sch, unsigned long arg,
17531754
if (cl->parent)
17541755
cl->parent->children--;
17551756

1756-
if (cl->prio_activity)
1757-
htb_deactivate(q, cl);
1757+
htb_deactivate(q, cl);
17581758

17591759
if (cl->cmode != HTB_CAN_SEND)
17601760
htb_safe_rb_erase(&cl->pq_node,
@@ -1970,8 +1970,7 @@ static int htb_change_class(struct Qdisc *sch, u32 classid,
19701970
/* turn parent into inner node */
19711971
qdisc_purge_queue(parent->leaf.q);
19721972
parent_qdisc = parent->leaf.q;
1973-
if (parent->prio_activity)
1974-
htb_deactivate(q, parent);
1973+
htb_deactivate(q, parent);
19751974

19761975
/* remove from evt list because of level change */
19771976
if (parent->cmode != HTB_CAN_SEND) {

0 commit comments

Comments
 (0)