* 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