netdev - Re: [PATCH 1/2] netfilter: fix the hash random initializing race

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: <1282321023.2484.285.camel@edumazet-laptop>
Date:	Fri, 20 Aug 2010 18:17:03 +0200
From:	Eric Dumazet <eric.dumazet@...il.com>
To:	Changli Gao <xiaosuo@...il.com>
Cc:	Patrick McHardy <kaber@...sh.net>,
	"David S. Miller" <davem@...emloft.net>,
	netfilter-devel@...r.kernel.org, netdev@...r.kernel.org
Subject: Re: [PATCH 1/2] netfilter: fix the hash random initializing race

Le samedi 21 août 2010 à 00:06 +0800, Changli Gao a écrit :
> nf_conntrack_alloc() isn't called with nf_conntrack_lock locked, so hash
> random initializing code maybe executed more than once on different CPUs.
> 
> Signed-off-by: Changli Gao <xiaosuo@...il.com>
> ---
>  net/netfilter/nf_conntrack_core.c |   17 +++++++++++------
>  1 file changed, 11 insertions(+), 6 deletions(-)
> diff --git a/net/netfilter/nf_conntrack_core.c b/net/netfilter/nf_conntrack_core.c
> index df3eedb..7ae7f71 100644
> --- a/net/netfilter/nf_conntrack_core.c
> +++ b/net/netfilter/nf_conntrack_core.c
> @@ -65,8 +65,7 @@ EXPORT_SYMBOL_GPL(nf_conntrack_max);
>  DEFINE_PER_CPU(struct nf_conn, nf_conntrack_untracked);
>  EXPORT_PER_CPU_SYMBOL(nf_conntrack_untracked);
>  
> -static int nf_conntrack_hash_rnd_initted;
> -static unsigned int nf_conntrack_hash_rnd;
> +static unsigned int nf_conntrack_hash_rnd __read_mostly;
>  
>  static u_int32_t __hash_conntrack(const struct nf_conntrack_tuple *tuple,
>  				  u16 zone, unsigned int size, unsigned int rnd)
> @@ -574,10 +573,16 @@ struct nf_conn *nf_conntrack_alloc(struct net *net, u16 zone,
>  {
>  	struct nf_conn *ct;
>  
> -	if (unlikely(!nf_conntrack_hash_rnd_initted)) {
> -		get_random_bytes(&nf_conntrack_hash_rnd,
> -				sizeof(nf_conntrack_hash_rnd));
> -		nf_conntrack_hash_rnd_initted = 1;
> +	if (unlikely(!nf_conntrack_hash_rnd)) {
> +		unsigned int rand;
> +
> +		/* Why not initialize nf_conntrack_rnd in a "init()" function ?
> +		 * Because there isn't enough entropy when system initializing,
> +		 * and we initialize it as late as possible. */

This patch is fine but a fine multi line comment is :
	/* Some
	 * fine
	 * comment
	 */

> +		do {
> +			get_random_bytes(&rand, sizeof(rand));
> +		} while (!rand);
> +		cmpxchg(&nf_conntrack_hash_rnd, 0, rand);
>  	}
>  
>  	/* We don't want any race condition at early drop stage */


--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html