netdev - Re: [PATCH RFC] net: introduce support for lazy initialization of secret keys

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]

Date:	Wed, 25 Sep 2013 15:42:26 -0700
From:	Eric Dumazet <eric.dumazet@...il.com>
To:	Hannes Frederic Sowa <hannes@...essinduktion.org>
Cc:	netdev@...r.kernel.org, fw@...len.de, edumazet@...gle.com,
	davem@...emloft.net, ycheng@...gle.com
Subject: Re: [PATCH RFC] net: introduce support for lazy initialization of
 secret keys

On Wed, 2013-09-25 at 23:34 +0200, Hannes Frederic Sowa wrote:
> net_get_random_once is a new macro which handles the initialization of
> secret keys at use-site. It is possible to call it in the fast path. Only
> the initialization depends on the spinlock and is rather slow. Otherwise
> it should get used just before the key is used to delay the entropy
> extration as late as possible to get better randomness. It returns true
> if the key got initialized.

So you don't like cmpxchg() ;)

> +/* BE CAREFUL: this function is not interrupt safe */
> +#define net_get_random_once(buf, nbytes)				\
> +	({								\
> +		static DEFINE_SPINLOCK(__lock);				\
> +		static bool __done = false;				\
> +		bool __ret = false;					\
> +		if (unlikely(!__done))					\
> +			__ret = __net_get_random_once(buf,		\
> +						    nbytes,		\
> +						    &__done,		\
> +						    &__lock);		\
> +		__ret;							\
> +	})
> +
>  

No idea why its needed to have one spinlock per call point.

A single lock should be more than enough.

The spinlock could be private to __net_get_random_once()

+bool __net_get_random_once(void *buf, int nbytes, bool *done,
+                           spinlock_t *lock)
+{
+       spin_lock_bh(lock);
+       if (*done) {
+               spin_unlock_bh(lock);
+               return false;
+       }
+
+       get_random_bytes(buf, nbytes);

I think you might need a memory barrier here.

(smp_wmb();)

+       *done = true;
+       spin_unlock_bh(lock);


BTW, build_ehash_secret() is called like that :

if (unlikely(!inet_ehash_secret))
    if (sock->type != SOCK_RAW && sock->type != SOCK_DGRAM)
        build_ehash_secret();

So it would be better to make sure inet_ehash_secret is not 0 by
accident.



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