In cake_drop(), qdisc_tree_reduce_backlog() is called to decrement the qlen of the qdisc hierarchy. However, this can incorrectly reduce qlen when the dropped packet was never enqueued, leading to a possible NULL dereference (e.g., when QFQ is the parent qdisc). This happens when cake_enqueue() returns NET_XMIT_CN: the parent qdisc does not enqueue the skb, but cake_drop() still reduces backlog. This patch avoids the extra reduction by checking whether the packet was actually enqueued. It also moves qdisc_tree_reduce_backlog() out of cake_drop() to keep backlog accounting consistent. Fixes: 15de71d06a40 ("net/sched: Make cake_enqueue return NET_XMIT_CN when past buffer_limit") Signed-off-by: Xiang Mei --- v2: add missing cc v3: move qdisc_tree_reduce_backlog out of cake_drop net/sched/sch_cake.c | 40 ++++++++++++++++++++++++---------------- 1 file changed, 24 insertions(+), 16 deletions(-) diff --git a/net/sched/sch_cake.c b/net/sched/sch_cake.c index 32bacfc314c2..179cafe05085 100644 --- a/net/sched/sch_cake.c +++ b/net/sched/sch_cake.c @@ -1597,7 +1597,6 @@ static unsigned int cake_drop(struct Qdisc *sch, struct sk_buff **to_free) qdisc_drop_reason(skb, sch, to_free, SKB_DROP_REASON_QDISC_OVERLIMIT); sch->q.qlen--; - qdisc_tree_reduce_backlog(sch, 1, len); cake_heapify(q, 0); @@ -1750,7 +1749,9 @@ static s32 cake_enqueue(struct sk_buff *skb, struct Qdisc *sch, ktime_t now = ktime_get(); struct cake_tin_data *b; struct cake_flow *flow; - u32 idx, tin; + u32 dropped = 0; + u32 idx, tin, prev_qlen, prev_backlog, drop_id; + bool same_flow = false; /* choose flow to insert into */ idx = cake_classify(sch, &b, skb, q->flow_mode, &ret); @@ -1927,24 +1928,31 @@ static s32 cake_enqueue(struct sk_buff *skb, struct Qdisc *sch, if (q->buffer_used > q->buffer_max_used) q->buffer_max_used = q->buffer_used; - if (q->buffer_used > q->buffer_limit) { - bool same_flow = false; - u32 dropped = 0; - u32 drop_id; + if (q->buffer_used <= q->buffer_limit) + return NET_XMIT_SUCCESS; - while (q->buffer_used > q->buffer_limit) { - dropped++; - drop_id = cake_drop(sch, to_free); + prev_qlen = sch->q.qlen; + prev_backlog = sch->qstats.backlog; - if ((drop_id >> 16) == tin && - (drop_id & 0xFFFF) == idx) - same_flow = true; - } - b->drop_overlimit += dropped; + while (q->buffer_used > q->buffer_limit) { + dropped++; + drop_id = cake_drop(sch, to_free); + if ((drop_id >> 16) == tin && + (drop_id & 0xFFFF) == idx) + same_flow = true; + } + b->drop_overlimit += dropped; + + /* Compute the droppped qlen and pkt length */ + prev_qlen -= sch->q.qlen; + prev_backlog -= sch->qstats.backlog; - if (same_flow) - return NET_XMIT_CN; + if (same_flow) { + qdisc_tree_reduce_backlog(sch, prev_qlen - 1, + prev_backlog - len); + return NET_XMIT_CN; } + qdisc_tree_reduce_backlog(sch, prev_qlen, prev_backlog); return NET_XMIT_SUCCESS; } -- 2.43.0