[PATCH net 1/2] net/sched: cls_u32: fix duplicate handle when node id pool exhausted

[PATCH net 1/2] net/sched: cls_u32: fix duplicate handle when node id pool exhausted

[Jamal Hadi Salim]

gen_new_kid() tries two idr_alloc_u32() allocations for auto-generated
knode handles. If both fail (all node IDs in 0x001..0xFFF are already
reserved), it silently returns max (htid | 0xFFF) instead of an error.
u32_change() trusts that value and inserts a new knode with an
already-live handle, breaking handle uniqueness and allowing the table
to grow past the intended 4095-knode cap.

Conditions to recreate the bug:
- CONFIG_NET_SCHED=y, CONFIG_NET_CLS_U32=y, CONFIG_NET_CLS_ACT=y.
- Create a clsact/ingress qdisc on a device (e.g. lo).
- Add 4095 u32 filters with auto-generated handles to fill the
  entire node ID space (0x001..0xFFF) for the root hash table:
    yes 'filter add dev lo ingress protocol ip u32 match u8 0 0' \
      | head -n 4095 | tc -batch -
- Add a 4096th filter with an auto-generated handle. On the unfixed
  kernel this succeeds (silently reuses handle 800::fff, creating a
  duplicate). On the fixed kernel it fails with ENOSPC.
- Reachable from an unprivileged user in a fresh user+net namespace
  (unshare -Urn) with namespace-local CAP_NET_ADMIN.

Fixes: e7614370d6f04 ("net_sched: use idr to allocate u32 filter handles")
Reported-by: vega@nebusec.ai
Tested-by: Victor Nogueira <victor@mojatatu.com>
Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
---
 net/sched/cls_u32.c | 19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)

diff --git a/net/sched/cls_u32.c b/net/sched/cls_u32.c
index c297d7dbcf91..13ffad47cad4 100644
--- a/net/sched/cls_u32.c
+++ b/net/sched/cls_u32.c
@@ -695,16 +695,19 @@ static int u32_delete(struct tcf_proto *tp, void *arg, bool *last,
 	return ret;
 }
 
-static u32 gen_new_kid(struct tc_u_hnode *ht, u32 htid)
+static u32 gen_new_kid(struct tc_u_hnode *ht, u32 htid, int *err)
 {
 	u32 index = htid | 0x800;
 	u32 max = htid | 0xFFF;
 
+	*err = 0;
+
 	if (idr_alloc_u32(&ht->handle_idr, NULL, &index, max, GFP_KERNEL)) {
 		index = htid + 1;
-		if (idr_alloc_u32(&ht->handle_idr, NULL, &index, max,
-				 GFP_KERNEL))
-			index = max;
+		*err = idr_alloc_u32(&ht->handle_idr, NULL, &index, max,
+				     GFP_KERNEL);
+		if (*err)
+			return 0;
 	}
 
 	return index;
@@ -1079,7 +1082,9 @@ static int u32_change(struct net *net, struct sk_buff *in_skb,
 		 * handle which is used to uniquely identify the match entry.
 		 */
 		if (!TC_U32_NODE(handle)) {
-			handle = gen_new_kid(ht, htid);
+			handle = gen_new_kid(ht, htid, &err);
+			if (err)
+				return err;
 		} else {
 			handle = htid | TC_U32_NODE(handle);
 			err = idr_alloc_u32(&ht->handle_idr, NULL, &handle,
@@ -1091,7 +1096,9 @@ static int u32_change(struct net *net, struct sk_buff *in_skb,
 		/* The user did not give us a handle; lets just generate one
 		 * from the table's pool of nodeids.
 		 */
-		handle = gen_new_kid(ht, htid);
+		handle = gen_new_kid(ht, htid, &err);
+		if (err)
+			return err;
 	}
 
 	if (tb[TCA_U32_SEL] == NULL) {
-- 
2.43.0

[PATCH net 2/2] selftests: tc-testing: add u32 node ID pool exhaustion test

[Jamal Hadi Salim]

Add a tdc test case (7ec8) that fills the u32 node ID space with 4095
auto-generated handles, then attempts to add a 4096th. On the fixed
kernel the 4096th filter is rejected with ENOSPC (exit 2). On the
unfixed kernel it silently succeeds with a duplicate handle.

Fixes: e7614370d6f04 ("net_sched: use idr to allocate u32 filter handles")
Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
---
 .../tc-testing/tc-tests/filters/u32.json      | 23 +++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/tools/testing/selftests/tc-testing/tc-tests/filters/u32.json b/tools/testing/selftests/tc-testing/tc-tests/filters/u32.json
index b2ca9d4e991b..b9019b14d05c 100644
--- a/tools/testing/selftests/tc-testing/tc-tests/filters/u32.json
+++ b/tools/testing/selftests/tc-testing/tc-tests/filters/u32.json
@@ -353,5 +353,28 @@
         "teardown": [
             "$TC qdisc del dev $DEV1 parent root drr"
         ]
+    },
+    {
+        "id": "7ec8",
+        "name": "Add u32 filter when node ID pool is exhausted (4096th filter rejected)",
+        "category": [
+            "filter",
+            "u32"
+        ],
+        "plugins": {
+            "requires": "nsPlugin"
+        },
+        "setup": [
+            "$TC qdisc add dev $DUMMY clsact",
+            "yes 'filter add dev $DUMMY ingress prio 1 protocol ip u32 match u8 0 0 at 0' | head -n 4095 | $TC -b -"
+        ],
+        "cmdUnderTest": "$TC filter add dev $DUMMY ingress prio 1 protocol ip u32 match u8 0 0 at 0",
+        "expExitCode": "2",
+        "verifyCmd": "$TC filter show dev $DUMMY ingress",
+        "matchPattern": "pref 1 u32",
+        "matchCount": "4095",
+        "teardown": [
+            "$TC qdisc del dev $DUMMY clsact"
+        ]
     }
 ]
-- 
2.43.0