All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH net 1/2] net/sched: cls_u32: fix duplicate handle when node id pool exhausted
@ 2026-08-20  9:52 Jamal Hadi Salim
  2026-08-20  9:52 ` [PATCH net 2/2] selftests: tc-testing: add u32 node ID pool exhaustion test Jamal Hadi Salim
  2026-08-20 16:40 ` [PATCH net 1/2] net/sched: cls_u32: fix duplicate handle when node id pool exhausted Jamal Hadi Salim
  0 siblings, 2 replies; 4+ messages in thread
From: Jamal Hadi Salim @ 2026-08-20  9:52 UTC (permalink / raw)
  To: netdev
  Cc: Jamal Hadi Salim, Jiri Pirko, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Simon Horman, stable, vega,
	Victor Nogueira

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


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [PATCH net 2/2] selftests: tc-testing: add u32 node ID pool exhaustion test
  2026-08-20  9:52 [PATCH net 1/2] net/sched: cls_u32: fix duplicate handle when node id pool exhausted Jamal Hadi Salim
@ 2026-08-20  9:52 ` Jamal Hadi Salim
  2026-08-20 16:42   ` Jamal Hadi Salim
  2026-08-20 16:40 ` [PATCH net 1/2] net/sched: cls_u32: fix duplicate handle when node id pool exhausted Jamal Hadi Salim
  1 sibling, 1 reply; 4+ messages in thread
From: Jamal Hadi Salim @ 2026-08-20  9:52 UTC (permalink / raw)
  To: netdev
  Cc: Jamal Hadi Salim, Jiri Pirko, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Simon Horman, stable, vega,
	Victor Nogueira

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


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH net 1/2] net/sched: cls_u32: fix duplicate handle when node id pool exhausted
  2026-08-20  9:52 [PATCH net 1/2] net/sched: cls_u32: fix duplicate handle when node id pool exhausted Jamal Hadi Salim
  2026-08-20  9:52 ` [PATCH net 2/2] selftests: tc-testing: add u32 node ID pool exhaustion test Jamal Hadi Salim
@ 2026-08-20 16:40 ` Jamal Hadi Salim
  1 sibling, 0 replies; 4+ messages in thread
From: Jamal Hadi Salim @ 2026-08-20 16:40 UTC (permalink / raw)
  To: netdev
  Cc: Jiri Pirko, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Simon Horman, stable, vega, Victor Nogueira

On Thu, Aug 20, 2026 at 5:52 AM Jamal Hadi Salim <jhs@mojatatu.com> wrote:
>
> 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

Some of our long running tests have failed because primarily of the
tdc test in this set. Will resend with a renewed tdc test.

--
pw-bot: cr

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH net 2/2] selftests: tc-testing: add u32 node ID pool exhaustion test
  2026-08-20  9:52 ` [PATCH net 2/2] selftests: tc-testing: add u32 node ID pool exhaustion test Jamal Hadi Salim
@ 2026-08-20 16:42   ` Jamal Hadi Salim
  0 siblings, 0 replies; 4+ messages in thread
From: Jamal Hadi Salim @ 2026-08-20 16:42 UTC (permalink / raw)
  To: netdev
  Cc: Jiri Pirko, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Simon Horman, stable, vega, Victor Nogueira

On Thu, Aug 20, 2026 at 5:52 AM Jamal Hadi Salim <jhs@mojatatu.com> wrote:
>
> 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

Some of our long running tests have failed because primarily of the
tdc test in this set. Will resend with a renewed tdc test. Hopefully
my attempt at this pw-bot thing works.

--
pw-bot: cr

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-08-20 16:42 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-20  9:52 [PATCH net 1/2] net/sched: cls_u32: fix duplicate handle when node id pool exhausted Jamal Hadi Salim
2026-08-20  9:52 ` [PATCH net 2/2] selftests: tc-testing: add u32 node ID pool exhaustion test Jamal Hadi Salim
2026-08-20 16:42   ` Jamal Hadi Salim
2026-08-20 16:40 ` [PATCH net 1/2] net/sched: cls_u32: fix duplicate handle when node id pool exhausted Jamal Hadi Salim

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.