[PATCH net v2] net/sched: cbs: perform rate conversions in signed 64-bit arithmetic

Jack Wang <[email protected]>
Newsgroups org.kernel.vger.netdev,org.kernel.vger.linux-kernel
Message-ID <[email protected]>
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

Also reject a non-positive idleslope. A negative idleslope can arm the
watchdog in the past and busy-loop.

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 <[email protected]>
---
v2:
  - Reject a non-positive idleslope to prevent scheduling the watchdog in
    the past.
  - Leave the pre-existing timediff_to_credits() overflow for a separate
    follow-up.

  v1: https://lore.kernel.org/netdev/[email protected]/

 net/sched/sch_cbs.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/net/sched/sch_cbs.c b/net/sched/sch_cbs.c
index 1c93469c56e3..e960c8e01ef4 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",
@@ -392,6 +392,10 @@ static int cbs_change(struct Qdisc *sch, struct nlattr *opt,
 	}
 
 	qopt = nla_data(tb[TCA_CBS_PARMS]);
+	if (qopt->idleslope <= 0) {
+		NL_SET_ERR_MSG(extack, "Idleslope must be greater than zero");
+		return -EINVAL;
+	}
 
 	if (!qopt->offload) {
 		cbs_set_port_rate(dev, q);
@@ -405,8 +409,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;

base-commit: 7b53449540502cb21b32bca62a6258e22cd97bbe
-- 
2.53.0
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.