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

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 <redacted>
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