netdev - U32 - systematic ht leak since ... the beginning

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [day] [month] [year] [list]

Message-ID: <74eaea75-5231-4ab9-a11e-4647f9eb8e4e@orange.com>
Date: Fri, 1 Nov 2024 01:24:24 +0100
From: Alexandre Ferrieux <alexandre.ferrieux@...il.com>
To: netdev@...r.kernel.org
Cc: Eric Dumazet <edumazet@...gle.com>
Subject: U32 - systematic ht leak since ... the beginning

Hi,

When creating an u32 filter needing a new hashtable, gen_new_htid() is called to
allocate its ID. It is (currently) based on IDR, but "encodes" the
IDR-originated ID in a peculiar way:

  static u32 gen_new_htid(struct tc_u_common *tp_c, struct tc_u_hnode *ptr)
  {
        int id = idr_alloc_cyclic(&tp_c->handle_idr, ptr, 1, 0x7FF, GFP_KERNEL);
        if (id < 0)
                return 0;
        return (id | 0x800U) << 20;
  }

Unfortunately, on the remove side, there is no corresponding "decoding", and the
garbled value it passed untouched to idr_remove():

  static int u32_destroy_hnode(struct tcf_proto *tp, struct tc_u_hnode *ht,
                             struct netlink_ext_ack *extack)
  {
  ...
                        idr_remove(&tp_c->handle_idr, ht->handle);
  ...
  }

As a consequence, the IDR fills up. Eventually, all attempts to create new
filters on the interface get rejected. Additionally, the error is cryptic
("Filter already exists") due to the special handle zero (returned by
gen_new_tid) *not* being intercepted, and being used as a normal value instead,
hence the collision.

Unless I'm mistaken, this dates back to .. the beginning of (git) time; the
problem was apparently identical before the migration to IDR:

^1da177e4c3f4 (Linus Torvalds     2005-04-16 15:20:36 -0700
  326)       return i > 0 ? (tp_c->hgenerator|0x800)<<20 : 0;

Please tell me I'm wrong :}

-Alex