[PATCH v6 0/7] random,x86,kvm: Rework arch RNG seeds and get some from kvm

[PATCH v6 0/7] random,x86,kvm: Rework arch RNG seeds and get some from kvm

[Andy Lutomirski]

This introduces and uses a very simple synchronous mechanism to get
/dev/urandom-style bits appropriate for initial KVM PV guest RNG
seeding.

It also re-works the way that architectural random data is fed into
random.c's pools.  Timekeeping randomness now comes directly from
the timekeeping core rather than being pulled in from init_std_data,
and timekeeping randomness is added both on boot and on resume.  I
added a new arch hook called arch_rng_init.  The default
implementation is more or less the same as the current code, except
that random_get_entropy is now called unconditionally.  We now also
call init_std_data on resume.

x86 gets a custom arch_rng_init.  It will use KVM_GET_RNG_SEED if
available, and, if it does anything, it will log the number of bits
collected from each available architectural source.  If more
paravirt seed sources show up, it will be a natural place to add
them.

I sent the corresponding kvm-unit-tests and qemu changes separately.

Changes from v5:
 - Moved the generic changes to the beginning.
 - Renamed arch_get_rng_seed to arch_rng_init.
 - The timekeeping change is new.
 - random.c registers a syscore callback to reseed on resume.

Changes from v4:
 - Got rid of the RDRAND behavior change.  If this series is accepted,
   I may resend it separately, but I think it's an unrelated issue.
 - Fix up the changelog entries -- I misunderstood how the old code
   worked.
 - Avoid lots of failed attempts to use KVM_GET_RNG_SEED if it's not
   available.

Changes from v3:
 - Other than KASLR, the guest pieces are completely rewritten.
   Patches 2-4 have essentially nothing in common with v2.

Changes from v2:
 - Bisection fix (patch 2 had a misplaced brace).  The final states is
   identical to that of v2.
 - Improve the 0/5 description a little bit.

Changes from v1:
 - Split patches 2 and 3
 - Log all arch sources in init_std_data
 - Fix the 32-bit kaslr build

Andy Lutomirski (7):
  random: Add and use arch_rng_init
  random, timekeeping: Collect timekeeping entropy in the timekeeping
    code
  random: Reseed pools on resume
  x86,kvm: Add MSR_KVM_GET_RNG_SEED and a matching feature bit
  x86,random: Add an x86 implementation of arch_rng_init
  x86,random,kvm: Use KVM_GET_RNG_SEED in arch_rng_init
  x86,kaslr: Use MSR_KVM_GET_RNG_SEED for KASLR if available

 Documentation/virtual/kvm/cpuid.txt  |  3 ++
 arch/x86/Kconfig                     |  4 ++
 arch/x86/boot/compressed/aslr.c      | 27 +++++++++++++
 arch/x86/include/asm/archrandom.h    |  6 +++
 arch/x86/include/asm/kvm_guest.h     |  9 +++++
 arch/x86/include/asm/processor.h     | 21 ++++++++--
 arch/x86/include/uapi/asm/kvm_para.h |  2 +
 arch/x86/kernel/Makefile             |  2 +
 arch/x86/kernel/archrandom.c         | 74 ++++++++++++++++++++++++++++++++++++
 arch/x86/kernel/kvm.c                | 10 +++++
 arch/x86/kvm/cpuid.c                 |  3 +-
 arch/x86/kvm/x86.c                   |  4 ++
 drivers/char/random.c                | 42 ++++++++++++++++----
 include/linux/random.h               | 40 +++++++++++++++++++
 kernel/time/timekeeping.c            | 11 ++++++
 15 files changed, 246 insertions(+), 12 deletions(-)
 create mode 100644 arch/x86/kernel/archrandom.c

-- 
1.9.3

[PATCH v6 1/7] random: Add and use arch_rng_init

[Andy Lutomirski]

Currently, init_std_data contains its own logic for using arch
random sources.  This replaces that logic with a generic function
arch_rng_init that allows arch code to supply its own logic.  The
default implementation tries arch_get_random_seed_long and
arch_get_random_long individually.

The only functional change here is that random_get_entropy() is used
unconditionally instead of being used only when the arch sources
fail.  This may add a tiny amount of security.

Acked-by: Theodore Ts'o <tytso@mit.edu>
Signed-off-by: Andy Lutomirski <luto@amacapital.net>
---
 drivers/char/random.c  | 14 +++++++++++---
 include/linux/random.h | 40 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 51 insertions(+), 3 deletions(-)

diff --git a/drivers/char/random.c b/drivers/char/random.c
index 71529e1..7673e60 100644
--- a/drivers/char/random.c
+++ b/drivers/char/random.c
@@ -1246,6 +1246,10 @@ void get_random_bytes_arch(void *buf, int nbytes)
 }
 EXPORT_SYMBOL(get_random_bytes_arch);
 
+static void seed_entropy_store(void *ctx, u32 data)
+{
+	mix_pool_bytes((struct entropy_store *)ctx, &data, sizeof(data), NULL);
+}
 
 /*
  * init_std_data - initialize pool with system data
@@ -1261,15 +1265,19 @@ static void init_std_data(struct entropy_store *r)
 	int i;
 	ktime_t now = ktime_get_real();
 	unsigned long rv;
+	char log_prefix[128];
 
 	r->last_pulled = jiffies;
 	mix_pool_bytes(r, &now, sizeof(now), NULL);
 	for (i = r->poolinfo->poolbytes; i > 0; i -= sizeof(rv)) {
-		if (!arch_get_random_seed_long(&rv) &&
-		    !arch_get_random_long(&rv))
-			rv = random_get_entropy();
+		rv = random_get_entropy();
 		mix_pool_bytes(r, &rv, sizeof(rv), NULL);
 	}
+
+	sprintf(log_prefix, "random: seeded %s pool", r->name);
+	arch_rng_init(r, seed_entropy_store, 8 * r->poolinfo->poolbytes,
+		      log_prefix);
+
 	mix_pool_bytes(r, utsname(), sizeof(*(utsname())), NULL);
 }
 
diff --git a/include/linux/random.h b/include/linux/random.h
index 57fbbff..c8d692e 100644
--- a/include/linux/random.h
+++ b/include/linux/random.h
@@ -106,6 +106,46 @@ static inline int arch_has_random_seed(void)
 }
 #endif
 
+#ifndef __HAVE_ARCH_RNG_INIT
+
+/**
+ * arch_rng_init() - get architectural rng seed data
+ * @ctx: context for the seed function
+ * @seed: function to call for each u32 obtained
+ * @bits_per_source: number of bits from each source to try to use
+ * @log_prefix: beginning of log output (may be NULL)
+ *
+ * Synchronously load some architectural entropy or other best-effort
+ * random seed data.  An arch-specific implementation should be no worse
+ * than this generic implementation.  If the arch code does something
+ * interesting, it may log something of the form "log_prefix with
+ * 8 bits of stuff".
+ *
+ * No arch-specific implementation should be any worse than the generic
+ * implementation.
+ */
+static inline void arch_rng_init(void *ctx,
+				 void (*seed)(void *ctx, u32 data),
+				 int bits_per_source,
+				 const char *log_prefix)
+{
+	int i;
+
+	for (i = 0; i < bits_per_source; i += 8 * sizeof(long)) {
+		unsigned long rv;
+
+		if (arch_get_random_seed_long(&rv) ||
+		    arch_get_random_long(&rv)) {
+			seed(ctx, (u32)rv);
+#if BITS_PER_LONG > 32
+			seed(ctx, (u32)(rv >> 32));
+#endif
+		}
+	}
+}
+
+#endif /* __HAVE_ARCH_RNG_INIT */
+
 /* Pseudo random number generator from numerical recipes. */
 static inline u32 next_pseudo_random32(u32 seed)
 {
-- 
1.9.3

[PATCH v6 2/7] random, timekeeping: Collect timekeeping entropy in the timekeeping code

[Andy Lutomirski]

Currently, init_std_data calls ktime_get_real().  This imposes
awkward constraints on when init_std_data can be called, and
init_std_data is unlikely to collect the full unpredictable data
available to the timekeeping code, especially after resume.

Remove this code from random.c and add the appropriate
add_device_randomness calls to timekeeping.c instead.

Cc: John Stultz <john.stultz@linaro.org>
Signed-off-by: Andy Lutomirski <luto@amacapital.net>
---
 drivers/char/random.c     |  2 --
 kernel/time/timekeeping.c | 11 +++++++++++
 2 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/drivers/char/random.c b/drivers/char/random.c
index 7673e60..8dc3e3a 100644
--- a/drivers/char/random.c
+++ b/drivers/char/random.c
@@ -1263,12 +1263,10 @@ static void seed_entropy_store(void *ctx, u32 data)
 static void init_std_data(struct entropy_store *r)
 {
 	int i;
-	ktime_t now = ktime_get_real();
 	unsigned long rv;
 	char log_prefix[128];
 
 	r->last_pulled = jiffies;
-	mix_pool_bytes(r, &now, sizeof(now), NULL);
 	for (i = r->poolinfo->poolbytes; i > 0; i -= sizeof(rv)) {
 		rv = random_get_entropy();
 		mix_pool_bytes(r, &rv, sizeof(rv), NULL);
diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
index 32d8d6a..9609db9 100644
--- a/kernel/time/timekeeping.c
+++ b/kernel/time/timekeeping.c
@@ -23,6 +23,7 @@
 #include <linux/stop_machine.h>
 #include <linux/pvclock_gtod.h>
 #include <linux/compiler.h>
+#include <linux/random.h>
 
 #include "tick-internal.h"
 #include "ntp_internal.h"
@@ -835,6 +836,9 @@ void __init timekeeping_init(void)
 	memcpy(&shadow_timekeeper, &timekeeper, sizeof(timekeeper));
 
 	write_seqcount_end(&timekeeper_seq);
+
+	add_device_randomness(tk, sizeof(tk));
+
 	raw_spin_unlock_irqrestore(&timekeeper_lock, flags);
 }
 
@@ -976,6 +980,13 @@ static void timekeeping_resume(void)
 	timekeeping_suspended = 0;
 	timekeeping_update(tk, TK_MIRROR | TK_CLOCK_WAS_SET);
 	write_seqcount_end(&timekeeper_seq);
+
+	/*
+	 * The timekeeping state has a decent chance of differing
+	 * between resumptions of the same image.
+	 */
+	add_device_randomness(tk, sizeof(tk));
+
 	raw_spin_unlock_irqrestore(&timekeeper_lock, flags);
 
 	touch_softlockup_watchdog();
-- 
1.9.3

[PATCH v6 3/7] random: Reseed pools on resume

[Andy Lutomirski]

After a suspend/resume cycle, and especially after hibernating, we
should assume that the random pools might have leaked.  To minimize
the risk this poses, try to collect fresh architectural entropy on
resume.

Signed-off-by: Andy Lutomirski <luto@amacapital.net>
---
 drivers/char/random.c | 26 +++++++++++++++++++++++---
 1 file changed, 23 insertions(+), 3 deletions(-)

diff --git a/drivers/char/random.c b/drivers/char/random.c
index 8dc3e3a..0811ad4 100644
--- a/drivers/char/random.c
+++ b/drivers/char/random.c
@@ -257,6 +257,7 @@
 #include <linux/kmemcheck.h>
 #include <linux/workqueue.h>
 #include <linux/irq.h>
+#include <linux/syscore_ops.h>
 
 #include <asm/processor.h>
 #include <asm/uaccess.h>
@@ -1279,6 +1280,26 @@ static void init_std_data(struct entropy_store *r)
 	mix_pool_bytes(r, utsname(), sizeof(*(utsname())), NULL);
 }
 
+static void init_all_pools(void)
+{
+	init_std_data(&input_pool);
+	init_std_data(&blocking_pool);
+	init_std_data(&nonblocking_pool);
+}
+
+static void random_resume(void)
+{
+	/*
+	 * After resume (and especially after hibernation / kexec resume),
+	 * make a best-effort attempt to collect fresh entropy.
+	 */
+	init_all_pools();
+}
+
+static struct syscore_ops random_syscore_ops = {
+	.resume = random_resume,
+};
+
 /*
  * Note that setup_arch() may call add_device_randomness()
  * long before we get here. This allows seeding of the pools
@@ -1291,9 +1312,8 @@ static void init_std_data(struct entropy_store *r)
  */
 static int rand_initialize(void)
 {
-	init_std_data(&input_pool);
-	init_std_data(&blocking_pool);
-	init_std_data(&nonblocking_pool);
+	init_all_pools();
+	register_syscore_ops(&random_syscore_ops);
 	return 0;
 }
 early_initcall(rand_initialize);
-- 
1.9.3

[PATCH v6 4/7] x86,kvm: Add MSR_KVM_GET_RNG_SEED and a matching feature bit

[Andy Lutomirski]

This adds a simple interface to allow a guest to request 64 bits of
host nonblocking entropy.  This is independent of virtio-rng for a
couple of reasons:

 - It's intended to be usable during early boot, when a trivial
   synchronous interface is needed.

 - virtio-rng gives blocking entropy, and making guest boot wait for
   the host's /dev/random will cause problems.

MSR_KVM_GET_RNG_SEED is intended to provide 64 bits of best-effort
cryptographically secure data for use as a seed.  It provides no
guarantee that the result contains any actual entropy.

Acked-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Andy Lutomirski <luto@amacapital.net>
---
 Documentation/virtual/kvm/cpuid.txt  | 3 +++
 arch/x86/include/uapi/asm/kvm_para.h | 2 ++
 arch/x86/kvm/cpuid.c                 | 3 ++-
 arch/x86/kvm/x86.c                   | 4 ++++
 4 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/Documentation/virtual/kvm/cpuid.txt b/Documentation/virtual/kvm/cpuid.txt
index 3c65feb..0ab043b 100644
--- a/Documentation/virtual/kvm/cpuid.txt
+++ b/Documentation/virtual/kvm/cpuid.txt
@@ -54,6 +54,9 @@ KVM_FEATURE_PV_UNHALT              ||     7 || guest checks this feature bit
                                    ||       || before enabling paravirtualized
                                    ||       || spinlock support.
 ------------------------------------------------------------------------------
+KVM_FEATURE_GET_RNG_SEED           ||     8 || host provides rng seed data via
+                                   ||       || MSR_KVM_GET_RNG_SEED.
+------------------------------------------------------------------------------
 KVM_FEATURE_CLOCKSOURCE_STABLE_BIT ||    24 || host will warn if no guest-side
                                    ||       || per-cpu warps are expected in
                                    ||       || kvmclock.
diff --git a/arch/x86/include/uapi/asm/kvm_para.h b/arch/x86/include/uapi/asm/kvm_para.h
index 94dc8ca..e2eaf93 100644
--- a/arch/x86/include/uapi/asm/kvm_para.h
+++ b/arch/x86/include/uapi/asm/kvm_para.h
@@ -24,6 +24,7 @@
 #define KVM_FEATURE_STEAL_TIME		5
 #define KVM_FEATURE_PV_EOI		6
 #define KVM_FEATURE_PV_UNHALT		7
+#define KVM_FEATURE_GET_RNG_SEED	8
 
 /* The last 8 bits are used to indicate how to interpret the flags field
  * in pvclock structure. If no bits are set, all flags are ignored.
@@ -40,6 +41,7 @@
 #define MSR_KVM_ASYNC_PF_EN 0x4b564d02
 #define MSR_KVM_STEAL_TIME  0x4b564d03
 #define MSR_KVM_PV_EOI_EN      0x4b564d04
+#define MSR_KVM_GET_RNG_SEED 0x4b564d05
 
 struct kvm_steal_time {
 	__u64 steal;
diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
index 38a0afe..40d6763 100644
--- a/arch/x86/kvm/cpuid.c
+++ b/arch/x86/kvm/cpuid.c
@@ -479,7 +479,8 @@ static inline int __do_cpuid_ent(struct kvm_cpuid_entry2 *entry, u32 function,
 			     (1 << KVM_FEATURE_ASYNC_PF) |
 			     (1 << KVM_FEATURE_PV_EOI) |
 			     (1 << KVM_FEATURE_CLOCKSOURCE_STABLE_BIT) |
-			     (1 << KVM_FEATURE_PV_UNHALT);
+			     (1 << KVM_FEATURE_PV_UNHALT) |
+			     (1 << KVM_FEATURE_GET_RNG_SEED);
 
 		if (sched_info_on())
 			entry->eax |= (1 << KVM_FEATURE_STEAL_TIME);
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index ef432f8..695b682 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -48,6 +48,7 @@
 #include <linux/pci.h>
 #include <linux/timekeeper_internal.h>
 #include <linux/pvclock_gtod.h>
+#include <linux/random.h>
 #include <trace/events/kvm.h>
 
 #define CREATE_TRACE_POINTS
@@ -2480,6 +2481,9 @@ int kvm_get_msr_common(struct kvm_vcpu *vcpu, u32 msr, u64 *pdata)
 	case MSR_KVM_PV_EOI_EN:
 		data = vcpu->arch.pv_eoi.msr_val;
 		break;
+	case MSR_KVM_GET_RNG_SEED:
+		get_random_bytes(&data, sizeof(data));
+		break;
 	case MSR_IA32_P5_MC_ADDR:
 	case MSR_IA32_P5_MC_TYPE:
 	case MSR_IA32_MCG_CAP:
-- 
1.9.3

[PATCH v6 5/7] x86,random: Add an x86 implementation of arch_rng_init

[Andy Lutomirski]

This does the same thing as the generic implementation, except
that it logs how many bits of each type it collected.  I want to
know whether the initial seeding is working and, if so, whether
the RNG is fast enough.

(I know that hpa assures me that the hardware RNG is more than
 fast enough, but I'd still like a direct way to verify this.)

Arguably, arch_get_random_seed could be removed now: I'm having some
trouble imagining a sensible non-architecture-specific use of it
that wouldn't be better served by arch_rng_init.

Signed-off-by: Andy Lutomirski <luto@amacapital.net>
---
 arch/x86/include/asm/archrandom.h |  6 +++++
 arch/x86/kernel/Makefile          |  2 ++
 arch/x86/kernel/archrandom.c      | 51 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 59 insertions(+)
 create mode 100644 arch/x86/kernel/archrandom.c

diff --git a/arch/x86/include/asm/archrandom.h b/arch/x86/include/asm/archrandom.h
index 69f1366..5611c21 100644
--- a/arch/x86/include/asm/archrandom.h
+++ b/arch/x86/include/asm/archrandom.h
@@ -117,6 +117,12 @@ GET_SEED(arch_get_random_seed_int, unsigned int, RDSEED_INT, ASM_NOP4);
 #define arch_has_random()	static_cpu_has(X86_FEATURE_RDRAND)
 #define arch_has_random_seed()	static_cpu_has(X86_FEATURE_RDSEED)
 
+#define __HAVE_ARCH_RNG_INIT
+extern void arch_rng_init(void *ctx,
+			  void (*seed)(void *ctx, u32 data),
+			  int bits_per_source,
+			  const char *log_prefix);
+
 #else
 
 static inline int rdrand_long(unsigned long *v)
diff --git a/arch/x86/kernel/Makefile b/arch/x86/kernel/Makefile
index 047f9ff..0718bae 100644
--- a/arch/x86/kernel/Makefile
+++ b/arch/x86/kernel/Makefile
@@ -92,6 +92,8 @@ obj-$(CONFIG_PARAVIRT)		+= paravirt.o paravirt_patch_$(BITS).o
 obj-$(CONFIG_PARAVIRT_SPINLOCKS)+= paravirt-spinlocks.o
 obj-$(CONFIG_PARAVIRT_CLOCK)	+= pvclock.o
 
+obj-$(CONFIG_ARCH_RANDOM)	+= archrandom.o
+
 obj-$(CONFIG_PCSPKR_PLATFORM)	+= pcspeaker.o
 
 obj-$(CONFIG_X86_CHECK_BIOS_CORRUPTION) += check.o
diff --git a/arch/x86/kernel/archrandom.c b/arch/x86/kernel/archrandom.c
new file mode 100644
index 0000000..e8d2ffb
--- /dev/null
+++ b/arch/x86/kernel/archrandom.c
@@ -0,0 +1,51 @@
+/*
+ * This file is part of the Linux kernel.
+ *
+ * Copyright (c) 2014 Andy Lutomirski
+ * Authors: Andy Lutomirski <luto@amacapital.net>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ */
+
+#include <asm/archrandom.h>
+
+void arch_rng_init(void *ctx,
+		   void (*seed)(void *ctx, u32 data),
+		   int bits_per_source,
+		   const char *log_prefix)
+{
+	int i;
+	int rdseed_bits = 0, rdrand_bits = 0;
+	char buf[128] = "";
+	char *msgptr = buf;
+
+	for (i = 0; i < bits_per_source; i += 8 * sizeof(long)) {
+		unsigned long rv;
+
+		if (arch_get_random_seed_long(&rv))
+			rdseed_bits += 8 * sizeof(rv);
+		else if (arch_get_random_long(&rv))
+			rdrand_bits += 8 * sizeof(rv);
+		else
+			continue;	/* Don't waste time mixing. */
+
+		seed(ctx, (u32)rv);
+#if BITS_PER_LONG > 32
+		seed(ctx, (u32)(rv >> 32));
+#endif
+	}
+
+	if (rdseed_bits)
+		msgptr += sprintf(msgptr, ", %d bits from RDSEED", rdseed_bits);
+	if (rdrand_bits)
+		msgptr += sprintf(msgptr, ", %d bits from RDRAND", rdrand_bits);
+	if (buf[0])
+		pr_info("%s with %s\n", log_prefix, buf + 2);
+}
-- 
1.9.3

[PATCH v6 6/7] x86,random,kvm: Use KVM_GET_RNG_SEED in arch_get_rng_seed

[Andy Lutomirski]

This is a straightforward implementation: for each bit of internal
RNG state, request one bit from KVM_GET_RNG_SEED.  This is done even
if RDSEED/RDRAND worked, since KVM_GET_RNG_SEED is likely to provide
cryptographically secure output even if the CPU's RNG is weak or
compromised.

Acked-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Andy Lutomirski <luto@amacapital.net>
---
 arch/x86/Kconfig                 |  4 ++++
 arch/x86/include/asm/kvm_guest.h |  9 +++++++++
 arch/x86/kernel/archrandom.c     | 25 ++++++++++++++++++++++++-
 arch/x86/kernel/kvm.c            | 10 ++++++++++
 4 files changed, 47 insertions(+), 1 deletion(-)

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index d24887b..ad87278 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -594,6 +594,7 @@ config KVM_GUEST
 	bool "KVM Guest support (including kvmclock)"
 	depends on PARAVIRT
 	select PARAVIRT_CLOCK
+	select ARCH_RANDOM
 	default y
 	---help---
 	  This option enables various optimizations for running under the KVM
@@ -1508,6 +1509,9 @@ config ARCH_RANDOM
 	  If supported, this is a high bandwidth, cryptographically
 	  secure hardware random number generator.
 
+	  This also enables paravirt RNGs such as KVM's if the relevant
+	  PV guest support is enabled.
+
 config X86_SMAP
 	def_bool y
 	prompt "Supervisor Mode Access Prevention" if EXPERT
diff --git a/arch/x86/include/asm/kvm_guest.h b/arch/x86/include/asm/kvm_guest.h
index a92b176..8c4dbd5 100644
--- a/arch/x86/include/asm/kvm_guest.h
+++ b/arch/x86/include/asm/kvm_guest.h
@@ -3,4 +3,13 @@
 
 int kvm_setup_vsyscall_timeinfo(void);
 
+#if defined(CONFIG_KVM_GUEST) && defined(CONFIG_ARCH_RANDOM)
+extern bool kvm_get_rng_seed(u64 *rv);
+#else
+static inline bool kvm_get_rng_seed(u64 *rv)
+{
+	return false;
+}
+#endif
+
 #endif /* _ASM_X86_KVM_GUEST_H */
diff --git a/arch/x86/kernel/archrandom.c b/arch/x86/kernel/archrandom.c
index e8d2ffb..adbaa25 100644
--- a/arch/x86/kernel/archrandom.c
+++ b/arch/x86/kernel/archrandom.c
@@ -15,6 +15,7 @@
  */
 
 #include <asm/archrandom.h>
+#include <asm/kvm_guest.h>
 
 void arch_rng_init(void *ctx,
 		   void (*seed)(void *ctx, u32 data),
@@ -22,7 +23,7 @@ void arch_rng_init(void *ctx,
 		   const char *log_prefix)
 {
 	int i;
-	int rdseed_bits = 0, rdrand_bits = 0;
+	int rdseed_bits = 0, rdrand_bits = 0, kvm_bits = 0;
 	char buf[128] = "";
 	char *msgptr = buf;
 
@@ -42,10 +43,32 @@ void arch_rng_init(void *ctx,
 #endif
 	}
 
+	/*
+	 * Use KVM_GET_RNG_SEED regardless of whether the CPU RNG
+	 * worked, since it incorporates entropy unavailable to the CPU,
+	 * and we shouldn't trust the hardware RNG more than we need to.
+	 * We request enough bits for the entire internal RNG state,
+	 * because there's no good reason not to.
+	 */
+	for (i = 0; i < bits_per_source; i += 64) {
+		u64 rv;
+
+		if (kvm_get_rng_seed(&rv)) {
+			seed(ctx, (u32)rv);
+			seed(ctx, (u32)(rv >> 32));
+			kvm_bits += 8 * sizeof(rv);
+		} else {
+			break;	/* If it fails once, it will keep failing. */
+		}
+	}
+
 	if (rdseed_bits)
 		msgptr += sprintf(msgptr, ", %d bits from RDSEED", rdseed_bits);
 	if (rdrand_bits)
 		msgptr += sprintf(msgptr, ", %d bits from RDRAND", rdrand_bits);
+	if (kvm_bits)
+		msgptr += sprintf(msgptr, ", %d bits from KVM_GET_RNG_BITS",
+				  kvm_bits);
 	if (buf[0])
 		pr_info("%s with %s\n", log_prefix, buf + 2);
 }
diff --git a/arch/x86/kernel/kvm.c b/arch/x86/kernel/kvm.c
index 3dd8e2c..bd8783a 100644
--- a/arch/x86/kernel/kvm.c
+++ b/arch/x86/kernel/kvm.c
@@ -416,6 +416,16 @@ void kvm_disable_steal_time(void)
 	wrmsr(MSR_KVM_STEAL_TIME, 0, 0);
 }
 
+bool kvm_get_rng_seed(u64 *v)
+{
+	/*
+	 * Allow migration from a hypervisor with the GET_RNG_SEED
+	 * feature to a hypervisor without it.
+	 */
+	return (kvm_para_has_feature(KVM_FEATURE_GET_RNG_SEED) &&
+		rdmsrl_safe(MSR_KVM_GET_RNG_SEED, v) == 0);
+}
+
 #ifdef CONFIG_SMP
 static void __init kvm_smp_prepare_boot_cpu(void)
 {
-- 
1.9.3

[PATCH v6 6/7] x86,random,kvm: Use KVM_GET_RNG_SEED in arch_rng_init

[Andy Lutomirski]

This is a straightforward implementation: for each bit of internal
RNG state, request one bit from KVM_GET_RNG_SEED.  This is done even
if RDSEED/RDRAND worked, since KVM_GET_RNG_SEED is likely to provide
cryptographically secure output even if the CPU's RNG is weak or
compromised.

Acked-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Andy Lutomirski <luto@amacapital.net>
---
 arch/x86/Kconfig                 |  4 ++++
 arch/x86/include/asm/kvm_guest.h |  9 +++++++++
 arch/x86/kernel/archrandom.c     | 25 ++++++++++++++++++++++++-
 arch/x86/kernel/kvm.c            | 10 ++++++++++
 4 files changed, 47 insertions(+), 1 deletion(-)

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index d24887b..ad87278 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -594,6 +594,7 @@ config KVM_GUEST
 	bool "KVM Guest support (including kvmclock)"
 	depends on PARAVIRT
 	select PARAVIRT_CLOCK
+	select ARCH_RANDOM
 	default y
 	---help---
 	  This option enables various optimizations for running under the KVM
@@ -1508,6 +1509,9 @@ config ARCH_RANDOM
 	  If supported, this is a high bandwidth, cryptographically
 	  secure hardware random number generator.
 
+	  This also enables paravirt RNGs such as KVM's if the relevant
+	  PV guest support is enabled.
+
 config X86_SMAP
 	def_bool y
 	prompt "Supervisor Mode Access Prevention" if EXPERT
diff --git a/arch/x86/include/asm/kvm_guest.h b/arch/x86/include/asm/kvm_guest.h
index a92b176..8c4dbd5 100644
--- a/arch/x86/include/asm/kvm_guest.h
+++ b/arch/x86/include/asm/kvm_guest.h
@@ -3,4 +3,13 @@
 
 int kvm_setup_vsyscall_timeinfo(void);
 
+#if defined(CONFIG_KVM_GUEST) && defined(CONFIG_ARCH_RANDOM)
+extern bool kvm_get_rng_seed(u64 *rv);
+#else
+static inline bool kvm_get_rng_seed(u64 *rv)
+{
+	return false;
+}
+#endif
+
 #endif /* _ASM_X86_KVM_GUEST_H */
diff --git a/arch/x86/kernel/archrandom.c b/arch/x86/kernel/archrandom.c
index e8d2ffb..adbaa25 100644
--- a/arch/x86/kernel/archrandom.c
+++ b/arch/x86/kernel/archrandom.c
@@ -15,6 +15,7 @@
  */
 
 #include <asm/archrandom.h>
+#include <asm/kvm_guest.h>
 
 void arch_rng_init(void *ctx,
 		   void (*seed)(void *ctx, u32 data),
@@ -22,7 +23,7 @@ void arch_rng_init(void *ctx,
 		   const char *log_prefix)
 {
 	int i;
-	int rdseed_bits = 0, rdrand_bits = 0;
+	int rdseed_bits = 0, rdrand_bits = 0, kvm_bits = 0;
 	char buf[128] = "";
 	char *msgptr = buf;
 
@@ -42,10 +43,32 @@ void arch_rng_init(void *ctx,
 #endif
 	}
 
+	/*
+	 * Use KVM_GET_RNG_SEED regardless of whether the CPU RNG
+	 * worked, since it incorporates entropy unavailable to the CPU,
+	 * and we shouldn't trust the hardware RNG more than we need to.
+	 * We request enough bits for the entire internal RNG state,
+	 * because there's no good reason not to.
+	 */
+	for (i = 0; i < bits_per_source; i += 64) {
+		u64 rv;
+
+		if (kvm_get_rng_seed(&rv)) {
+			seed(ctx, (u32)rv);
+			seed(ctx, (u32)(rv >> 32));
+			kvm_bits += 8 * sizeof(rv);
+		} else {
+			break;	/* If it fails once, it will keep failing. */
+		}
+	}
+
 	if (rdseed_bits)
 		msgptr += sprintf(msgptr, ", %d bits from RDSEED", rdseed_bits);
 	if (rdrand_bits)
 		msgptr += sprintf(msgptr, ", %d bits from RDRAND", rdrand_bits);
+	if (kvm_bits)
+		msgptr += sprintf(msgptr, ", %d bits from KVM_GET_RNG_BITS",
+				  kvm_bits);
 	if (buf[0])
 		pr_info("%s with %s\n", log_prefix, buf + 2);
 }
diff --git a/arch/x86/kernel/kvm.c b/arch/x86/kernel/kvm.c
index 3dd8e2c..bd8783a 100644
--- a/arch/x86/kernel/kvm.c
+++ b/arch/x86/kernel/kvm.c
@@ -416,6 +416,16 @@ void kvm_disable_steal_time(void)
 	wrmsr(MSR_KVM_STEAL_TIME, 0, 0);
 }
 
+bool kvm_get_rng_seed(u64 *v)
+{
+	/*
+	 * Allow migration from a hypervisor with the GET_RNG_SEED
+	 * feature to a hypervisor without it.
+	 */
+	return (kvm_para_has_feature(KVM_FEATURE_GET_RNG_SEED) &&
+		rdmsrl_safe(MSR_KVM_GET_RNG_SEED, v) == 0);
+}
+
 #ifdef CONFIG_SMP
 static void __init kvm_smp_prepare_boot_cpu(void)
 {
-- 
1.9.3

[PATCH v6 7/7] x86,kaslr: Use MSR_KVM_GET_RNG_SEED for KASLR if available

[Andy Lutomirski]

It's considerably better than any of the alternatives on KVM.

Rather than reinventing all of the cpu feature query code, this fixes
native_cpuid to work in PIC objects.

I haven't combined it with boot/cpuflags.c's cpuid implementation:
including asm/processor.h from boot/cpuflags.c results in a flood of
unrelated errors, and fixing it might be messy.

Reviewed-by: Kees Cook <keescook@chromium.org>
Acked-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Andy Lutomirski <luto@amacapital.net>
---
 arch/x86/boot/compressed/aslr.c  | 27 +++++++++++++++++++++++++++
 arch/x86/include/asm/processor.h | 21 ++++++++++++++++++---
 2 files changed, 45 insertions(+), 3 deletions(-)

diff --git a/arch/x86/boot/compressed/aslr.c b/arch/x86/boot/compressed/aslr.c
index fc6091a..8583f0e 100644
--- a/arch/x86/boot/compressed/aslr.c
+++ b/arch/x86/boot/compressed/aslr.c
@@ -5,6 +5,8 @@
 #include <asm/archrandom.h>
 #include <asm/e820.h>
 
+#include <uapi/asm/kvm_para.h>
+
 #include <generated/compile.h>
 #include <linux/module.h>
 #include <linux/uts.h>
@@ -15,6 +17,22 @@
 static const char build_str[] = UTS_RELEASE " (" LINUX_COMPILE_BY "@"
 		LINUX_COMPILE_HOST ") (" LINUX_COMPILER ") " UTS_VERSION;
 
+static bool kvm_para_has_feature(unsigned int feature)
+{
+	u32 kvm_base;
+	u32 features;
+
+	if (!has_cpuflag(X86_FEATURE_HYPERVISOR))
+		return false;
+
+	kvm_base = hypervisor_cpuid_base("KVMKVMKVM\0\0\0", KVM_CPUID_FEATURES);
+	if (!kvm_base)
+		return false;
+
+	features = cpuid_eax(kvm_base | KVM_CPUID_FEATURES);
+	return features & (1UL << feature);
+}
+
 #define I8254_PORT_CONTROL	0x43
 #define I8254_PORT_COUNTER0	0x40
 #define I8254_CMD_READBACK	0xC0
@@ -81,6 +99,15 @@ static unsigned long get_random_long(void)
 		}
 	}
 
+	if (kvm_para_has_feature(KVM_FEATURE_GET_RNG_SEED)) {
+		u64 seed;
+
+		debug_putstr(" MSR_KVM_GET_RNG_SEED");
+		rdmsrl(MSR_KVM_GET_RNG_SEED, seed);
+		random ^= (unsigned long)seed;
+		use_i8254 = false;
+	}
+
 	if (has_cpuflag(X86_FEATURE_TSC)) {
 		debug_putstr(" RDTSC");
 		rdtscll(raw);
diff --git a/arch/x86/include/asm/processor.h b/arch/x86/include/asm/processor.h
index a4ea023..6096f3c 100644
--- a/arch/x86/include/asm/processor.h
+++ b/arch/x86/include/asm/processor.h
@@ -189,10 +189,25 @@ static inline int have_cpuid_p(void)
 static inline void native_cpuid(unsigned int *eax, unsigned int *ebx,
 				unsigned int *ecx, unsigned int *edx)
 {
-	/* ecx is often an input as well as an output. */
-	asm volatile("cpuid"
+	/*
+	 * This function can be used from the boot code, so it needs
+	 * to avoid using EBX in constraints in PIC mode.
+	 *
+	 * ecx is often an input as well as an output.
+	 */
+	asm volatile(".ifnc %%ebx,%1 ; .ifnc %%rbx,%1           \n\t"
+		     "movl  %%ebx,%1                            \n\t"
+		     ".endif ; .endif                           \n\t"
+		     "cpuid					\n\t"
+		     ".ifnc %%ebx,%1 ; .ifnc %%rbx,%1           \n\t"
+		     "xchgl %%ebx,%1                            \n\t"
+		     ".endif ; .endif"
 	    : "=a" (*eax),
-	      "=b" (*ebx),
+#if defined(__i386__) && defined(__PIC__)
+	      "=r" (*ebx),	/* gcc won't let us use ebx */
+#else
+	      "=b" (*ebx),	/* ebx is okay */
+#endif
 	      "=c" (*ecx),
 	      "=d" (*edx)
 	    : "0" (*eax), "2" (*ecx)
-- 
1.9.3

Re: [PATCH v6 2/7] random, timekeeping: Collect timekeeping entropy in the timekeeping code

[Andy Lutomirski]

On Wed, Aug 13, 2014 at 10:43 PM, Andy Lutomirski <luto@amacapital.net> wrote:
> Currently, init_std_data calls ktime_get_real().  This imposes
> awkward constraints on when init_std_data can be called, and
> init_std_data is unlikely to collect the full unpredictable data
> available to the timekeeping code, especially after resume.
>
> Remove this code from random.c and add the appropriate
> add_device_randomness calls to timekeeping.c instead.

*sigh* this is buggy:


> +       add_device_randomness(tk, sizeof(tk));

sizeof(*tk)

> +       add_device_randomness(tk, sizeof(tk));

ditto.

I'll fix this for v7, but I'll wait awhile for other comments to reduce spam.

--Andy

Re: [PATCH v6 2/7] random, timekeeping: Collect timekeeping entropy in the timekeeping code

[John Stultz]

On Thu, Aug 14, 2014 at 12:43 AM, Andy Lutomirski <luto@amacapital.net> wrote:
> Currently, init_std_data calls ktime_get_real().  This imposes
> awkward constraints on when init_std_data can be called, and
> init_std_data is unlikely to collect the full unpredictable data
> available to the timekeeping code, especially after resume.
>
> Remove this code from random.c and add the appropriate
> add_device_randomness calls to timekeeping.c instead.
>
> Cc: John Stultz <john.stultz@linaro.org>
> Signed-off-by: Andy Lutomirski <luto@amacapital.net>
> ---
>  drivers/char/random.c     |  2 --
>  kernel/time/timekeeping.c | 11 +++++++++++
>  2 files changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/char/random.c b/drivers/char/random.c
> index 7673e60..8dc3e3a 100644
> --- a/drivers/char/random.c
> +++ b/drivers/char/random.c
> @@ -1263,12 +1263,10 @@ static void seed_entropy_store(void *ctx, u32 data)
>  static void init_std_data(struct entropy_store *r)
>  {
>         int i;
> -       ktime_t now = ktime_get_real();
>         unsigned long rv;
>         char log_prefix[128];
>
>         r->last_pulled = jiffies;
> -       mix_pool_bytes(r, &now, sizeof(now), NULL);
>         for (i = r->poolinfo->poolbytes; i > 0; i -= sizeof(rv)) {
>                 rv = random_get_entropy();
>                 mix_pool_bytes(r, &rv, sizeof(rv), NULL);
> diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
> index 32d8d6a..9609db9 100644
> --- a/kernel/time/timekeeping.c
> +++ b/kernel/time/timekeeping.c
> @@ -23,6 +23,7 @@
>  #include <linux/stop_machine.h>
>  #include <linux/pvclock_gtod.h>
>  #include <linux/compiler.h>
> +#include <linux/random.h>
>
>  #include "tick-internal.h"
>  #include "ntp_internal.h"
> @@ -835,6 +836,9 @@ void __init timekeeping_init(void)
>         memcpy(&shadow_timekeeper, &timekeeper, sizeof(timekeeper));
>
>         write_seqcount_end(&timekeeper_seq);
> +
> +       add_device_randomness(tk, sizeof(tk));
> +


So I can't (and really don't want to) vouch for the correctness side
of this. The initial idea of using the structure instead of reading
the time worried me a bit, but we have already read the clocksource
and stored it in cycle_last so there's a wee bit more then just the
RTC time and a bunch of zeros in the timekeeper structure.

Though on some systems the read_persistent_clock call can't access the
RTC at timekeeping_init, so I'm not sure we're really getting that
much more then the cycle_last clocksource value here. Probably should
add something like this to the RTC hctosys logic.

thanks
-john