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>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <290f16f7-8f31-46c9-907d-ce298a9b8630@orange.com>
Date: Tue, 24 Sep 2024 19:18:48 +0200
From: Alexandre Ferrieux <alexandre.ferrieux@...il.com>
To: Eric Dumazet <edumazet@...gle.com>,
 Alexandre Ferrieux <alexandre.ferrieux@...il.com>
Cc: Simon Horman <horms@...nel.org>,
 Przemek Kitszel <przemyslaw.kitszel@...el.com>, netdev@...r.kernel.org
Subject: Re: Massive hash collisions on FIB

On 24/09/2024 16:36, Eric Dumazet wrote:
> 
>> [...] the single FIB
>> hashtable, shared by all netns, lends itself to massive collision if many netns
>> contain the same local address.
> 
> Shocking
>>
>> To solve this, I'd naively inject a few bits of entropy from the netns itself
>> (inode number, middle bits of (struct net *) address, etc.), by XORing them to
>> the hash value. Unless I'm mistaken, the netns is always unambiguous when a FIB
>> decision is made, be it for a packet or for some interface configuration task.
>>
>> Would that be acceptable ?
> 
> Sure, but please use the standard way : net_hash_mix(net)

I see you did the work for the two other hashes (laddr and devindex).
I tried to inject the dispersion the same way as you did, just before the final
scrambling. Is this what you'd do ?


diff --git a/net/ipv4/fib_semantics.c b/net/ipv4/fib_semantics.c
index f669da98d11d..49fea7cf0860 100644
--- a/net/ipv4/fib_semantics.c
+++ b/net/ipv4/fib_semantics.c
@@ -347,10 +347,12 @@ static unsigned int fib_info_hashfn_1(int init_val, u8
protocol, u8 scope,
        return val;
 }

-static unsigned int fib_info_hashfn_result(unsigned int val)
+static unsigned int fib_info_hashfn_result(struct net *net, unsigned int val)
 {
        unsigned int mask = (fib_info_hash_size - 1);

+       val ^= net_hash_mix(net);
+
        return (val ^ (val >> 7) ^ (val >> 12)) & mask;
 }

@@ -370,7 +372,7 @@ static inline unsigned int fib_info_hashfn(struct fib_info *fi)
                } endfor_nexthops(fi)
        }

-       return fib_info_hashfn_result(val);
+       return fib_info_hashfn_result(fi->fib_net, val);
 }

 /* no metrics, only nexthop id */
@@ -385,7 +387,7 @@ static struct fib_info *fib_find_info_nh(struct net *net,
                                 cfg->fc_protocol, cfg->fc_scope,
                                 (__force u32)cfg->fc_prefsrc,
                                 cfg->fc_priority);
-       hash = fib_info_hashfn_result(hash);
+       hash = fib_info_hashfn_result(net, hash);
        head = &fib_info_hash[hash];

        hlist_for_each_entry(fi, head, fib_hash) {

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ