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

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

[Jamal Hadi Salim]

gred_enqueue() admits a packet when the current backlog plus the packet
length fits within the queue limit:

  sch->qstats.backlog + qdisc_pkt_len(skb) <= sch->limit   (default VQ)
  gred_backlog(t, q, sch) + qdisc_pkt_len(skb) <= q->limit (configured VQ)

sch->qstats.backlog and q->backlog are u32, and qdisc_pkt_len() returns
unsigned int, so both sums are computed in 32 bits and wrap at 2^32.
Once the true backlog exceeds 4 GiB the wrapped sum becomes small and
admission keeps succeeding, so the queue grows without bound and the
kernel can be driven to OOM.

Promote the sums to u64 so admission stops once the true backlog exceeds
the limit.  The limit is u32, so the bounded queue stays below 2^32 and
the stored u32 backlog never wraps.

Fixes: a3eb95f891d6 ("net_sched: gred: add TCA_GRED_LIMIT attribute")
Reported-by: vega@nebusec.ai
Tested-by: Victor Nogueira <victor@mojatatu.com>
Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
---
 net/sched/sch_gred.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/sched/sch_gred.c b/net/sched/sch_gred.c
index fcc1a4c03636..f04f425c6c44 100644
--- a/net/sched/sch_gred.c
+++ b/net/sched/sch_gred.c
@@ -179,7 +179,7 @@ static int gred_enqueue(struct sk_buff *skb, struct Qdisc *sch,
 			 * if no default DP has been configured. This
 			 * allows for DP flows to be left untouched.
 			 */
-			if (likely(sch->qstats.backlog + qdisc_pkt_len(skb) <=
+			if (likely((u64)sch->qstats.backlog + qdisc_pkt_len(skb) <=
 					sch->limit))
 				return qdisc_enqueue_tail(skb, sch);
 			else
@@ -244,7 +244,7 @@ static int gred_enqueue(struct sk_buff *skb, struct Qdisc *sch,
 		break;
 	}
 
-	if (gred_backlog(t, q, sch) + qdisc_pkt_len(skb) <= q->limit) {
+	if ((u64)gred_backlog(t, q, sch) + qdisc_pkt_len(skb) <= q->limit) {
 		q->backlog += qdisc_pkt_len(skb);
 		return qdisc_enqueue_tail(skb, sch);
 	}
-- 
2.34.1

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

[Zhan Xusheng]

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

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

[Jamal Hadi Salim]

On Sun, Aug 9, 2026 at 5:43 AM Zhan Xusheng <zhanxusheng1024@gmail.com> wrote:
>
> 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.
>

Looking at the code, I can confirm you are correct. I notice also plug
and sfq have a similar issue.

> 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.
>

I should have paid closer attention instead of focussing on the bug report.
Since i already submitted, we can wait to see sashiko says - if
requires an update i can include the others; otherwise it will have to
be a followup patch.

cheers,
jamal

> 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