Skip to content

Commit c83e32c

Browse files
committed
net/sched: sch_qfq: account for stab overhead in qfq_enqueue
jira VULN-6560 cve CVE-2023-3611 commit-author Pedro Tammela <[email protected]> commit 3e33708 upstream-diff used linux-stable LT-4.19 sha ee3bc82 commit 3e33708 upstream. Lion says: ------- In the QFQ scheduler a similar issue to CVE-2023-31436 persists. Consider the following code in net/sched/sch_qfq.c: static int qfq_enqueue(struct sk_buff *skb, struct Qdisc *sch, struct sk_buff **to_free) { unsigned int len = qdisc_pkt_len(skb), gso_segs; // ... if (unlikely(cl->agg->lmax < len)) { pr_debug("qfq: increasing maxpkt from %u to %u for class %u", cl->agg->lmax, len, cl->common.classid); err = qfq_change_agg(sch, cl, cl->agg->class_weight, len); if (err) { cl->qstats.drops++; return qdisc_drop(skb, sch, to_free); } // ... } Similarly to CVE-2023-31436, "lmax" is increased without any bounds checks according to the packet length "len". Usually this would not impose a problem because packet sizes are naturally limited. This is however not the actual packet length, rather the "qdisc_pkt_len(skb)" which might apply size transformations according to "struct qdisc_size_table" as created by "qdisc_get_stab()" in net/sched/sch_api.c if the TCA_STAB option was set when modifying the qdisc. A user may choose virtually any size using such a table. As a result the same issue as in CVE-2023-31436 can occur, allowing heap out-of-bounds read / writes in the kmalloc-8192 cache. ------- We can create the issue with the following commands: tc qdisc add dev $DEV root handle 1: stab mtu 2048 tsize 512 mpu 0 \ overhead 999999999 linklayer ethernet qfq tc class add dev $DEV parent 1: classid 1:1 htb rate 6mbit burst 15k tc filter add dev $DEV parent 1: matchall classid 1:1 ping -I $DEV 1.1.1.2 This is caused by incorrectly assuming that qdisc_pkt_len() returns a length within the QFQ_MIN_LMAX < len < QFQ_MAX_LMAX. Fixes: 462dbc9 ("pkt_sched: QFQ Plus: fair-queueing service at DRR cost") Reported-by: Lion <[email protected]> Reviewed-by: Eric Dumazet <[email protected]> Signed-off-by: Jamal Hadi Salim <[email protected]> Signed-off-by: Pedro Tammela <[email protected]> Reviewed-by: Simon Horman <[email protected]> Signed-off-by: Paolo Abeni <[email protected]> Signed-off-by: Shaoying Xu <[email protected]> Signed-off-by: Greg Kroah-Hartman <[email protected]> (cherry picked from commit ee3bc82) Signed-off-by: Marcin Wcisło <[email protected]>
1 parent 35d1c79 commit c83e32c

File tree

1 file changed

+6
-1
lines changed

1 file changed

+6
-1
lines changed

net/sched/sch_qfq.c

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -378,8 +378,13 @@ static int qfq_change_agg(struct Qdisc *sch, struct qfq_class *cl, u32 weight,
378378
u32 lmax)
379379
{
380380
struct qfq_sched *q = qdisc_priv(sch);
381-
struct qfq_aggregate *new_agg = qfq_find_agg(q, lmax, weight);
381+
struct qfq_aggregate *new_agg;
382382

383+
/* 'lmax' can range from [QFQ_MIN_LMAX, pktlen + stab overhead] */
384+
if (lmax > (1UL << QFQ_MTU_SHIFT))
385+
return -EINVAL;
386+
387+
new_agg = qfq_find_agg(q, lmax, weight);
383388
if (new_agg == NULL) { /* create new aggregate */
384389
new_agg = kzalloc(sizeof(*new_agg), GFP_ATOMIC);
385390
if (new_agg == NULL)

0 commit comments

Comments
 (0)