[PATCH net 1/2] net/sched: cls_route: reject change with no routing attribute

[PATCH net 1/2] net/sched: cls_route: reject change with no routing attribute

[Victor Nogueira]

route4_change on a filter change that supplies no routing attribute
at all (no to/from/iif) reaches route4_set_parms with fold set, so the
handle-mismatch check -- which only fires when creating -- is skipped.
nhandle is built as the all-wildcard bucket (0x8000 | 0xFFFF << 16) and
the filter is rekeyed to 0xFFFF8000, so the handle userspace already
stored no longer addresses the filter. (The netlink reply carries the
new handle; what goes stale is the handle the caller holds.)

The rekey actually misclassifies the packets. The new handle's from_hash
is the wildcard chain (route4_hash_wild), whose filters route4_classify
applies without any id/iif comparison, and its bucket (256) is reached
by every packet through the h < 256 restart. A filter that matched only
a specific realm therefore becomes an unconditional catch-all that applies
its classid and actions to all otherwise-unmatched traffic.

Reject a change that would rekey the filter this way, with the
routing-attributes-required error. A change that names at least one of
to/from/iif, and an in-place replace that keeps the routing identity via
a supplied attribute, are unaffected. A change of a filter that already
sits at the wildcard handle is a metadata-only update and remains valid,
so only a change that actually moves the handle is rejected.

This is a follow-up to commit 41e85e54e564 ("net/sched: cls_route: Fix
in-place replace"); this patch closes the no-routing-attribute variant
that the parent series did not address.

Conditions to recreate the bug:
  tc qdisc add dev lo clsact
  tc filter add dev lo ingress protocol ip pref 100 route from 1 to 1
  tc filter change dev lo ingress protocol ip pref 100 handle 0x10001 \
      route classid 1:2
  tc filter show dev lo ingress   # handle became 0xffff8000; iproute2's
                                  # stored 0x10001 no longer addresses it

A change of a filter already at the wildcard handle, e.g. one created by
"tc filter add dev lo ingress protocol ip pref 100 route classid 1:1"
(iproute2 defaults the handle to 0xffff8000), stays a valid metadata-only
update.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Reported-by: Sashiko <sashiko-bot@kernel.org>
Closes: https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260907192133.2639067-1-victor@mojatatu.com
Reviewed-by: Jamal Hadi Salim <jhs@mojatatu.com>
Signed-off-by: Victor Nogueira <victor@mojatatu.com>
---
 net/sched/cls_route.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/net/sched/cls_route.c b/net/sched/cls_route.c
index 0f211f030fd9..dca812a75269 100644
--- a/net/sched/cls_route.c
+++ b/net/sched/cls_route.c
@@ -434,6 +434,12 @@ static int route4_set_parms(struct net *net, struct tcf_proto *tp,
 	if (handle && (!fold || nhandle == (handle & ~0x7F00)))
 		nhandle |= handle & 0x7F00;
 
+	if (fold && !tb[TCA_ROUTE4_TO] && !tb[TCA_ROUTE4_FROM] &&
+	    !tb[TCA_ROUTE4_IIF] && nhandle != fold->handle) {
+		NL_SET_ERR_MSG(extack, "Routing attributes required");
+		return -EINVAL;
+	}
+
 	if (handle && !fold && nhandle != handle) {
 		NL_SET_ERR_MSG_FMT(extack,
 				   "Handle mismatch constructed: %x (expected: %x)",
-- 
2.43.0

[PATCH net 2/2] selftests: tc-testing: add cls_route no-routing-attribute change tests

[Victor Nogueira]

Add a boundary pair for route4 change without a routing attribute:

- a classid-only change of a wildcard filter (handle 0xffff8000) is a
  metadata-only update and must succeed;
- the same change on a non-wildcard handle (0x10001) would rekey the
  filter to the wildcard bucket, leaving the handle userspace stored stale,
  and must be rejected with -EINVAL.

The change of a wildcard filter is the flow iproute2 uses by default
("tc filter add ... route classid X:Y" gives handle 0xffff8000).

Reviewed-by: Jamal Hadi Salim <jhs@mojatatu.com>
Signed-off-by: Victor Nogueira <victor@mojatatu.com>
---
 .../tc-testing/tc-tests/filters/route.json    | 46 +++++++++++++++++++
 1 file changed, 46 insertions(+)

diff --git a/tools/testing/selftests/tc-testing/tc-tests/filters/route.json b/tools/testing/selftests/tc-testing/tc-tests/filters/route.json
index 2d5843aebd72..c3b6ff157d3e 100644
--- a/tools/testing/selftests/tc-testing/tc-tests/filters/route.json
+++ b/tools/testing/selftests/tc-testing/tc-tests/filters/route.json
@@ -412,5 +412,51 @@
         "teardown": [
             "$TC qdisc del dev $DEV1 ingress"
         ]
+    },
+    {
+        "id": "4000",
+        "name": "Change a wildcard route filter classid only (no routing attribute)",
+        "category": [
+            "filter",
+            "route"
+        ],
+        "plugins": {
+            "requires": "nsPlugin"
+        },
+        "setup": [
+            "$TC qdisc add dev $DEV1 ingress",
+            "$TC filter add dev $DEV1 parent ffff: protocol ip prio 100 route classid 1:1"
+        ],
+        "cmdUnderTest": "$TC filter change dev $DEV1 parent ffff: protocol ip prio 100 route classid 1:9",
+        "expExitCode": "0",
+        "verifyCmd": "$TC filter ls dev $DEV1 parent ffff:",
+        "matchPattern": "fh 0xffff8000 flowid 1:9",
+        "matchCount": "1",
+        "teardown": [
+            "$TC qdisc del dev $DEV1 ingress"
+        ]
+    },
+    {
+        "id": "907a",
+        "name": "Try to change a route filter with no routing attribute and a non-wildcard handle",
+        "category": [
+            "filter",
+            "route"
+        ],
+        "plugins": {
+            "requires": "nsPlugin"
+        },
+        "setup": [
+            "$TC qdisc add dev $DEV1 ingress",
+            "$TC filter add dev $DEV1 parent ffff: protocol ip prio 100 route from 1 to 1 classid 1:1"
+        ],
+        "cmdUnderTest": "$TC filter change dev $DEV1 parent ffff: protocol ip prio 100 handle 0x10001 route classid 1:2",
+        "expExitCode": "2",
+        "verifyCmd": "$TC filter ls dev $DEV1 parent ffff:",
+        "matchPattern": "fh 0x00010001 flowid 1:1 to 1 from 1",
+        "matchCount": "1",
+        "teardown": [
+            "$TC qdisc del dev $DEV1 ingress"
+        ]
     }
 ]
-- 
2.43.0

Re: [PATCH net 2/2] selftests: tc-testing: add cls_route no-routing-attribute change tests

[Simon Horman]

On Thu, Oct 01, 2026 at 12:32:01PM -0300, Victor Nogueira wrote:
> Add a boundary pair for route4 change without a routing attribute:
> 
> - a classid-only change of a wildcard filter (handle 0xffff8000) is a
>   metadata-only update and must succeed;
> - the same change on a non-wildcard handle (0x10001) would rekey the
>   filter to the wildcard bucket, leaving the handle userspace stored stale,
>   and must be rejected with -EINVAL.
> 
> The change of a wildcard filter is the flow iproute2 uses by default
> ("tc filter add ... route classid X:Y" gives handle 0xffff8000).
> 
> Reviewed-by: Jamal Hadi Salim <jhs@mojatatu.com>
> Signed-off-by: Victor Nogueira <victor@mojatatu.com>

Reviewed-by: Simon Horman <horms@kernel.org>

Re: [PATCH net 1/2] net/sched: cls_route: reject change with no routing attribute

[Simon Horman]

On Thu, Oct 01, 2026 at 12:32:00PM -0300, Victor Nogueira wrote:
> route4_change on a filter change that supplies no routing attribute
> at all (no to/from/iif) reaches route4_set_parms with fold set, so the
> handle-mismatch check -- which only fires when creating -- is skipped.
> nhandle is built as the all-wildcard bucket (0x8000 | 0xFFFF << 16) and
> the filter is rekeyed to 0xFFFF8000, so the handle userspace already
> stored no longer addresses the filter. (The netlink reply carries the
> new handle; what goes stale is the handle the caller holds.)
> 
> The rekey actually misclassifies the packets. The new handle's from_hash
> is the wildcard chain (route4_hash_wild), whose filters route4_classify
> applies without any id/iif comparison, and its bucket (256) is reached
> by every packet through the h < 256 restart. A filter that matched only
> a specific realm therefore becomes an unconditional catch-all that applies
> its classid and actions to all otherwise-unmatched traffic.
> 
> Reject a change that would rekey the filter this way, with the
> routing-attributes-required error. A change that names at least one of
> to/from/iif, and an in-place replace that keeps the routing identity via
> a supplied attribute, are unaffected. A change of a filter that already
> sits at the wildcard handle is a metadata-only update and remains valid,
> so only a change that actually moves the handle is rejected.
> 
> This is a follow-up to commit 41e85e54e564 ("net/sched: cls_route: Fix
> in-place replace"); this patch closes the no-routing-attribute variant
> that the parent series did not address.
> 
> Conditions to recreate the bug:
>   tc qdisc add dev lo clsact
>   tc filter add dev lo ingress protocol ip pref 100 route from 1 to 1
>   tc filter change dev lo ingress protocol ip pref 100 handle 0x10001 \
>       route classid 1:2
>   tc filter show dev lo ingress   # handle became 0xffff8000; iproute2's
>                                   # stored 0x10001 no longer addresses it
> 
> A change of a filter already at the wildcard handle, e.g. one created by
> "tc filter add dev lo ingress protocol ip pref 100 route classid 1:1"
> (iproute2 defaults the handle to 0xffff8000), stays a valid metadata-only
> update.
> 
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Reported-by: Sashiko <sashiko-bot@kernel.org>
> Closes: https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260907192133.2639067-1-victor@mojatatu.com
> Reviewed-by: Jamal Hadi Salim <jhs@mojatatu.com>
> Signed-off-by: Victor Nogueira <victor@mojatatu.com>

Reviewed-by: Simon Horman <horms@kernel.org>

Re: [PATCH net 1/2] net/sched: cls_route: reject change with no routing attribute

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Thu,  1 Oct 2026 12:32:00 -0300 you wrote:
> route4_change on a filter change that supplies no routing attribute
> at all (no to/from/iif) reaches route4_set_parms with fold set, so the
> handle-mismatch check -- which only fires when creating -- is skipped.
> nhandle is built as the all-wildcard bucket (0x8000 | 0xFFFF << 16) and
> the filter is rekeyed to 0xFFFF8000, so the handle userspace already
> stored no longer addresses the filter. (The netlink reply carries the
> new handle; what goes stale is the handle the caller holds.)
> 
> [...]

Here is the summary with links:
  - [net,1/2] net/sched: cls_route: reject change with no routing attribute
    https://git.kernel.org/netdev/net/c/b72e025ac556
  - [net,2/2] selftests: tc-testing: add cls_route no-routing-attribute change tests
    https://git.kernel.org/netdev/net/c/6fa2e7db99fb

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html