Re: [PATCH] net/sched: cbs: perform rate conversions in signed 64-bit arithmetic
From: Jamal Hadi Salim <jhs@mojatatu.com>
Date: 2026-08-10 15:11:20
Also in:
lkml
On Thu, Aug 6, 2026 at 11:53 AM Jack Wang [off-list ref] wrote:
cbs_set_port_rate() and cbs_change() multiply link rates and slope values by BYTES_PER_KBIT, an unsigned long constant. On 32-bit architectures, the multiplications therefore take place in 32-bit unsigned arithmetic before the results are assigned to s64 fields. For port rates above approximately 34.36 Gbit/s this wraps port_rate. The same conversion turns a negative sendslope into a large positive value, reversing the CBS credit adjustment. This affects software CBS; port_rate is also refreshed on NETDEV_UP and NETDEV_CHANGE notifications. Cast the first operand of each multiplication to s64 so all intermediate operations use signed 64-bit arithmetic and preserve the value's sign on every architecture.
Check what the sashikos are saying please - i agree with their view. I believe we need a v2: I'd suggest a v2 that addresses both (and optionally validates the idleslope sign and a negative idleslope can arm the watchdog in the past and busy-loop). For completeness, the pre-existing timediff_to_credits() s64 overflow that gemini sashiko flagged is a separate follow-up, not part of this patch. cheers, jamal
quoted hunk ↗ jump to hunk
Fixes: 585d763af09c ("net/sched: Introduce Credit Based Shaper (CBS) qdisc") Fixes: 397006ba5d918 ("net/sched: cbs: Fix integer overflow in cbs_set_port_rate()") Signed-off-by: Jack Wang <redacted> --- net/sched/sch_cbs.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)diff --git a/net/sched/sch_cbs.c b/net/sched/sch_cbs.c index 1c93469c56e3..9ef31ecdaa05 100644 --- a/net/sched/sch_cbs.c +++ b/net/sched/sch_cbs.c@@ -335,7 +335,7 @@ static void cbs_set_port_rate(struct net_device *dev, struct cbs_sched_data *q) speed = ecmd.base.speed; skip: - port_rate = speed * 1000 * BYTES_PER_KBIT; + port_rate = (s64)speed * 1000 * BYTES_PER_KBIT; atomic64_set(&q->port_rate, port_rate); netdev_dbg(dev, "cbs: set %s's port_rate to: %lld, linkspeed: %d\n",@@ -405,8 +405,8 @@ static int cbs_change(struct Qdisc *sch, struct nlattr *opt, /* Everything went OK, save the parameters used. */ WRITE_ONCE(q->hicredit, qopt->hicredit); WRITE_ONCE(q->locredit, qopt->locredit); - WRITE_ONCE(q->idleslope, qopt->idleslope * BYTES_PER_KBIT); - WRITE_ONCE(q->sendslope, qopt->sendslope * BYTES_PER_KBIT); + WRITE_ONCE(q->idleslope, (s64)qopt->idleslope * BYTES_PER_KBIT); + WRITE_ONCE(q->sendslope, (s64)qopt->sendslope * BYTES_PER_KBIT); WRITE_ONCE(q->offload, qopt->offload); return 0; --2.53.0