All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Theodore Ts'o" <tytso@mit.edu>
To: Sebastian Andrzej Siewior <sebastian@breakpoint.cc>
Cc: "Luck, Tony" <tony.luck@intel.com>,
	Andi Kleen <andi@firstfloor.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	Andi Kleen <ak@linux.intel.com>,
	tglx@linutronix.de, Herbert Xu <herbert@gondor.apana.org.au>,
	Russell King <rmk+kernel@arm.linux.org.uk>,
	Arnd Bergmann <arnd@arndb.de>, Felipe Balbi <balbi@ti.com>,
	shawn.guo@linaro.org, grant.likely@linaro.org
Subject: Re: [PATCH 01/11] random: don't feed stack data into pool when interrupt regs NULL
Date: Mon, 7 Apr 2014 00:01:37 -0400	[thread overview]
Message-ID: <20140407040137.GA29755@thunk.org> (raw)
In-Reply-To: <20140404165447.GA28040@breakpoint.cc>

On Fri, Apr 04, 2014 at 06:54:47PM +0200, Sebastian Andrzej Siewior wrote:
> 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.

Yes, ARM sucks for not implement random_get_entropy() on all
platforms.  Film at 11.  I'm told that Cortex ARM systms are supposed
to have a cycle counter.  I'm not sure why it hasn't been wired up,
but it really should be.

> [0] = irq (8 bit)
> [1] = jiffies
> [2] = a constant unless regs is available and
> [3] = 0

Actually regs should be available nearly all of the time.  So the
situation isn't quite as dire as you describe, but agreed, it is
pretty bad.

Still, it would be nice if random_get_entropy() could be wired up on
as many platforms as possible.  

> +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);

This is lacking in subtly, and while I'm sympathetic with your
frustration, and unfortunately, there are a huge number of CPU's/SOC's
that don't implement a cycle counter or some other kind of cheap, high
resolution counter.  I'm not against putting in a warning printk, but
issuing a WARN_ON(1) would be really be too obnoxious.  Maybe
something like, "you're running on a crap CPU architecture and so
/dev/random may very well be insecure", but I think that's about as
blunt as we can really afford to be at this point.

	     	    	   	     - Ted

P.S.  Maybe if Intel started a marketing campaign on G+ explaining why
ChromeOS on Intel machines is much more secure than ChromeOS on ARM,
we could finally get some action out of the !@#!?! ARM chip
manufacturers.  :-/

  reply	other threads:[~2014-04-07  4:02 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-09-30 20:29 Various static checker fixes Andi Kleen
2013-09-30 20:29 ` [PATCH 01/11] random: don't feed stack data into pool when interrupt regs NULL Andi Kleen
2013-09-30 20:51   ` Luck, Tony
2013-10-01 12:44     ` Theodore Ts'o
2014-04-04 16:54       ` Sebastian Andrzej Siewior
2014-04-07  4:01         ` Theodore Ts'o [this message]
2014-04-07 19:30           ` Sebastian Andrzej Siewior
2014-04-07 23:26             ` Theodore Ts'o
2014-04-09 18:14             ` rkuo
2013-09-30 20:29 ` [PATCH 02/11] Disable initialized_var for clang Andi Kleen
2013-09-30 20:29 ` [PATCH 03/11] posix-timers: Initialize timer value to 0 for invalid timers Andi Kleen
2013-09-30 20:29 ` [PATCH 04/11] block: Return error code of elevator init function Andi Kleen
2013-10-01 12:25   ` Jeff Moyer
2013-09-30 20:29 ` [PATCH 05/11] seq_file: Handle ->next error in seq_read Andi Kleen
2013-09-30 20:29 ` [PATCH 06/11] sysctl: remove unnecessary variable initialization Andi Kleen
2013-09-30 20:29 ` [PATCH 07/11] igb: Avoid uninitialized advertised variable in eee_set_cur Andi Kleen
2013-10-01 15:00   ` Wyborny, Carolyn
2013-10-01 23:10   ` Jeff Kirsher
2013-10-02 20:33   ` David Miller
2013-09-30 20:29 ` [PATCH 08/11] ext4: Fix end of group handling in ext4_mb_init_cache Andi Kleen
2013-10-01 12:45   ` Theodore Ts'o
2013-10-01 14:20     ` Andi Kleen
2013-09-30 20:29 ` [PATCH 09/11] epoll: Remove unnecessary error path Andi Kleen
2013-09-30 20:59   ` Eric Wong
2013-09-30 21:01     ` Andi Kleen
2013-09-30 20:29 ` [PATCH 10/11] tcp: Always set options to 0 before calling tcp_established_options Andi Kleen
2013-10-02 20:33   ` David Miller
2013-09-30 20:29 ` [PATCH 11/11] perf: Avoid uninitialized sample type reference in __perf_event__output_id_sample Andi Kleen
2013-10-02  8:57   ` Peter Zijlstra
2013-10-02 17:25     ` Andi Kleen
2013-10-02 17:36       ` Peter Zijlstra
2013-10-03  6:42         ` Ingo Molnar
2013-10-03 18:16           ` Andi Kleen
2013-10-04  6:24             ` Ingo Molnar

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20140407040137.GA29755@thunk.org \
    --to=tytso@mit.edu \
    --cc=ak@linux.intel.com \
    --cc=andi@firstfloor.org \
    --cc=arnd@arndb.de \
    --cc=balbi@ti.com \
    --cc=grant.likely@linaro.org \
    --cc=herbert@gondor.apana.org.au \
    --cc=linux-kernel@vger.kernel.org \
    --cc=rmk+kernel@arm.linux.org.uk \
    --cc=sebastian@breakpoint.cc \
    --cc=shawn.guo@linaro.org \
    --cc=tglx@linutronix.de \
    --cc=tony.luck@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.