| Message ID | 20170606005108.5646-4-Jason@zx2c4.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Delegated to: | Herbert Xu |
| Headers | show |
Hi Jason,
[auto build test WARNING on linus/master]
[also build test WARNING on v4.12-rc4]
[cannot apply to next-20170607]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Jason-A-Donenfeld/Unseeded-In-Kernel-Randomness-Fixes/20170606-171249
config: sh-ul2_defconfig (attached as .config)
compiler: sh4-linux-gnu-gcc (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
wget https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=sh
Note: it may well be a FALSE warning. FWIW you are at least aware of it now.
http://gcc.gnu.org/wiki/Better_Uninitialized_Warnings
All warnings (new ones prefixed by >>):
In file included from include/linux/spinlock.h:53:0,
from include/linux/rcupdate.h:38,
from include/linux/rculist.h:10,
from include/linux/pid.h:4,
from include/linux/sched.h:13,
from include/linux/utsname.h:5,
from drivers//char/random.c:238:
drivers//char/random.c: In function 'get_random_u64':
>> include/linux/irqflags.h:69:3: warning: 'flags' may be used uninitialized in this function [-Wmaybe-uninitialized]
arch_local_irq_restore(flags); \
^~~~~~~~~~~~~~~~~~~~~~
drivers//char/random.c:2063:16: note: 'flags' was declared here
unsigned long flags;
^~~~~
In file included from include/linux/spinlock.h:53:0,
from include/linux/rcupdate.h:38,
from include/linux/rculist.h:10,
from include/linux/pid.h:4,
from include/linux/sched.h:13,
from include/linux/utsname.h:5,
from drivers//char/random.c:238:
drivers//char/random.c: In function 'get_random_u32':
>> include/linux/irqflags.h:69:3: warning: 'flags' may be used uninitialized in this function [-Wmaybe-uninitialized]
arch_local_irq_restore(flags); \
^~~~~~~~~~~~~~~~~~~~~~
drivers//char/random.c:2095:16: note: 'flags' was declared here
unsigned long flags;
^~~~~
--
In file included from include/linux/spinlock.h:53:0,
from include/linux/rcupdate.h:38,
from include/linux/rculist.h:10,
from include/linux/pid.h:4,
from include/linux/sched.h:13,
from include/linux/utsname.h:5,
from drivers/char/random.c:238:
drivers/char/random.c: In function 'get_random_u64':
>> include/linux/irqflags.h:69:3: warning: 'flags' may be used uninitialized in this function [-Wmaybe-uninitialized]
arch_local_irq_restore(flags); \
^~~~~~~~~~~~~~~~~~~~~~
drivers/char/random.c:2063:16: note: 'flags' was declared here
unsigned long flags;
^~~~~
In file included from include/linux/spinlock.h:53:0,
from include/linux/rcupdate.h:38,
from include/linux/rculist.h:10,
from include/linux/pid.h:4,
from include/linux/sched.h:13,
from include/linux/utsname.h:5,
from drivers/char/random.c:238:
drivers/char/random.c: In function 'get_random_u32':
>> include/linux/irqflags.h:69:3: warning: 'flags' may be used uninitialized in this function [-Wmaybe-uninitialized]
arch_local_irq_restore(flags); \
^~~~~~~~~~~~~~~~~~~~~~
drivers/char/random.c:2095:16: note: 'flags' was declared here
unsigned long flags;
^~~~~
vim +/flags +69 include/linux/irqflags.h
81d68a96a Steven Rostedt 2008-05-12 53 # define start_critical_timings() do { } while (0)
81d68a96a Steven Rostedt 2008-05-12 54 #endif
81d68a96a Steven Rostedt 2008-05-12 55
df9ee2927 David Howells 2010-10-07 56 /*
df9ee2927 David Howells 2010-10-07 57 * Wrap the arch provided IRQ routines to provide appropriate checks.
df9ee2927 David Howells 2010-10-07 58 */
df9ee2927 David Howells 2010-10-07 59 #define raw_local_irq_disable() arch_local_irq_disable()
df9ee2927 David Howells 2010-10-07 60 #define raw_local_irq_enable() arch_local_irq_enable()
df9ee2927 David Howells 2010-10-07 61 #define raw_local_irq_save(flags) \
df9ee2927 David Howells 2010-10-07 62 do { \
df9ee2927 David Howells 2010-10-07 63 typecheck(unsigned long, flags); \
df9ee2927 David Howells 2010-10-07 64 flags = arch_local_irq_save(); \
df9ee2927 David Howells 2010-10-07 65 } while (0)
df9ee2927 David Howells 2010-10-07 66 #define raw_local_irq_restore(flags) \
df9ee2927 David Howells 2010-10-07 67 do { \
df9ee2927 David Howells 2010-10-07 68 typecheck(unsigned long, flags); \
df9ee2927 David Howells 2010-10-07 @69 arch_local_irq_restore(flags); \
df9ee2927 David Howells 2010-10-07 70 } while (0)
df9ee2927 David Howells 2010-10-07 71 #define raw_local_save_flags(flags) \
df9ee2927 David Howells 2010-10-07 72 do { \
df9ee2927 David Howells 2010-10-07 73 typecheck(unsigned long, flags); \
df9ee2927 David Howells 2010-10-07 74 flags = arch_local_save_flags(); \
df9ee2927 David Howells 2010-10-07 75 } while (0)
df9ee2927 David Howells 2010-10-07 76 #define raw_irqs_disabled_flags(flags) \
df9ee2927 David Howells 2010-10-07 77 ({ \
:::::: The code at line 69 was first introduced by commit
:::::: df9ee29270c11dba7d0fe0b83ce47a4d8e8d2101 Fix IRQ flag handling naming
:::::: TO: David Howells <dhowells@redhat.com>
:::::: CC: David Howells <dhowells@redhat.com>
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
diff --git a/drivers/char/random.c b/drivers/char/random.c index 035a5d7c06bd..c328e9b11f1f 100644 --- a/drivers/char/random.c +++ b/drivers/char/random.c @@ -762,6 +762,8 @@ static DECLARE_WAIT_QUEUE_HEAD(crng_init_wait); static struct crng_state **crng_node_pool __read_mostly; #endif +static void invalidate_batched_entropy(void); + static void crng_initialize(struct crng_state *crng) { int i; @@ -800,6 +802,7 @@ static int crng_fast_load(const char *cp, size_t len) } if (crng_init_cnt >= CRNG_INIT_CNT_THRESH) { crng_init = 1; + invalidate_batched_entropy(); wake_up_interruptible(&crng_init_wait); pr_notice("random: fast init done\n"); } @@ -837,6 +840,7 @@ static void crng_reseed(struct crng_state *crng, struct entropy_store *r) crng->init_time = jiffies; if (crng == &primary_crng && crng_init < 2) { crng_init = 2; + invalidate_batched_entropy(); process_random_ready_list(); wake_up_interruptible(&crng_init_wait); pr_notice("random: crng init done\n"); @@ -2037,6 +2041,7 @@ struct batched_entropy { }; unsigned int position; }; +static rwlock_t batched_entropy_reset_lock = __RW_LOCK_UNLOCKED(batched_entropy_reset_lock); /* * Get a random word for internal kernel use only. The quality of the random @@ -2050,6 +2055,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; struct batched_entropy *batch; #if BITS_PER_LONG == 64 @@ -2062,11 +2069,15 @@ u64 get_random_u64(void) #endif batch = &get_cpu_var(batched_entropy_u64); + if (use_lock) + read_lock_irqsave(&batched_entropy_reset_lock, flags); if (batch->position % ARRAY_SIZE(batch->entropy_u64) == 0) { extract_crng((u8 *)batch->entropy_u64); batch->position = 0; } ret = batch->entropy_u64[batch->position++]; + if (use_lock) + read_unlock_irqrestore(&batched_entropy_reset_lock, flags); put_cpu_var(batched_entropy_u64); return ret; } @@ -2076,22 +2087,45 @@ 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; struct batched_entropy *batch; if (arch_get_random_int(&ret)) return ret; batch = &get_cpu_var(batched_entropy_u32); + if (use_lock) + read_lock_irqsave(&batched_entropy_reset_lock, flags); if (batch->position % ARRAY_SIZE(batch->entropy_u32) == 0) { extract_crng((u8 *)batch->entropy_u32); batch->position = 0; } ret = batch->entropy_u32[batch->position++]; + if (use_lock) + read_unlock_irqrestore(&batched_entropy_reset_lock, flags); put_cpu_var(batched_entropy_u32); return ret; } EXPORT_SYMBOL(get_random_u32); +/* It's important to invalidate all potential batched entropy that might + * be stored before the crng is initialized, which we can do lazily by + * simply resetting the counter to zero so that it's re-extracted on the + * next usage. */ +static void invalidate_batched_entropy(void) +{ + int cpu; + unsigned long flags; + + write_lock_irqsave(&batched_entropy_reset_lock, flags); + for_each_possible_cpu (cpu) { + per_cpu_ptr(&batched_entropy_u32, cpu)->position = 0; + per_cpu_ptr(&batched_entropy_u64, cpu)->position = 0; + } + write_unlock_irqrestore(&batched_entropy_reset_lock, flags); +} + /** * randomize_page - Generate a random, page aligned address * @start: The smallest acceptable address the caller will take.
It's possible that get_random_{u32,u64} is used before the crng has initialized, in which case, its output might not be cryptographically secure. For this problem, directly, this patch set is introducing the *_wait variety of functions, but even with that, there's a subtle issue: what happens to our batched entropy that was generated before initialization. Prior to this commit, it'd stick around, supplying bad numbers. After this commit, we force the entropy to be re-extracted after each phase of the crng has initialized. In order to avoid a race condition with the position counter, we introduce a simple rwlock for this invalidation. Since it's only during this awkward transition period, after things are all set up, we stop using it, so that it doesn't have an impact on performance. Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com> --- drivers/char/random.c | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+)