Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1660404 > unrolled thread

[PATCH v5 00/13] Unseeded In-Kernel Randomness Fixes

Started by"Jason A. Donenfeld" <Jason@zx2c4.com>
First post2017-06-08 01:30 +0200
Last post2017-06-08 01:30 +0200
Articles 3 on this page of 23 — 3 participants

Back to article view | Back to linux.kernel


Contents

  [PATCH v5 00/13] Unseeded In-Kernel Randomness Fixes "Jason A. Donenfeld" <Jason@zx2c4.com> - 2017-06-08 01:30 +0200
    [PATCH v5 13/13] random: warn when kernel uses unseeded randomness "Jason A. Donenfeld" <Jason@zx2c4.com> - 2017-06-08 01:30 +0200
    [PATCH v5 06/13] iscsi: ensure RNG is seeded before use "Jason A. Donenfeld" <Jason@zx2c4.com> - 2017-06-08 01:30 +0200
    [PATCH v5 05/13] crypto/rng: ensure that the RNG is ready before using "Jason A. Donenfeld" <Jason@zx2c4.com> - 2017-06-08 01:30 +0200
    [PATCH v5 11/13] net/route: use get_random_int for random counter "Jason A. Donenfeld" <Jason@zx2c4.com> - 2017-06-08 01:30 +0200
    [PATCH v5 10/13] net/neighbor: use get_random_u32 for 32-bit hash random "Jason A. Donenfeld" <Jason@zx2c4.com> - 2017-06-08 01:30 +0200
    [PATCH v5 08/13] cifs: use get_random_u32 for 32-bit lock random "Jason A. Donenfeld" <Jason@zx2c4.com> - 2017-06-08 01:30 +0200
    [PATCH v5 12/13] bluetooth/smp: ensure RNG is properly seeded before ECDH use "Jason A. Donenfeld" <Jason@zx2c4.com> - 2017-06-08 01:30 +0200
    [PATCH v5 02/13] random: add synchronous API for the urandom pool "Jason A. Donenfeld" <Jason@zx2c4.com> - 2017-06-08 01:30 +0200
    [PATCH v5 01/13] random: invalidate batched entropy after crng init "Jason A. Donenfeld" <Jason@zx2c4.com> - 2017-06-08 01:30 +0200
      Re: [PATCH v5 01/13] random: invalidate batched entropy after crng  init Sebastian Andrzej Siewior <sebastian@breakpoint.cc> - 2017-06-14 21:40 +0200
        Re: [PATCH v5 01/13] random: invalidate batched entropy after crng init "Jason A. Donenfeld" <Jason@zx2c4.com> - 2017-06-15 00:40 +0200
          Re: [PATCH v5 01/13] random: invalidate batched entropy after crng  init Sebastian Andrzej Siewior <sebastian@breakpoint.cc> - 2017-06-16 10:40 +0200
            Re: [PATCH v5 01/13] random: invalidate batched entropy after crng init "Jason A. Donenfeld" <Jason@zx2c4.com> - 2017-06-16 14:20 +0200
              Re: [PATCH v5 01/13] random: invalidate batched entropy after crng  init Sebastian Andrzej Siewior <sebastian@breakpoint.cc> - 2017-06-16 16:40 +0200
        [PATCH] random: silence compiler warnings and fix race "Jason A. Donenfeld" <Jason@zx2c4.com> - 2017-06-15 00:50 +0200
          Re: [PATCH] random: silence compiler warnings and fix race Sebastian Andrzej Siewior <bigeasy@linutronix.de> - 2017-06-16 16:40 +0200
            Re: [PATCH] random: silence compiler warnings and fix race "Jason A. Donenfeld" <Jason@zx2c4.com> - 2017-06-17 02:40 +0200
              Re: [PATCH] random: silence compiler warnings and fix race Sebastian Andrzej Siewior <bigeasy@linutronix.de> - 2017-06-19 09:50 +0200
                Re: [PATCH] random: silence compiler warnings and fix race "Jason A. Donenfeld" <Jason@zx2c4.com> - 2017-06-19 23:00 +0200
          Re: [PATCH] random: silence compiler warnings and fix race "Jason A. Donenfeld" <Jason@zx2c4.com> - 2017-06-19 23:00 +0200
    [PATCH v5 03/13] random: add get_random_{bytes,u32,u64,int,long,once}_wait family "Jason A. Donenfeld" <Jason@zx2c4.com> - 2017-06-08 01:30 +0200
    [PATCH v5 04/13] security/keys: ensure RNG is seeded before use "Jason A. Donenfeld" <Jason@zx2c4.com> - 2017-06-08 01:30 +0200

Page 2 of 2 — ← Prev page 1 [2]


#1669946 — Re: [PATCH] random: silence compiler warnings and fix race

From"Jason A. Donenfeld" <Jason@zx2c4.com>
Date2017-06-19 23:00 +0200
SubjectRe: [PATCH] random: silence compiler warnings and fix race
Message-ID<tUbWa-7E3-21@gated-at.bofh.it>
In reply to#1666305
Hello Ted,

With rc6 already released and rc7 coming up, I'd really appreciate you
stepping in here and either ACKing the above commit, or giving your
two cents about it in case I need to roll something different.

Thanks,
Jason

On Thu, Jun 15, 2017 at 12:45 AM, Jason A. Donenfeld <Jason@zx2c4.com> wrote:
> Odd versions of gcc for the sh4 architecture will actually warn about
> flags being used while uninitialized, so we set them to zero. Non crazy
> gccs will optimize that out again, so it doesn't make a difference.
>
> Next, over aggressive gccs could inline the expression that defines
> use_lock, which could then introduce a race resulting in a lock
> imbalance. By using READ_ONCE, we prevent that fate. Finally, we make
> that assignment const, so that gcc can still optimize a nice amount.
>
> Finally, we fix a potential deadlock between primary_crng.lock and
> batched_entropy_reset_lock, where they could be called in opposite
> order. Moving the call to invalidate_batched_entropy to outside the lock
> rectifies this issue.
>
> Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
> ---
> Ted -- the first part of this is the fixup patch we discussed earlier.
> Then I added on top a fix for a potentially related race.
>
> I'm not totally convinced that moving this block to outside the spinlock
> is 100% okay, so please give this a close look before merging.
>
>
>  drivers/char/random.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/char/random.c b/drivers/char/random.c
> index e870f329db88..01a260f67437 100644
> --- a/drivers/char/random.c
> +++ b/drivers/char/random.c
> @@ -803,13 +803,13 @@ static int crng_fast_load(const char *cp, size_t len)
>                 p[crng_init_cnt % CHACHA20_KEY_SIZE] ^= *cp;
>                 cp++; crng_init_cnt++; len--;
>         }
> +       spin_unlock_irqrestore(&primary_crng.lock, flags);
>         if (crng_init_cnt >= CRNG_INIT_CNT_THRESH) {
>                 invalidate_batched_entropy();
>                 crng_init = 1;
>                 wake_up_interruptible(&crng_init_wait);
>                 pr_notice("random: fast init done\n");
>         }
> -       spin_unlock_irqrestore(&primary_crng.lock, flags);
>         return 1;
>  }
>
> @@ -841,6 +841,7 @@ static void crng_reseed(struct crng_state *crng, struct entropy_store *r)
>         }
>         memzero_explicit(&buf, sizeof(buf));
>         crng->init_time = jiffies;
> +       spin_unlock_irqrestore(&primary_crng.lock, flags);
>         if (crng == &primary_crng && crng_init < 2) {
>                 invalidate_batched_entropy();
>                 crng_init = 2;
> @@ -848,7 +849,6 @@ static void crng_reseed(struct crng_state *crng, struct entropy_store *r)
>                 wake_up_interruptible(&crng_init_wait);
>                 pr_notice("random: crng init done\n");
>         }
> -       spin_unlock_irqrestore(&primary_crng.lock, flags);
>  }
>
>  static inline void crng_wait_ready(void)
> @@ -2041,8 +2041,8 @@ static DEFINE_PER_CPU(struct batched_entropy, batched_entropy_u64);
>  u64 get_random_u64(void)
>  {
>         u64 ret;
> -       bool use_lock = crng_init < 2;
> -       unsigned long flags;
> +       bool use_lock = READ_ONCE(crng_init) < 2;
> +       unsigned long flags = 0;
>         struct batched_entropy *batch;
>
>  #if BITS_PER_LONG == 64
> @@ -2073,8 +2073,8 @@ static DEFINE_PER_CPU(struct batched_entropy, batched_entropy_u32);
>  u32 get_random_u32(void)
>  {
>         u32 ret;
> -       bool use_lock = crng_init < 2;
> -       unsigned long flags;
> +       bool use_lock = READ_ONCE(crng_init) < 2;
> +       unsigned long flags = 0;
>         struct batched_entropy *batch;
>
>         if (arch_get_random_int(&ret))
> --
> 2.13.1
>

[toc] | [prev] | [next] | [standalone]


#1660434 — [PATCH v5 03/13] random: add get_random_{bytes,u32,u64,int,long,once}_wait family

From"Jason A. Donenfeld" <Jason@zx2c4.com>
Date2017-06-08 01:30 +0200
Subject[PATCH v5 03/13] random: add get_random_{bytes,u32,u64,int,long,once}_wait family
Message-ID<tPSyM-4M5-77@gated-at.bofh.it>
In reply to#1660404
These functions are simple convenience wrappers that call
wait_for_random_bytes before calling the respective get_random_*
function.

Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
---
 include/linux/net.h    |  2 ++
 include/linux/once.h   |  2 ++
 include/linux/random.h | 25 +++++++++++++++++++++++++
 3 files changed, 29 insertions(+)

diff --git a/include/linux/net.h b/include/linux/net.h
index abcfa46a2bd9..dda2cc939a53 100644
--- a/include/linux/net.h
+++ b/include/linux/net.h
@@ -274,6 +274,8 @@ do {									\
 
 #define net_get_random_once(buf, nbytes)			\
 	get_random_once((buf), (nbytes))
+#define net_get_random_once_wait(buf, nbytes)			\
+	get_random_once_wait((buf), (nbytes))
 
 int kernel_sendmsg(struct socket *sock, struct msghdr *msg, struct kvec *vec,
 		   size_t num, size_t len);
diff --git a/include/linux/once.h b/include/linux/once.h
index 285f12cb40e6..9c98aaa87cbc 100644
--- a/include/linux/once.h
+++ b/include/linux/once.h
@@ -53,5 +53,7 @@ void __do_once_done(bool *done, struct static_key *once_key,
 
 #define get_random_once(buf, nbytes)					     \
 	DO_ONCE(get_random_bytes, (buf), (nbytes))
+#define get_random_once_wait(buf, nbytes)                                    \
+	DO_ONCE(get_random_bytes_wait, (buf), (nbytes))                      \
 
 #endif /* _LINUX_ONCE_H */
diff --git a/include/linux/random.h b/include/linux/random.h
index e29929347c95..4aecc339558d 100644
--- a/include/linux/random.h
+++ b/include/linux/random.h
@@ -58,6 +58,31 @@ static inline unsigned long get_random_long(void)
 #endif
 }
 
+/* Calls wait_for_random_bytes() and then calls get_random_bytes(buf, nbytes).
+ * Returns the result of the call to wait_for_random_bytes. */
+static inline int get_random_bytes_wait(void *buf, int nbytes)
+{
+	int ret = wait_for_random_bytes();
+	if (unlikely(ret))
+		return ret;
+	get_random_bytes(buf, nbytes);
+	return 0;
+}
+
+#define declare_get_random_var_wait(var) \
+	static inline int get_random_ ## var ## _wait(var *out) { \
+		int ret = wait_for_random_bytes(); \
+		if (unlikely(ret)) \
+			return ret; \
+		*out = get_random_ ## var(); \
+		return 0; \
+	}
+declare_get_random_var_wait(u32)
+declare_get_random_var_wait(u64)
+declare_get_random_var_wait(int)
+declare_get_random_var_wait(long)
+#undef declare_get_random_var
+
 unsigned long randomize_page(unsigned long start, unsigned long range);
 
 u32 prandom_u32(void);
-- 
2.13.0

[toc] | [prev] | [next] | [standalone]


#1660440 — [PATCH v5 04/13] security/keys: ensure RNG is seeded before use

From"Jason A. Donenfeld" <Jason@zx2c4.com>
Date2017-06-08 01:30 +0200
Subject[PATCH v5 04/13] security/keys: ensure RNG is seeded before use
Message-ID<tPSyM-4M5-87@gated-at.bofh.it>
In reply to#1660404
Otherwise, we might use bad random numbers which, particularly in the
case of IV generation, could be quite bad. It makes sense to use the
synchronous API here, because we're always in process context (as the
code is littered with GFP_KERNEL and the like). However, we can't change
to using a blocking function in key serial allocation, because this will
block booting in some configurations, so here we use the more
appropriate get_random_u32, which will use RDRAND if available.

Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
Cc: David Howells <dhowells@redhat.com>
Cc: Mimi Zohar <zohar@linux.vnet.ibm.com>
Cc: David Safford <safford@us.ibm.com>
---
 security/keys/encrypted-keys/encrypted.c |  8 +++++---
 security/keys/key.c                      | 16 ++++++++--------
 2 files changed, 13 insertions(+), 11 deletions(-)

diff --git a/security/keys/encrypted-keys/encrypted.c b/security/keys/encrypted-keys/encrypted.c
index 0010955d7876..d51a28fc5cd5 100644
--- a/security/keys/encrypted-keys/encrypted.c
+++ b/security/keys/encrypted-keys/encrypted.c
@@ -777,10 +777,12 @@ static int encrypted_init(struct encrypted_key_payload *epayload,
 
 	__ekey_init(epayload, format, master_desc, datalen);
 	if (!hex_encoded_iv) {
-		get_random_bytes(epayload->iv, ivsize);
+		ret = get_random_bytes_wait(epayload->iv, ivsize);
+		if (unlikely(ret))
+			return ret;
 
-		get_random_bytes(epayload->decrypted_data,
-				 epayload->decrypted_datalen);
+		ret = get_random_bytes_wait(epayload->decrypted_data,
+					    epayload->decrypted_datalen);
 	} else
 		ret = encrypted_key_decrypt(epayload, format, hex_encoded_iv);
 	return ret;
diff --git a/security/keys/key.c b/security/keys/key.c
index 455c04d80bbb..b72078e532f2 100644
--- a/security/keys/key.c
+++ b/security/keys/key.c
@@ -134,17 +134,15 @@ void key_user_put(struct key_user *user)
  * Allocate a serial number for a key.  These are assigned randomly to avoid
  * security issues through covert channel problems.
  */
-static inline void key_alloc_serial(struct key *key)
+static inline int key_alloc_serial(struct key *key)
 {
 	struct rb_node *parent, **p;
 	struct key *xkey;
 
-	/* propose a random serial number and look for a hole for it in the
-	 * serial number tree */
+	/* propose a non-negative random serial number and look for a hole for
+	 * it in the serial number tree */
 	do {
-		get_random_bytes(&key->serial, sizeof(key->serial));
-
-		key->serial >>= 1; /* negative numbers are not permitted */
+		key->serial = get_random_u32() >> 1;
 	} while (key->serial < 3);
 
 	spin_lock(&key_serial_lock);
@@ -170,7 +168,7 @@ static inline void key_alloc_serial(struct key *key)
 	rb_insert_color(&key->serial_node, &key_serial_tree);
 
 	spin_unlock(&key_serial_lock);
-	return;
+	return 0;
 
 	/* we found a key with the proposed serial number - walk the tree from
 	 * that point looking for the next unused serial number */
@@ -314,7 +312,9 @@ struct key *key_alloc(struct key_type *type, const char *desc,
 
 	/* publish the key by giving it a serial number */
 	atomic_inc(&user->nkeys);
-	key_alloc_serial(key);
+	ret = key_alloc_serial(key);
+	if (ret < 0)
+		goto security_error;
 
 error:
 	return key;
-- 
2.13.0

[toc] | [prev] | [standalone]


Page 2 of 2 — ← Prev page 1 [2]

Back to top | Article view | linux.kernel


csiph-web