[PATCH net 1/2] net/sched: hhf: cap hh_flows_limit at change time

[PATCH net 1/2] net/sched: hhf: cap hh_flows_limit at change time

[Jamal Hadi Salim]

hhf_change() stores TCA_HHF_HH_FLOWS_LIMIT with no upper bound. A huge
hh_flows_limit lets each new heavy-hitter flow pass the
hh_flows_current_cnt check in alloc_new_hh() and forces a fixed-size
kzalloc(GFP_ATOMIC) per flow under spoofed traffic, for unbounded memory
growth.

Bound the attribute with NLA_POLICY_MAX() at 2*HH_FLOWS_CNT (the
hhf_init() default) and report the rejected value via extack. The
deprecated nested parse is kept: legacy tc does not set NLA_F_NESTED on
TCA_OPTIONS. Configs relying on hh_limit above the default were relying
on unbounded, unsafe behaviour and are not supported going forward.

hhf_init() also ran hhf_change() before setting the default
hh_flows_limit, so a user-supplied hh_limit at add time was clobbered
back to 2048. Set the default before hhf_change() so the configured
value sticks.

This is a follow-up to commit eb56a495f59b ("net/sched: hhf: clamp
quantum in change and init paths"), which bounded the quantum of the
same qdisc; the hh_flows_limit bound is the remaining unbounded knob of
that series' scope.

Conditions to recreate the bug: CAP_NET_ADMIN in a user namespace;
tc qdisc change dev X root hhf hh_limit 4294967295 succeeds and the
value is echoed by tc qdisc show, unbounding heavy-hitter flow
allocations; also tc qdisc add dev X root hhf hh_limit 500 stores 2048
instead of 500.

Fixes: 10239edf86f1 ("net-qdisc-hhf: Heavy-Hitter Filter (HHF) qdisc")
Cc: stable@vger.kernel.org
Reported-by: Sashiko (gemini) <sashiko-bot@kernel.org>
Closes: https://sashiko.dev/#/patchset/20260822195509.112717-1-jhs@mojatatu.com
Reviewed-by: Victor Nogueira <victor@mojatatu.com>
Tested-by: hybris <hybris@mojatatu.ai>
Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
---
 net/sched/sch_hhf.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/net/sched/sch_hhf.c b/net/sched/sch_hhf.c
index fc72f825fbd9..5dec1ed969ad 100644
--- a/net/sched/sch_hhf.c
+++ b/net/sched/sch_hhf.c
@@ -527,7 +527,7 @@ static void hhf_destroy(struct Qdisc *sch)
 static const struct nla_policy hhf_policy[TCA_HHF_MAX + 1] = {
 	[TCA_HHF_BACKLOG_LIMIT]	 = { .type = NLA_U32 },
 	[TCA_HHF_QUANTUM]	 = { .type = NLA_U32 },
-	[TCA_HHF_HH_FLOWS_LIMIT] = { .type = NLA_U32 },
+	[TCA_HHF_HH_FLOWS_LIMIT] = NLA_POLICY_MAX(NLA_U32, 2 * HH_FLOWS_CNT),
 	[TCA_HHF_RESET_TIMEOUT]	 = { .type = NLA_U32 },
 	[TCA_HHF_ADMIT_BYTES]	 = { .type = NLA_U32 },
 	[TCA_HHF_EVICT_TIMEOUT]	 = { .type = NLA_U32 },
@@ -546,7 +546,7 @@ static int hhf_change(struct Qdisc *sch, struct nlattr *opt,
 	u32 new_hhf_non_hh_weight = q->hhf_non_hh_weight;
 
 	err = nla_parse_nested_deprecated(tb, TCA_HHF_MAX, opt, hhf_policy,
-					  NULL);
+					  extack);
 	if (err < 0)
 		return err;
 
@@ -624,6 +624,9 @@ static int hhf_init(struct Qdisc *sch, struct nlattr *opt,
 	q->hhf_evict_timeout = HZ;      /* 1  sec */
 	q->hhf_non_hh_weight = 2;
 
+	/* Cap max active HHs at twice len of hh_flows table. */
+	q->hh_flows_limit = 2 * HH_FLOWS_CNT;
+
 	if (opt) {
 		int err = hhf_change(sch, opt, extack);
 
@@ -639,8 +642,6 @@ static int hhf_init(struct Qdisc *sch, struct nlattr *opt,
 		for (i = 0; i < HH_FLOWS_CNT; i++)
 			INIT_LIST_HEAD(&q->hh_flows[i]);
 
-		/* Cap max active HHs at twice len of hh_flows table. */
-		q->hh_flows_limit = 2 * HH_FLOWS_CNT;
 		q->hh_flows_overlimit = 0;
 		q->hh_flows_total_cnt = 0;
 		q->hh_flows_current_cnt = 0;
-- 
2.43.0

[PATCH net 2/2] selftests/tc-testing: add hhf hh_limit cap tests

[Jamal Hadi Salim]

Cover the new TCA_HHF_HH_FLOWS_LIMIT bound: values above 2*HH_FLOWS_CNT
(4294967295, 65536, 2049) are rejected with the configured limit left
untouched on both the change and the add path, the boundary value 2048 is
accepted (installed at 100 first so the boundary change is load-bearing),
and an add-time hh_limit 500 is preserved instead of being clobbered by
the default.

Reviewed-by: Victor Nogueira <victor@mojatatu.com>
Tested-by: hybris <hybris@mojatatu.ai>
Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
---
 .../tc-tests/qdiscs/hhf_flows_limit.json      | 128 ++++++++++++++++++
 1 file changed, 128 insertions(+)
 create mode 100644 tools/testing/selftests/tc-testing/tc-tests/qdiscs/hhf_flows_limit.json

diff --git a/tools/testing/selftests/tc-testing/tc-tests/qdiscs/hhf_flows_limit.json b/tools/testing/selftests/tc-testing/tc-tests/qdiscs/hhf_flows_limit.json
new file mode 100644
index 000000000000..44538b9266b6
--- /dev/null
+++ b/tools/testing/selftests/tc-testing/tc-tests/qdiscs/hhf_flows_limit.json
@@ -0,0 +1,128 @@
+[
+    {
+        "id": "e3cc",
+        "name": "HHF hh_limit rejects value above 2*HH_FLOWS_CNT cap (4294967295)",
+        "category": [
+            "qdisc",
+            "hhf"
+        ],
+        "plugins": {
+            "requires": "nsPlugin"
+        },
+        "setup": [
+            "$TC qdisc add dev $DUMMY handle 1: root hhf"
+        ],
+        "cmdUnderTest": "$TC qdisc change dev $DUMMY handle 1: root hhf hh_limit 4294967295",
+        "expExitCode": "2",
+        "verifyCmd": "$TC qdisc show dev $DUMMY",
+        "matchPattern": "qdisc hhf 1: root refcnt [0-9]+.*hh_limit 2048",
+        "matchCount": "1",
+        "teardown": [
+            "$TC qdisc del dev $DUMMY handle 1: root"
+        ]
+    },
+    {
+        "id": "f681",
+        "name": "HHF hh_limit rejects 65536 (above 2*HH_FLOWS_CNT cap)",
+        "category": [
+            "qdisc",
+            "hhf"
+        ],
+        "plugins": {
+            "requires": "nsPlugin"
+        },
+        "setup": [
+            "$TC qdisc add dev $DUMMY handle 1: root hhf"
+        ],
+        "cmdUnderTest": "$TC qdisc change dev $DUMMY handle 1: root hhf hh_limit 65536",
+        "expExitCode": "2",
+        "verifyCmd": "$TC qdisc show dev $DUMMY",
+        "matchPattern": "qdisc hhf 1: root refcnt [0-9]+.*hh_limit 2048",
+        "matchCount": "1",
+        "teardown": [
+            "$TC qdisc del dev $DUMMY handle 1: root"
+        ]
+    },
+    {
+        "id": "223d",
+        "name": "HHF hh_limit accepts boundary value 2048 (2*HH_FLOWS_CNT)",
+        "category": [
+            "qdisc",
+            "hhf"
+        ],
+        "plugins": {
+            "requires": "nsPlugin"
+        },
+        "setup": [
+            "$TC qdisc add dev $DUMMY handle 1: root hhf hh_limit 100"
+        ],
+        "cmdUnderTest": "$TC qdisc change dev $DUMMY handle 1: root hhf hh_limit 2048",
+        "expExitCode": "0",
+        "verifyCmd": "$TC qdisc show dev $DUMMY",
+        "matchPattern": "qdisc hhf 1: root refcnt [0-9]+.*hh_limit 2048",
+        "matchCount": "1",
+        "teardown": [
+            "$TC qdisc del dev $DUMMY handle 1: root"
+        ]
+    },
+    {
+        "id": "147f",
+        "name": "HHF hh_limit rejects first value above cap (2049)",
+        "category": [
+            "qdisc",
+            "hhf"
+        ],
+        "plugins": {
+            "requires": "nsPlugin"
+        },
+        "setup": [
+            "$TC qdisc add dev $DUMMY handle 1: root hhf"
+        ],
+        "cmdUnderTest": "$TC qdisc change dev $DUMMY handle 1: root hhf hh_limit 2049",
+        "expExitCode": "2",
+        "verifyCmd": "$TC qdisc show dev $DUMMY",
+        "matchPattern": "qdisc hhf 1: root refcnt [0-9]+.*hh_limit 2048",
+        "matchCount": "1",
+        "teardown": [
+            "$TC qdisc del dev $DUMMY handle 1: root"
+        ]
+    },
+    {
+        "id": "4d4f",
+        "name": "HHF add-time hh_limit 500 is preserved (init does not clobber user value)",
+        "category": [
+            "qdisc",
+            "hhf"
+        ],
+        "plugins": {
+            "requires": "nsPlugin"
+        },
+        "setup": [],
+        "cmdUnderTest": "$TC qdisc add dev $DUMMY handle 1: root hhf hh_limit 500",
+        "expExitCode": "0",
+        "verifyCmd": "$TC qdisc show dev $DUMMY",
+        "matchPattern": "qdisc hhf 1: root refcnt [0-9]+.*hh_limit 500",
+        "matchCount": "1",
+        "teardown": [
+            "$TC qdisc del dev $DUMMY handle 1: root"
+        ]
+    },
+    {
+        "id": "ca99",
+        "name": "HHF add-time hh_limit 4294967295 is rejected (no qdisc installed)",
+        "category": [
+            "qdisc",
+            "hhf"
+        ],
+        "plugins": {
+            "requires": "nsPlugin"
+        },
+        "setup": [],
+        "cmdUnderTest": "$TC qdisc add dev $DUMMY handle 1: root hhf hh_limit 4294967295",
+        "expExitCode": "2",
+        "verifyCmd": "$TC qdisc show dev $DUMMY",
+        "matchPattern": "qdisc hhf 1: root",
+        "matchCount": "0",
+        "teardown": []
+    }
+]
-- 
2.43.0

Re: [PATCH net 1/2] net/sched: hhf: cap hh_flows_limit at change time

[netdev-bot+sashiko]

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

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

- [Medium] net/sched/sch_hhf.c: the new `[TCA_HHF_HH_FLOWS_LIMIT] =
  NLA_POLICY_MAX(NLA_U32, 2 * HH_FLOWS_CNT)` policy entry (line 530) is…

--

net/sched: hhf: cap hh_flows_limit at change time

The patch bounds TCA_HHF_HH_FLOWS_LIMIT with NLA_POLICY_MAX() at
2*HH_FLOWS_CNT, passes extack down to the nested parse so the rejected
value is reported, and moves the hh_flows_limit default in hhf_init()
ahead of the hhf_change() call so a user-supplied value at add time is no
longer overwritten.

Closes: https://sashiko.dev/#/patchset/20260822195509.112717-1-jhs@mojatatu.com

> diff --git a/net/sched/sch_hhf.c b/net/sched/sch_hhf.c
> index fc72f825fbd92..5dec1ed969adf 100644
> --- a/net/sched/sch_hhf.c
> +++ b/net/sched/sch_hhf.c
> @@ -527,7 +527,7 @@ static void hhf_destroy(struct Qdisc *sch)
>  static const struct nla_policy hhf_policy[TCA_HHF_MAX + 1] = {
>  	[TCA_HHF_BACKLOG_LIMIT]	 = { .type = NLA_U32 },
>  	[TCA_HHF_QUANTUM]	 = { .type = NLA_U32 },
> -	[TCA_HHF_HH_FLOWS_LIMIT] = { .type = NLA_U32 },
> +	[TCA_HHF_HH_FLOWS_LIMIT] = NLA_POLICY_MAX(NLA_U32, 2 * HH_FLOWS_CNT),

[Severity: Medium]
Does this policy entry also change the behaviour of the qdisc creation
path?  The subject says "cap hh_flows_limit at change time", but hhf_init()
calls hhf_change(), so the same policy is applied on add.

validate_nla() dispatches the max check from the pt->validation_type
switch, independently of the validation level:

lib/nlattr.c:validate_nla() {
	...
	case NLA_VALIDATE_MAX:
		err = nla_validate_int_range(pt, nla, extack, validate);
		if (err)
			return err;
	...
}

so even the NL_VALIDATE_LIBERAL level used by
nla_parse_nested_deprecated() returns -ERANGE, and hhf_init() propagates
it:

	if (opt) {
		int err = hhf_change(sch, opt, extack);

		if (err)
			return err;
	}

At baseline, tc qdisc add dev X root hhf hh_limit 4294967295 returned
success, because hhf_init() unconditionally reassigned the safe default
after hhf_change():

	if (!q->hh_flows) {
		...
		/* Cap max active HHs at twice len of hh_flows table. */
		q->hh_flows_limit = 2 * HH_FLOWS_CNT;

With this change the same command fails with -ERANGE and no qdisc is
installed.  Is that intended for a Cc: stable patch, and should it be
mentioned in the changelog?

The commit message says:

    Configs relying on hh_limit above the default were relying
    on unbounded, unsafe behaviour and are not supported going forward.

Is that accurate for the add path?  A setup doing tc qdisc add ... hh_limit
<value above 2048> was running with hh_flows_limit clobbered to 2048, so it
was not exposed to the unbounded allocation behaviour described earlier in
the changelog; only tc qdisc change could reach that state.

The sibling knob in the same function is clamped rather than rejected, from
the referenced eb56a495f59b:

	if (tb[TCA_HHF_QUANTUM])
		new_quantum = max(256U, nla_get_u32(tb[TCA_HHF_QUANTUM]));

Would a min() on hh_flows_limit in hhf_change() give the same memory bound
without turning previously-successful add commands into failures?  If
rejection is preferred, could the changelog state that the add path now
returns -ERANGE for values above 2*HH_FLOWS_CNT?

> @@ -546,7 +546,7 @@ static int hhf_change(struct Qdisc *sch, struct nlattr *opt,
>  	u32 new_hhf_non_hh_weight = q->hhf_non_hh_weight;
>  
>  	err = nla_parse_nested_deprecated(tb, TCA_HHF_MAX, opt, hhf_policy,
> -					  NULL);
> +					  extack);
>  	if (err < 0)
>  		return err;
>  
> @@ -624,6 +624,9 @@ static int hhf_init(struct Qdisc *sch, struct nlattr *opt,
>  	q->hhf_evict_timeout = HZ;      /* 1  sec */
>  	q->hhf_non_hh_weight = 2;
>  
> +	/* Cap max active HHs at twice len of hh_flows table. */
> +	q->hh_flows_limit = 2 * HH_FLOWS_CNT;
> +
>  	if (opt) {
>  		int err = hhf_change(sch, opt, extack);
>  
> @@ -639,8 +642,6 @@ static int hhf_init(struct Qdisc *sch, struct nlattr *opt,
>  		for (i = 0; i < HH_FLOWS_CNT; i++)
>  			INIT_LIST_HEAD(&q->hh_flows[i]);
>  
> -		/* Cap max active HHs at twice len of hh_flows table. */
> -		q->hh_flows_limit = 2 * HH_FLOWS_CNT;
>  		q->hh_flows_overlimit = 0;
>  		q->hh_flows_total_cnt = 0;
>  		q->hh_flows_current_cnt = 0;

Moving the default ahead of hhf_change() is what makes the add-time value
effective for the first time, so this hunk and the policy hunk together
produce the new add-path failure described above.

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/QDISC-B855.v1.20260911153152%40mojatatu.com

Re: [PATCH net 2/2] selftests/tc-testing: add hhf hh_limit cap tests

[netdev-bot+sashiko]

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

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

- [Medium] The three change-path rejection tests (e3cc, f681, 147f)
  assert a witness value that is identical to the kernel default, so…
- [Low] Commit message overstates the add-path coverage of the new test
  file.

--

selftests/tc-testing: add hhf hh_limit cap tests

Adds tools/testing/selftests/tc-testing/tc-tests/qdiscs/hhf_flows_limit.json
with six cases exercising the TCA_HHF_HH_FLOWS_LIMIT policy bound on the hhf
qdisc, covering rejected out-of-range values, the accepted boundary value
2048, and an add-time value of 500.

A couple of questions about the coverage claims and the assertions.

> Cover the new TCA_HHF_HH_FLOWS_LIMIT bound: values above 2*HH_FLOWS_CNT
> (4294967295, 65536, 2049) are rejected with the configured limit left
> untouched on both the change and the add path, the boundary value 2048 is
> accepted (installed at 100 first so the boundary change is load-bearing),
> and an add-time hh_limit 500 is preserved instead of being clobbered by
> the default.

[Severity: Low]
Does the file really cover all three values "on both the change and the add
path"?

In hhf_flows_limit.json the values 4294967295, 65536 and 2049 appear only in
cmdUnderTest lines that use "qdisc change" (tests e3cc, f681 and 147f).

The only invalid add-path case is ca99, and it uses 4294967295 only:

    "cmdUnderTest": "$TC qdisc add dev $DUMMY handle 1: root hhf hh_limit 4294967295",
    "expExitCode": "2",
    "verifyCmd": "$TC qdisc show dev $DUMMY",
    "matchPattern": "qdisc hhf 1: root",
    "matchCount": "0",

Also, ca99 asserts that no qdisc exists at all, so there is no "configured
limit left untouched" property being checked on the add path.

Could the changelog describe the actual matrix, or could add-path cases for
65536 and 2049 be added?

> diff --git a/tools/testing/selftests/tc-testing/tc-tests/qdiscs/hhf_flows_limit.json b/tools/testing/selftests/tc-testing/tc-tests/qdiscs/hhf_flows_limit.json
> new file mode 100644
> index 0000000000000..44538b9266b65
> --- /dev/null
> +++ b/tools/testing/selftests/tc-testing/tc-tests/qdiscs/hhf_flows_limit.json
> @@ -0,0 +1,128 @@
> +[
> +    {
> +        "id": "e3cc",
> +        "name": "HHF hh_limit rejects value above 2*HH_FLOWS_CNT cap (4294967295)",

[ ... ]

> +        "setup": [
> +            "$TC qdisc add dev $DUMMY handle 1: root hhf"
> +        ],
> +        "cmdUnderTest": "$TC qdisc change dev $DUMMY handle 1: root hhf hh_limit 4294967295",
> +        "expExitCode": "2",
> +        "verifyCmd": "$TC qdisc show dev $DUMMY",
> +        "matchPattern": "qdisc hhf 1: root refcnt [0-9]+.*hh_limit 2048",
> +        "matchCount": "1",

[Severity: Medium]
Can this assertion distinguish "limit left untouched" from "limit reset or
clamped to the cap"?

The setup adds hhf without hh_limit, so hhf_init() leaves the default in
place:

net/sched/sch_hhf.c:hhf_init() {
	...
	/* Cap max active HHs at twice len of hh_flows table. */
	q->hh_flows_limit = 2 * HH_FLOWS_CNT;
	...
}

With HH_FLOWS_CNT 1024 that default is 2048, which is also the policy bound
used for the attribute:

	[TCA_HHF_HH_FLOWS_LIMIT] = NLA_POLICY_MAX(NLA_U32, 2 * HH_FLOWS_CNT),

So the matched witness "hh_limit 2048" is the same string that hhf_dump()
would print if a future implementation validated late and wrote or clamped
q->hh_flows_limit to 2048 before returning the error. Only expExitCode 2 is
actually pinned down here.

The same applies to f681 (hh_limit 65536) and 147f (hh_limit 2049) below,
which use the identical defaulted setup and the identical "hh_limit 2048"
matchPattern.

Would installing a distinct in-range value in setup, e.g.
"$TC qdisc add dev $DUMMY handle 1: root hhf hh_limit 100" and then matching
"hh_limit 100" after the rejected change, make these three assertions
load-bearing? Test 223d further down already uses exactly that technique:

> +        "setup": [
> +            "$TC qdisc add dev $DUMMY handle 1: root hhf hh_limit 100"
> +        ],
> +        "cmdUnderTest": "$TC qdisc change dev $DUMMY handle 1: root hhf hh_limit 2048",

[ ... ]

> +    {
> +        "id": "f681",
> +        "name": "HHF hh_limit rejects 65536 (above 2*HH_FLOWS_CNT cap)",

[ ... ]

> +        "setup": [
> +            "$TC qdisc add dev $DUMMY handle 1: root hhf"
> +        ],
> +        "cmdUnderTest": "$TC qdisc change dev $DUMMY handle 1: root hhf hh_limit 65536",
> +        "expExitCode": "2",
> +        "verifyCmd": "$TC qdisc show dev $DUMMY",
> +        "matchPattern": "qdisc hhf 1: root refcnt [0-9]+.*hh_limit 2048",
> +        "matchCount": "1",

[ ... ]

> +    {
> +        "id": "147f",
> +        "name": "HHF hh_limit rejects first value above cap (2049)",

[ ... ]

> +        "setup": [
> +            "$TC qdisc add dev $DUMMY handle 1: root hhf"
> +        ],
> +        "cmdUnderTest": "$TC qdisc change dev $DUMMY handle 1: root hhf hh_limit 2049",
> +        "expExitCode": "2",
> +        "verifyCmd": "$TC qdisc show dev $DUMMY",
> +        "matchPattern": "qdisc hhf 1: root refcnt [0-9]+.*hh_limit 2048",
> +        "matchCount": "1",

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/QDISC-B855.v1.20260911153152%40mojatatu.com

Re: [PATCH net 1/2] net/sched: hhf: cap hh_flows_limit at change time

[Jamal Hadi Salim]

On Sat, Sep 12, 2026 at 4:36 PM <netdev-bot+sashiko@kernel.org> wrote:
>
> Thank you for your contribution! Sashiko AI review found 1 potential
> issue(s) to consider:
>
> Critical: 0 · High: 0 · Medium: 1 · Low: 0
>
> - [Medium] net/sched/sch_hhf.c: the new `[TCA_HHF_HH_FLOWS_LIMIT] =
>   NLA_POLICY_MAX(NLA_U32, 2 * HH_FLOWS_CNT)` policy entry (line 530) is…
>

These findings by sashiko-nipa are accurate but as in the previous
codel thread they concern changelog and selftest-robustness nits.
IOW, there is _no code defect_. The -ERANGE behavior change on add
path and the changelog overstatement are worth a one-line fix if/when
the thread is re-rev'd, but IMO are not a revision blocker


cheers,
jamal

> --
>
> net/sched: hhf: cap hh_flows_limit at change time
>
> The patch bounds TCA_HHF_HH_FLOWS_LIMIT with NLA_POLICY_MAX() at
> 2*HH_FLOWS_CNT, passes extack down to the nested parse so the rejected
> value is reported, and moves the hh_flows_limit default in hhf_init()
> ahead of the hhf_change() call so a user-supplied value at add time is no
> longer overwritten.
>
> Closes: https://sashiko.dev/#/patchset/20260822195509.112717-1-jhs@mojatatu.com
>
> > diff --git a/net/sched/sch_hhf.c b/net/sched/sch_hhf.c
> > index fc72f825fbd92..5dec1ed969adf 100644
> > --- a/net/sched/sch_hhf.c
> > +++ b/net/sched/sch_hhf.c
> > @@ -527,7 +527,7 @@ static void hhf_destroy(struct Qdisc *sch)
> >  static const struct nla_policy hhf_policy[TCA_HHF_MAX + 1] = {
> >       [TCA_HHF_BACKLOG_LIMIT]  = { .type = NLA_U32 },
> >       [TCA_HHF_QUANTUM]        = { .type = NLA_U32 },
> > -     [TCA_HHF_HH_FLOWS_LIMIT] = { .type = NLA_U32 },
> > +     [TCA_HHF_HH_FLOWS_LIMIT] = NLA_POLICY_MAX(NLA_U32, 2 * HH_FLOWS_CNT),
>
> [Severity: Medium]
> Does this policy entry also change the behaviour of the qdisc creation
> path?  The subject says "cap hh_flows_limit at change time", but hhf_init()
> calls hhf_change(), so the same policy is applied on add.
>
> validate_nla() dispatches the max check from the pt->validation_type
> switch, independently of the validation level:
>
> lib/nlattr.c:validate_nla() {
>         ...
>         case NLA_VALIDATE_MAX:
>                 err = nla_validate_int_range(pt, nla, extack, validate);
>                 if (err)
>                         return err;
>         ...
> }
>
> so even the NL_VALIDATE_LIBERAL level used by
> nla_parse_nested_deprecated() returns -ERANGE, and hhf_init() propagates
> it:
>
>         if (opt) {
>                 int err = hhf_change(sch, opt, extack);
>
>                 if (err)
>                         return err;
>         }
>
> At baseline, tc qdisc add dev X root hhf hh_limit 4294967295 returned
> success, because hhf_init() unconditionally reassigned the safe default
> after hhf_change():
>
>         if (!q->hh_flows) {
>                 ...
>                 /* Cap max active HHs at twice len of hh_flows table. */
>                 q->hh_flows_limit = 2 * HH_FLOWS_CNT;
>
> With this change the same command fails with -ERANGE and no qdisc is
> installed.  Is that intended for a Cc: stable patch, and should it be
> mentioned in the changelog?
>
> The commit message says:
>
>     Configs relying on hh_limit above the default were relying
>     on unbounded, unsafe behaviour and are not supported going forward.
>
> Is that accurate for the add path?  A setup doing tc qdisc add ... hh_limit
> <value above 2048> was running with hh_flows_limit clobbered to 2048, so it
> was not exposed to the unbounded allocation behaviour described earlier in
> the changelog; only tc qdisc change could reach that state.
>
> The sibling knob in the same function is clamped rather than rejected, from
> the referenced eb56a495f59b:
>
>         if (tb[TCA_HHF_QUANTUM])
>                 new_quantum = max(256U, nla_get_u32(tb[TCA_HHF_QUANTUM]));
>
> Would a min() on hh_flows_limit in hhf_change() give the same memory bound
> without turning previously-successful add commands into failures?  If
> rejection is preferred, could the changelog state that the add path now
> returns -ERANGE for values above 2*HH_FLOWS_CNT?
>
> > @@ -546,7 +546,7 @@ static int hhf_change(struct Qdisc *sch, struct nlattr *opt,
> >       u32 new_hhf_non_hh_weight = q->hhf_non_hh_weight;
> >
> >       err = nla_parse_nested_deprecated(tb, TCA_HHF_MAX, opt, hhf_policy,
> > -                                       NULL);
> > +                                       extack);
> >       if (err < 0)
> >               return err;
> >
> > @@ -624,6 +624,9 @@ static int hhf_init(struct Qdisc *sch, struct nlattr *opt,
> >       q->hhf_evict_timeout = HZ;      /* 1  sec */
> >       q->hhf_non_hh_weight = 2;
> >
> > +     /* Cap max active HHs at twice len of hh_flows table. */
> > +     q->hh_flows_limit = 2 * HH_FLOWS_CNT;
> > +
> >       if (opt) {
> >               int err = hhf_change(sch, opt, extack);
> >
> > @@ -639,8 +642,6 @@ static int hhf_init(struct Qdisc *sch, struct nlattr *opt,
> >               for (i = 0; i < HH_FLOWS_CNT; i++)
> >                       INIT_LIST_HEAD(&q->hh_flows[i]);
> >
> > -             /* Cap max active HHs at twice len of hh_flows table. */
> > -             q->hh_flows_limit = 2 * HH_FLOWS_CNT;
> >               q->hh_flows_overlimit = 0;
> >               q->hh_flows_total_cnt = 0;
> >               q->hh_flows_current_cnt = 0;
>
> Moving the default ahead of hhf_change() is what makes the add-time value
> effective for the first time, so this hunk and the policy hunk together
> produce the new add-path failure described above.
>
> --
> Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/QDISC-B855.v1.20260911153152%40mojatatu.com