Skip to content

Commit d6d53e5

Browse files
committed
net/sched: Always pass notifications when child class becomes empty
JIRA: https://issues.redhat.com/browse/RHEL-75598 commit 103406b Author: Lion Ackermann <nnamrec@gmail.com> Date: Mon Jun 30 15:27:30 2025 +0200 net/sched: Always pass notifications when child class becomes empty Certain classful qdiscs may invoke their classes' dequeue handler on an enqueue operation. This may unexpectedly empty the child qdisc and thus make an in-flight class passive via qlen_notify(). Most qdiscs do not expect such behaviour at this point in time and may re-activate the class eventually anyways which will lead to a use-after-free. The referenced fix commit attempted to fix this behavior for the HFSC case by moving the backlog accounting around, though this turned out to be incomplete since the parent's parent may run into the issue too. The following reproducer demonstrates this use-after-free: tc qdisc add dev lo root handle 1: drr tc filter add dev lo parent 1: basic classid 1:1 tc class add dev lo parent 1: classid 1:1 drr tc qdisc add dev lo parent 1:1 handle 2: hfsc def 1 tc class add dev lo parent 2: classid 2:1 hfsc rt m1 8 d 1 m2 0 tc qdisc add dev lo parent 2:1 handle 3: netem tc qdisc add dev lo parent 3:1 handle 4: blackhole echo 1 | socat -u STDIN UDP4-DATAGRAM:127.0.0.1:8888 tc class delete dev lo classid 1:1 echo 1 | socat -u STDIN UDP4-DATAGRAM:127.0.0.1:8888 Since backlog accounting issues leading to a use-after-frees on stale class pointers is a recurring pattern at this point, this patch takes a different approach. Instead of trying to fix the accounting, the patch ensures that qdisc_tree_reduce_backlog always calls qlen_notify when the child qdisc is empty. This solves the problem because deletion of qdiscs always involves a call to qdisc_reset() and / or qdisc_purge_queue() which ultimately resets its qlen to 0 thus causing the following qdisc_tree_reduce_backlog() to report to the parent. Note that this may call qlen_notify on passive classes multiple times. This is not a problem after the recent patch series that made all the classful qdiscs qlen_notify() handlers idempotent. Fixes: 3f98113 ("sch_hfsc: Fix qlen accounting bug when using peek in hfsc_enqueue()") Signed-off-by: Lion Ackermann <nnamrec@gmail.com> Reviewed-by: Jamal Hadi Salim <jhs@mojatatu.com> Acked-by: Cong Wang <xiyou.wangcong@gmail.com> Acked-by: Jamal Hadi Salim <jhs@mojatatu.com> Link: https://patch.msgid.link/d912cbd7-193b-4269-9857-525bee8bbb6a@gmail.com Signed-off-by: Jakub Kicinski <kuba@kernel.org> Signed-off-by: Ivan Vecera <ivecera@redhat.com>
1 parent df64916 commit d6d53e5

File tree

1 file changed

+5
-14
lines changed

1 file changed

+5
-14
lines changed

net/sched/sch_api.c

Lines changed: 5 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -779,15 +779,12 @@ static u32 qdisc_alloc_handle(struct net_device *dev)
779779

780780
void qdisc_tree_reduce_backlog(struct Qdisc *sch, int n, int len)
781781
{
782-
bool qdisc_is_offloaded = sch->flags & TCQ_F_OFFLOADED;
783782
const struct Qdisc_class_ops *cops;
784783
unsigned long cl;
785784
u32 parentid;
786785
bool notify;
787786
int drops;
788787

789-
if (n == 0 && len == 0)
790-
return;
791788
drops = max_t(int, n, 0);
792789
rcu_read_lock();
793790
while ((parentid = sch->parent)) {
@@ -796,17 +793,8 @@ void qdisc_tree_reduce_backlog(struct Qdisc *sch, int n, int len)
796793

797794
if (sch->flags & TCQ_F_NOPARENT)
798795
break;
799-
/* Notify parent qdisc only if child qdisc becomes empty.
800-
*
801-
* If child was empty even before update then backlog
802-
* counter is screwed and we skip notification because
803-
* parent class is already passive.
804-
*
805-
* If the original child was offloaded then it is allowed
806-
* to be seem as empty, so the parent is notified anyway.
807-
*/
808-
notify = !sch->q.qlen && !WARN_ON_ONCE(!n &&
809-
!qdisc_is_offloaded);
796+
/* Notify parent qdisc only if child qdisc becomes empty. */
797+
notify = !sch->q.qlen;
810798
/* TODO: perform the search on a per txq basis */
811799
sch = qdisc_lookup_rcu(qdisc_dev(sch), TC_H_MAJ(parentid));
812800
if (sch == NULL) {
@@ -815,6 +803,9 @@ void qdisc_tree_reduce_backlog(struct Qdisc *sch, int n, int len)
815803
}
816804
cops = sch->ops->cl_ops;
817805
if (notify && cops->qlen_notify) {
806+
/* Note that qlen_notify must be idempotent as it may get called
807+
* multiple times.
808+
*/
818809
cl = cops->find(sch, parentid);
819810
cops->qlen_notify(sch, cl);
820811
}

0 commit comments

Comments
 (0)