[PATCH net-next v2] net/sched: cls_flow: avoid integer overflow in perturb_period

[PATCH net-next v2] net/sched: cls_flow: avoid integer overflow in perturb_period

[Wei Tao]

flow_change() calculates perturb_period by directly multiplying
nla_get_u32(tb[TCA_FLOW_PERTURB]) by HZ, without any overflow
protection. Since perturb_period is a u32, the multiplication
overflows when the value from netlink exceeds UINT_MAX / HZ.
The result can be an unexpectedly short period or even zero.

  # tc qdisc add dev lo clsact
  # tc filter add dev lo ingress protocol ip pref 1 handle 1 flow \
        hash keys src perturb 42949673 baseclass 1:10

With HZ=100, 42949673*100=4294967300, but UINT_MAX is 4294967295,
so the product wraps to 4 jiffies.

Both the filter-modify and filter-create paths in flow_change()
are affected.

Since perturb_period is ultimately fed to mod_timer(), and
calc_wheel_index() internally casts delta to signed long for
comparison, reject values greater than INT_MAX / HZ. Besides
preventing the u32 multiplication from wrapping, this also
keeps the relative timeout within the signed-long range on
32-bit systems.

The missing check was noticed while reviewing commit 7ca52541c05c
("net_sched: sch_sfq: reject invalid perturb period"). An
LLM-assisted analysis of the same pattern then showed that cls_flow
performs the same unchecked seconds-to-jiffies conversion in both
its create and change paths, and needs the same range check.

Tested on aarch64 with CONFIG_HZ=100:

  # tc qdisc add dev lo clsact
  # tc filter add dev lo ingress protocol ip pref 1 handle 1 flow \
        hash keys src perturb 42949673 baseclass 1:10
  Error: cls_flow: invalid perturb period.

  # tc filter add dev lo ingress protocol ip pref 1 handle 1 flow \
        hash keys src perturb 100 baseclass 1:10
  # tc filter show dev lo ingress | cut -d' ' -f9-
  handle 0x1 hash keys src baseclass 1:10 perturb 100sec

  # tc filter change dev lo ingress protocol ip pref 1 handle 1 flow \
        hash keys src perturb 42949673 baseclass 1:10
  Error: cls_flow: invalid perturb period.
  # tc filter show dev lo ingress | cut -d' ' -f9-
  handle 0x1 hash keys src baseclass 1:10 perturb 100sec

Signed-off-by: Wei Tao <taowei@kylinos.cn>
---
v2: https://lore.kernel.org/netdev/20261016040330.59041-1-taowei@kylinos.cn/
  - drop the Fixes and Cc: stable tags: this is hardening
  - describe how the issue was found and add the tested commands
 net/sched/cls_flow.c | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/net/sched/cls_flow.c b/net/sched/cls_flow.c
index 2ef449706638..4f2d63722e16 100644
--- a/net/sched/cls_flow.c
+++ b/net/sched/cls_flow.c
@@ -408,6 +408,7 @@ static int flow_change(struct net *net, struct sk_buff *in_skb,
 	u32 keymask = 0;
 	u32 mode;
 	int err;
+	u32 val;
 
 	if (opt == NULL)
 		return -EINVAL;
@@ -486,7 +487,13 @@ static int flow_change(struct net *net, struct sk_buff *in_skb,
 		if (tb[TCA_FLOW_PERTURB]) {
 			if (mode != FLOW_MODE_HASH)
 				goto err2;
-			perturb_period = nla_get_u32(tb[TCA_FLOW_PERTURB]) * HZ;
+			val = nla_get_u32(tb[TCA_FLOW_PERTURB]);
+			if (val > INT_MAX / HZ) {
+				NL_SET_ERR_MSG_MOD(extack, "invalid perturb period");
+				err = -EINVAL;
+				goto err2;
+			}
+			perturb_period = val * HZ;
 		}
 	} else {
 		err = -EINVAL;
@@ -504,7 +511,13 @@ static int flow_change(struct net *net, struct sk_buff *in_skb,
 		if (tb[TCA_FLOW_PERTURB]) {
 			if (mode != FLOW_MODE_HASH)
 				goto err2;
-			perturb_period = nla_get_u32(tb[TCA_FLOW_PERTURB]) * HZ;
+			val = nla_get_u32(tb[TCA_FLOW_PERTURB]);
+			if (val > INT_MAX / HZ) {
+				NL_SET_ERR_MSG_MOD(extack, "invalid perturb period");
+				err = -EINVAL;
+				goto err2;
+			}
+			perturb_period = val * HZ;
 		}
 
 		if (TC_H_MAJ(baseclass) == 0) {

base-commit: 6628e7d4c6b25c8d18f1345e3a20d21829aae7e4
-- 
2.25.1

Re: [PATCH net-next v2] net/sched: cls_flow: avoid integer overflow in perturb_period

[Paolo Abeni]

On 11/6/26 07:38, Wei Tao wrote:
> @@ -486,7 +487,13 @@ static int flow_change(struct net *net, struct sk_buff *in_skb,
>   		if (tb[TCA_FLOW_PERTURB]) {
>   			if (mode != FLOW_MODE_HASH)
>   				goto err2;
> -			perturb_period = nla_get_u32(tb[TCA_FLOW_PERTURB]) * HZ;
> +			val = nla_get_u32(tb[TCA_FLOW_PERTURB]);
> +			if (val > INT_MAX / HZ) {
> +				NL_SET_ERR_MSG_MOD(extack, "invalid perturb period");
> +				err = -EINVAL;
> +				goto err2;

Sashiko noted the added code could by simplified/substituted with
a  NLA_POLICY_FULL_RANGE(NLA_U32, &range):

https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261106063823.99731-1-taowei@kylinos.cn

/P