Netdev List
 help / color / mirror / Atom feed
* [PATCH] net/sched: sch_netem: prevent packet length underflow with negative overhead
@ 2026-10-08  3:56 Bui Viet Dung
  2026-10-08  3:59 ` netdev-bot+sinfo
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Bui Viet Dung @ 2026-10-08  3:56 UTC (permalink / raw)
  To: Stephen Hemminger, Jamal Hadi Salim, Jiri Pirko, David S . Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni
  Cc: Simon Horman, netdev, linux-kernel, stable, Bui Viet Dung

packet_time_ns() calculates transmission time for netem rate shaping.
It adds q->packet_overhead (signed 32-bit int) to len (unsigned
64-bit int).

When userspace configures a negative packet overhead (via
TCA_NETEM_RATE packet_overhead attribute) and an enqueued packet's
length is smaller than the absolute overhead, (s64)len +
q->packet_overhead is negative. Because len is u64, the addition wraps
into an astronomical value (~2^64 - 1). When multiplied by NSEC_PER_SEC
and divided by q->rate, the calculated delay spans hours or days,
causing enqueued packets to be frozen in the qdisc indefinitely and
stalling transmission.

A similar underflow can occur in cell length calculation when
(q->cell_size + q->cell_overhead) is non-positive.

Clamp the effective packet length to 0 if the overhead adjustment
would underflow, and return 0 delay if effective cell length is
non-positive, matching the behavior in sch_cake and sch_tbf.

Fixes: 7bc0f28c7a0c ("netem: rate extension")
Cc: stable@vger.kernel.org
Signed-off-by: Bui Viet Dung <dungvn2345@gmail.com>
---
 net/sched/sch_netem.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/net/sched/sch_netem.c b/net/sched/sch_netem.c
index 8a1b89a289d..a3dcd33bec4 100644
--- a/net/sched/sch_netem.c
+++ b/net/sched/sch_netem.c
@@ -366,14 +366,20 @@ static s64 tabledist(s64 mu, s32 sigma,
 
 static u64 packet_time_ns(u64 len, const struct netem_sched_data *q)
 {
+	if ((s64)len + q->packet_overhead <= 0)
+		return 0;
+
 	len += q->packet_overhead;
 
 	if (q->cell_size) {
 		u32 cells = reciprocal_divide(len, q->cell_size_reciprocal);
+		s32 cell_len = (s32)q->cell_size + q->cell_overhead;
 
 		if (len > cells * q->cell_size)	/* extra cell needed for remainder */
 			cells++;
-		len = cells * (q->cell_size + q->cell_overhead);
+		if (cell_len <= 0)
+			return 0;
+		len = (u64)cells * cell_len;
 	}
 
 	return div64_u64(len * NSEC_PER_SEC, q->rate);
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] net/sched: sch_netem: prevent packet length underflow with negative overhead
  2026-10-08  3:56 [PATCH] net/sched: sch_netem: prevent packet length underflow with negative overhead Bui Viet Dung
@ 2026-10-08  3:59 ` netdev-bot+sinfo
  2026-10-11  0:03 ` Stephen Hemminger
  2026-10-12  4:27 ` netdev-bot+sashiko
  2 siblings, 0 replies; 5+ messages in thread
From: netdev-bot+sinfo @ 2026-10-08  3:59 UTC (permalink / raw)
  To: Bui Viet Dung
  Cc: Stephen Hemminger, Jamal Hadi Salim, Jiri Pirko, David S . Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Simon Horman, netdev,
	linux-kernel, stable

Hi!

This is an automated message. This series looks like a fix, but its
commit messages seem to be missing some information:

 - How the issue was discovered, e.g. hit in production, hit during
   development, syzbot report, manual code inspection, LLM or static
   analysis tool scan.

 - Whether the issue was actually triggered, or is only theoretical
   (e.g. found by code inspection). If it was triggered please include
   the symptoms, like the stack trace or error messages.

Please do not repost the series just to address the above. Instead,
reply to this email with the missing information, so that reviewers
can take it into account. If the series needs another revision for
other reasons, please include the information in the commit messages
then.

The evaluation is done by an LLM so it may be wrong, if you think
that is the case please reply and explain.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] net/sched: sch_netem: prevent packet length underflow with negative overhead
  2026-10-08  3:56 [PATCH] net/sched: sch_netem: prevent packet length underflow with negative overhead Bui Viet Dung
  2026-10-08  3:59 ` netdev-bot+sinfo
@ 2026-10-11  0:03 ` Stephen Hemminger
  2026-10-12  4:27 ` netdev-bot+sashiko
  2 siblings, 0 replies; 5+ messages in thread
From: Stephen Hemminger @ 2026-10-11  0:03 UTC (permalink / raw)
  To: Bui Viet Dung
  Cc: Jamal Hadi Salim, Jiri Pirko, David S . Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Simon Horman, netdev, linux-kernel,
	stable

On Thu,  8 Oct 2026 10:56:56 +0700
Bui Viet Dung <dungvn2345@gmail.com> wrote:

> packet_time_ns() calculates transmission time for netem rate shaping.
> It adds q->packet_overhead (signed 32-bit int) to len (unsigned
> 64-bit int).
> 
> When userspace configures a negative packet overhead (via
> TCA_NETEM_RATE packet_overhead attribute) and an enqueued packet's
> length is smaller than the absolute overhead, (s64)len +
> q->packet_overhead is negative. Because len is u64, the addition wraps
> into an astronomical value (~2^64 - 1). When multiplied by NSEC_PER_SEC
> and divided by q->rate, the calculated delay spans hours or days,
> causing enqueued packets to be frozen in the qdisc indefinitely and
> stalling transmission.
> 
> A similar underflow can occur in cell length calculation when
> (q->cell_size + q->cell_overhead) is non-positive.
> 
> Clamp the effective packet length to 0 if the overhead adjustment
> would underflow, and return 0 delay if effective cell length is
> non-positive, matching the behavior in sch_cake and sch_tbf.
> 
> Fixes: 7bc0f28c7a0c ("netem: rate extension")
> Cc: stable@vger.kernel.org
> Signed-off-by: Bui Viet Dung <dungvn2345@gmail.com>
> ---

I would rather cover this corner case in two ways:

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] net/sched: sch_netem: prevent packet length underflow with negative overhead
  2026-10-08  3:56 [PATCH] net/sched: sch_netem: prevent packet length underflow with negative overhead Bui Viet Dung
  2026-10-08  3:59 ` netdev-bot+sinfo
  2026-10-11  0:03 ` Stephen Hemminger
@ 2026-10-12  4:27 ` netdev-bot+sashiko
  2026-10-12  4:37   ` Bui Viet Dung
  2 siblings, 1 reply; 5+ messages in thread
From: netdev-bot+sashiko @ 2026-10-12  4:27 UTC (permalink / raw)
  To: dungvn2345
  Cc: stephen, jhs, jiri, davem, edumazet, kuba, pabeni, horms, netdev,
	linux-kernel, stable

Thank you for your contribution! Sashiko AI review found 3 potential
issue(s) to consider.

Critical: 0 · High: 0 · Medium: 1 · Low: 2

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] In net/sched/sch_netem.c, packet_time_ns() line 376 now does
  `s32 cell_len = (s32)q->cell_size + q->cell_overhead;`, and lines…
- [Low] In net/sched/sch_netem.c, packet_time_ns() now computes the cell-
  adjusted length as `len = (u64)cells * cell_len;` (line 382) instead…
- [Low] The commit message says the new clamping is 'matching the
  behavior in sch_cake and sch_tbf'.

--

net/sched: sch_netem: prevent packet length underflow with negative overhead

This patch changes packet_time_ns() in sch_netem. It now returns 0 when
the packet length plus a negative packet_overhead is non-positive, and
also when cell_size + cell_overhead is non-positive. The cell-adjusted
length is now computed as a u64 product instead of a u32 one.

> Clamp the effective packet length to 0 if the overhead adjustment
> would underflow, and return 0 delay if effective cell length is
> non-positive, matching the behavior in sch_cake and sch_tbf.

[Severity: Low]
Is the comparison to sch_cake and sch_tbf accurate?

sch_tbf uses psched_l2t_ns() with struct psched_ratecfg, where overhead is
a u16. Its overhead cannot be negative, so there is no negative-overhead
behavior to match there.

sch_cake's cake_calc_overhead() clamps to the MPU rather than to zero:

	len = max((s32)len + q->rate_overhead, (s32)q->rate_mpu);

Its ATM/PTM framing is fixed, so it has no configurable cell size and no
rule like "return 0 delay when cell_size + cell_overhead <= 0".

The commit message also doesn't mention that the cell product is now
computed as (u64)cells * cell_len instead of in wrapping u32 arithmetic.
That changes the computed delay for large configurations.

Since this is tagged for stable, could the message describe this as a new
policy (a non-positive effective length means no rate shaping for that
packet) instead of an existing convention?

> diff --git a/net/sched/sch_netem.c b/net/sched/sch_netem.c
> index 8a1b89a289d8f..a3dcd33bec499 100644
> --- a/net/sched/sch_netem.c
> +++ b/net/sched/sch_netem.c
> @@ -366,14 +366,20 @@ static s64 tabledist(s64 mu, s32 sigma,
>  
>  static u64 packet_time_ns(u64 len, const struct netem_sched_data *q)
>  {
> +	if ((s64)len + q->packet_overhead <= 0)
> +		return 0;
> +
>  	len += q->packet_overhead;
>  
>  	if (q->cell_size) {
>  		u32 cells = reciprocal_divide(len, q->cell_size_reciprocal);
> +		s32 cell_len = (s32)q->cell_size + q->cell_overhead;
>  
>  		if (len > cells * q->cell_size)	/* extra cell needed for remainder */
>  			cells++;
> -		len = cells * (q->cell_size + q->cell_overhead);
> +		if (cell_len <= 0)
> +			return 0;

[Severity: Medium]
Can cell_len come out negative here for configurations the kernel already
accepts?

q->cell_size comes from the __u32 cell_size field of struct tc_netem_rate.
get_rate() stores it without any range check:

net/sched/sch_netem.c:get_rate() {
    ...
	q->cell_size = r->cell_size;
	q->cell_overhead = r->cell_overhead;
    ...
}

In two cases cell_len goes negative even though the real cell length is
large and positive:

  - any cell_size >= 0x80000000 turns negative after the (s32) cast
  - cell_size = 0x7fffffff with cell_overhead = 1 wraps the s32 addition
    past INT_MAX

In both cases packet_time_ns() returns 0, so this line in netem_enqueue()
adds no serialization delay:

	delay += packet_time_ns(qdisc_pkt_len(skb), q);

The configured rate shaping is then silently skipped. Before this patch,
cell_size = 0x80000000 with cell_overhead = 0 still produced a large
positive delay, and netem_dump() reports the value back unchanged.

Would computing the sum in a wider type avoid this? For example:

	s64 cell_len = (s64)q->cell_size + q->cell_overhead;

Another option is to reject these values in get_rate()/netem_change()
with an extack error.

> +		len = (u64)cells * cell_len;
>  	}
>  
>  	return div64_u64(len * NSEC_PER_SEC, q->rate);

[Severity: Low]
len is now a u64 product, so it is no longer bounded by 2^32 - 1. Can
len * NSEC_PER_SEC overflow here?

Take cell_size = 1, cell_overhead = 0x7ffffffe (so cell_len = 0x7fffffff)
and a 1500 byte packet. reciprocal_value(1) gives cells = 1500, so len is
about 3.2e12. len * NSEC_PER_SEC is then about 3.2e21, which wraps u64.

With a small q->rate (for example 1 byte/s) the quotient can also exceed
S64_MAX. The s64 delay in netem_enqueue() would then turn negative:

	delay += packet_time_ns(qdisc_pkt_len(skb), q);
	...
	cb->time_to_send = now + delay;

and time_to_send would land in the past. The old u32 product capped this
at about 4.3e18, which is below S64_MAX.

The same configuration already gave a meaningless delay before this patch,
because the u32 product wrapped. The patch moves the wrap into the 64-bit
multiply and adds the negative-delay case.

Would mul_u64_u64_div_u64() or an explicit clamp make this well-defined?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261008035656.330667-1-dungvn2345%40gmail.com

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] net/sched: sch_netem: prevent packet length underflow with negative overhead
  2026-10-12  4:27 ` netdev-bot+sashiko
@ 2026-10-12  4:37   ` Bui Viet Dung
  0 siblings, 0 replies; 5+ messages in thread
From: Bui Viet Dung @ 2026-10-12  4:37 UTC (permalink / raw)
  To: netdev-bot+sashiko
  Cc: stephen, jhs, jiri, davem, edumazet, kuba, pabeni, horms, netdev,
	linux-kernel, stable

Hi,

Thanks for the review. The points are valid. I will fix the s32 casting
issue, use mul_u64_u64_div_u64() to prevent u64 multiplication overflow,
and refine the commit description in v2.

pw-bot: cr

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-10-12  4:37 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-08  3:56 [PATCH] net/sched: sch_netem: prevent packet length underflow with negative overhead Bui Viet Dung
2026-10-08  3:59 ` netdev-bot+sinfo
2026-10-11  0:03 ` Stephen Hemminger
2026-10-12  4:27 ` netdev-bot+sashiko
2026-10-12  4:37   ` Bui Viet Dung

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox