[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Fri, 4 Apr 2014 18:54:47 +0200
From: Sebastian Andrzej Siewior <sebastian@...akpoint.cc>
To: Theodore Ts'o <tytso@....edu>, "Luck, Tony" <tony.luck@...el.com>,
Andi Kleen <andi@...stfloor.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
Andi Kleen <ak@...ux.intel.com>, tglx@...utronix.de,
Herbert Xu <herbert@...dor.apana.org.au>,
Russell King <rmk+kernel@....linux.org.uk>,
Arnd Bergmann <arnd@...db.de>, Felipe Balbi <balbi@...com>,
shawn.guo@...aro.org
Subject: Re: [PATCH 01/11] random: don't feed stack data into pool when
interrupt regs NULL
On 2013-10-01 08:44:24 [-0400], Theodore Ts'o wrote:
> The changes queued for the next merge window in the random tree solve
> this problem slightly differently:
>
> ...
> input[0] = cycles ^ j_high ^ irq;
> input[1] = now ^ c_high;
> ip = regs ? instruction_pointer(regs) : _RET_IP_;
> input[2] = ip;
> input[3] = ip >> 32;
>
> fast_mix(fast_pool, input);
> ...
>
> (Note the lack of nbytes parameter in fast_mix --- there are some
> optimizations so that we mix in the changes by 32-bit words, instead
> of bytes, and the number of 32-bit words is fixed to 4, since it's the
> only way fast_mix is called).
>
> _RET_IP_ isn't that much better than 0, but it's at least kernel
> specific, and I figured it was better to shut up bogus warnings, as
> opposed to trying to depend on stack garbage (which would likely be
> kernel specific anyway).
That ip pointer was earlier optional. Now with _RET_IP_ it is a
constant since there is _one_ caller. Plus on 32bit the upper bits are
always zero. It probably didn't get worse because the four bytes on
stack were mostlikly constant as well. [2] is constant if _RET_IP_ is
used. The IP is kind of random. The lower bits are mostlikely 0 due to
32bit alignment (not on x86, okay).
Lets look further. c_high is != 0 only if cycles is larger than 4 bytes.
This is in most cases a long which makes 4 bytes on all 32bit arches.
This makes [1] the lower bytes of jiffies. And you can imagine how often
the upper 16bit change.
Which brings me to [0]. The irq number changes now and then and mostlikely
only the lower 8 bit. j_high is 0 on 32bit platforms. Even on 64bit
with HZ=250 the lower 32bit overflows every ~198 days unless I miscalculated.
Doesn't this make it a constant?
And finally cycles which is random_get_entropy(). On ARM (as previously on
MIPS) this returns 0. Well not always but for all platforms which do not
implement register_current_timer_delay() which makes a lot of them.
This makes
[0] = irq (8 bit)
[1] = jiffies
[2] = a constant unless regs is available and
[3] = 0
from looking at the code it reads like 16 random bytes are fed but now it
looks a little different.
May I kill this and save a few cycles in irq context?
Why don't you take a small amount of randomness and use that as a key and
feed into a block cipher in CTR mode instead of giving it to user right
away? This _can_ work in parallell and should provide *good* pseudo random
numbers on demand with a high performance.
But seriously. What about this:
>From 082dab5c482728a9ef695aa5b42217dcec8e3dd5 Mon Sep 17 00:00:00 2001
From: Sebastian Andrzej Siewior <bigeasy@...utronix.de>
Date: Fri, 4 Apr 2014 18:49:29 +0200
Subject: [PATCH] random: yell if random_get_entropy() returns a constant
A few architectures still return 0 (i.e. a constant) if
random_get_entropy() is invoked. Make them aware of this so they may fix
this.
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@...utronix.de>
---
drivers/char/random.c | 21 +++++++++++++++++++++
1 file changed, 21 insertions(+)
diff --git a/drivers/char/random.c b/drivers/char/random.c
index 429b75b..48775f8 100644
--- a/drivers/char/random.c
+++ b/drivers/char/random.c
@@ -241,6 +241,7 @@
#include <linux/major.h>
#include <linux/string.h>
#include <linux/fcntl.h>
+#include <linux/delay.h>
#include <linux/slab.h>
#include <linux/random.h>
#include <linux/poll.h>
@@ -1675,3 +1676,23 @@ randomize_range(unsigned long start, unsigned long end, unsigned long len)
return 0;
return PAGE_ALIGN(get_random_int() % range + start);
}
+
+static int check_random_get_entropy(void)
+{
+ cycles_t cyc1;
+ cycles_t cyc2;
+
+ cyc1 = random_get_entropy();
+ cyc2 = random_get_entropy();
+ if (cyc1 != cyc2)
+ return 0;
+ udelay(1);
+ cyc2 = random_get_entropy();
+
+ if (cyc1 != cyc2)
+ return 0;
+ pr_err("Error: random_get_entropy() does not return anything random\n");
+ WARN_ON(1);
+ return -EINVAL;
+}
+late_initcall(check_random_get_entropy);
--
1.9.1
> Cheers,
> - Ted
Sebastian
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists