Thread (3 messages) flat view 3 messages, 2 authors, 3d ago

Re: [PATCH net] net: sched: gred: fix 32-bit backlog wrap in gred_enqueue

From: Zhan Xusheng <hidden>
Date: 2026-08-09 09:43:32
Also in: stable

On Sun,  9 Aug 2026 05:16:57 -0400, Jamal Hadi Salim wrote:
-	if (likely(sch->qstats.backlog + qdisc_pkt_len(skb) <=
+	if (likely((u64)sch->qstats.backlog + qdisc_pkt_len(skb) <=
 		    sch->limit))
bfifo_enqueue() in net/sched/sch_fifo.c has the same expression, and this
patch does not touch it:

	if (likely(sch->qstats.backlog + qdisc_pkt_len(skb) <=
		   READ_ONCE(sch->limit)))

Same u32 backlog, same unsigned int length.  sch->limit comes from
tc_fifo_qopt.limit, a __u32 documented as "bytes for bfifo", and nothing
caps it on the way in -- both .init and .change are fifo_init(), which
stores ctl->limit directly.

The gred check is the newer of the two: it came in with a3eb95f891d6,
the commit in your Fixes tag, while the bfifo one goes back to the
initial git import, so there is no useful Fixes: tag for it.

bfifo also has more ways in than gred.  sch_red.c and sch_tbf.c install a
bfifo child through fifo_create_dflt() -> fifo_set_limit(), passing
ctl->limit and qopt->limit straight from userspace; tc_red_qopt.limit is
likewise documented as bytes.  Since this is heading to stable, fixing
gred alone leaves those paths unchanged.

Minor, and in the other direction: __fifo_init() does

	u32 limit = qdisc_dev(sch)->tx_queue_len;

	if (is_bfifo)
		limit *= psched_mtu(qdisc_dev(sch));

which also wraps in 32 bits, but fails safe -- the result stays below
2^32, so the limit ends up smaller than intended rather than unbounded.

Thanks,
Zhan Xusheng
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help