[PATCH net] net/sched: etf: propagate dequeue drops to parents

[PATCH net] net/sched: etf: propagate dequeue drops to parents

[Riley]

From: Riley Kitchen <security@copperfault.com>

etf_dequeue_timesortedlist() drops expired packets via
timesortedlist_drop(), but that path only decrements ETF's own qlen and
backlog. It never calls qdisc_tree_reduce_backlog(), so a classful
parent keeps accounting packets that no longer exist.

With ETF installed below QFQ, this leaves QFQ with a nonzero qlen after
its last active aggregate has drained. qfq_dequeue() then skips its
empty-queue early return, qfq_choose_next_agg() returns NULL, and
qfq_peek_skb() dereferences the NULL aggregate, panicking the kernel. An
unprivileged (uid 1000) process can trigger this deterministically after
creating user and network namespaces; no race and no modified kernel are
required.

Count the packets and bytes dropped in timesortedlist_drop() and call
qdisc_tree_reduce_backlog() once the local queue state is consistent, so
classful ancestors see the removal.

The issue was found and the fix developed with AI assistance. A
standalone C reproducer, run as uid 1000 with no kernel changes,
panicked v7.3 and mainline d135ab453a74 under KASAN; with this fix the
reproducer exits cleanly and parent and child backlogs both reach zero.
The reproducer is available privately on request.

Fixes: 25db26a91364 ("net/sched: Introduce the ETF Qdisc")
Cc: stable@vger.kernel.org
Signed-off-by: Riley Kitchen <security@copperfault.com>
---
 net/sched/sch_etf.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/net/sched/sch_etf.c b/net/sched/sch_etf.c
index aaafcf640bc..7e7532ea099 100644
--- a/net/sched/sch_etf.c
+++ b/net/sched/sch_etf.c
@@ -201,6 +201,7 @@ static int etf_enqueue_timesortedlist(struct sk_buff *nskb, struct Qdisc *sch,
 static void timesortedlist_drop(struct Qdisc *sch, struct sk_buff *skb,
 				ktime_t now)
 {
+	unsigned int dropped_pkts = 0, dropped_bytes = 0;
 	struct etf_sched_data *q = qdisc_priv(sch);
 	struct sk_buff *to_free = NULL;
 	struct sk_buff *tmp = NULL;
@@ -220,6 +221,8 @@ static void timesortedlist_drop(struct Qdisc *sch, struct sk_buff *skb,
 
 		report_sock_error(skb, ECANCELED, SO_EE_CODE_TXTIME_MISSED);
 
+		dropped_pkts++;
+		dropped_bytes += qdisc_pkt_len(skb);
 		qdisc_qstats_backlog_dec(sch, skb);
 		qdisc_drop(skb, sch, &to_free);
 		qdisc_qstats_overlimit(sch);
@@ -227,6 +230,7 @@ static void timesortedlist_drop(struct Qdisc *sch, struct sk_buff *skb,
 	}
 
 	kfree_skb_list(to_free);
+	qdisc_tree_reduce_backlog(sch, dropped_pkts, dropped_bytes);
 }
 
 static void timesortedlist_remove(struct Qdisc *sch, struct sk_buff *skb)
-- 
2.43.0

Re: [PATCH net] net/sched: etf: propagate dequeue drops to parents

[Vinicius Costa Gomes]

> From: Riley Kitchen <security@copperfault.com>
>
> etf_dequeue_timesortedlist() drops expired packets via
> timesortedlist_drop(), but that path only decrements ETF's own qlen and
> backlog. It never calls qdisc_tree_reduce_backlog(), so a classful
> parent keeps accounting packets that no longer exist.
>
> With ETF installed below QFQ, this leaves QFQ with a nonzero qlen after
> its last active aggregate has drained. qfq_dequeue() then skips its
> empty-queue early return, qfq_choose_next_agg() returns NULL, and
> qfq_peek_skb() dereferences the NULL aggregate, panicking the kernel. An
> unprivileged (uid 1000) process can trigger this deterministically after
> creating user and network namespaces; no race and no modified kernel are
> required.
>
> Count the packets and bytes dropped in timesortedlist_drop() and call
> qdisc_tree_reduce_backlog() once the local queue state is consistent, so
> classful ancestors see the removal.
>
> The issue was found and the fix developed with AI assistance. A
> standalone C reproducer, run as uid 1000 with no kernel changes,
> panicked v7.3 and mainline d135ab453a74 under KASAN; with this fix the
> reproducer exits cleanly and parent and child backlogs both reach zero.
> The reproducer is available privately on request.
>
> Fixes: 25db26a91364 ("net/sched: Introduce the ETF Qdisc")
> Cc: stable@vger.kernel.org
> Signed-off-by: Riley Kitchen <security@copperfault.com>
> ---

It seems that ETF under QFQ won't work well, as ETF can drop packets and
is not work conserving. But the patch looks good:

Acked-by: Vinicius Costa Gomes <vinicius.gomes@intel.com>


Cheers,
-- 
Vinicius