All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Theodore Ts'o" <tytso@mit.edu>
To: Stephan Mueller <smueller@chronox.de>
Cc: LKML <linux-kernel@vger.kernel.org>, dave.taht@bufferbloat.net
Subject: Re: [PATCH] /dev/random: Insufficient of entropy on many architectures
Date: Tue, 10 Sep 2013 14:25:24 -0400	[thread overview]
Message-ID: <20130910182524.GE29237@thunk.org> (raw)
In-Reply-To: <2722901.IcH4JOB8ab@tauon>

On Tue, Sep 10, 2013 at 06:54:38PM +0200, Stephan Mueller wrote:
> 
> Why do you say that clocksource is heavyweight? Yes, there is a bit more 
> code than for get_cycles, but that is all just leading to usually an 
> equally small clock read code as get_cycles.

I've had past experiences where a clock source can be *very*
expensive.  One such example was the IBM x440 server, where
gettimeofday() ended up requiring a fetching the clock across the
"scalability cable" (which connected up to 3 other boxes, each box
containing 4 CPU sockets --- this was a big, nasty NUMA system), and
it was expensive enough that a financial firm that was trying to get a
hard timestamp for every single stock transaction had its performance
hit massively.  And this was something running in userspace.  Remember
what I said, this is being done on *every* *single* *interrupt*.

The bigger problem is that people will scream about the performance
overhead, and then patches to remove the /dev/random hooks from the
core irq code will start showing up on LKML.  This is originally how
the SA_SAMPLE_RANDOM sampling got removed from device drivers, and I'm
sensitive to the fact that not everyone on LKML cares as much about
security, and many people are near fanatical about performance.  So I
don't want this to be a CONFIG option or a run-time option (since
distributions will be tempted to turn it off to win the performance
benchmarking war).  Maybe not right now, when everyone is all worked
up about the NSA.  But in the long term, I don't want anyone to have
an excuse to have the entropy sampling removed due to performance
reasons.

> Moreover, until having your proposed real fix, wouldn't it make sense to 
> have an interim patch to ensure we have entropy on the mentioned 
> platforms? I think /dev/random is critical enough to warrant some cache 
> miss even per interrupt?

We are already mixing in the IP from the saved irq registers, on every
single interrupt, so we are mixing in some entropy.  We would get more
entropy if we had a good cycle counter to mix in, but it's not the
case that we're completely exposed right now on those platforms which
don't have get_cycles() implemented.  If the system running so lock
step that the location of the interrupts is utterly predictable, then
it's not clear that using a clock source is going to help you....

Also note that the clocksource is not necessarily be the best choice;
it may not be the most fine grained sampling that we have available
(that is certainly be true for MIPS).  So doing something hacky
doesn't absolve us from the need to example every single platform that
as a no-op get_cycles() function.  (Well, at least those platforms
that we think really are going to be running security sensitive
systems ---- does anyone think we really have production systems
running m68k?  Maybe, but....)

If we believed that /dev/random was actually returning numbers which
are exploitable, because of this, I might agree with the "we must do
SOMETHING" attitude.  But I don't believe this to be the case.  Also
note that we're talking about embedded platforms, where upgrade cycles
are measured in years --- if you're lucky.  There are probably home
routers still stuck on 2.6, at which point they will be far more
succeptible to the problems described at http://factorable.net.

So I don't think we need to rush.  I'd much rather make sure we get
this fixed right.

					- Ted

  reply	other threads:[~2013-09-10 18:25 UTC|newest]

Thread overview: 58+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-09-10 11:31 [PATCH] /dev/random: Insufficient of entropy on many architectures Stephan Mueller
2013-09-10 11:46 ` Geert Uytterhoeven
2013-09-10 15:04 ` Theodore Ts'o
2013-09-10 16:54   ` Stephan Mueller
2013-09-10 18:25     ` Theodore Ts'o [this message]
2013-09-10 19:15       ` Stephan Mueller
2013-10-10  6:50       ` Pavel Machek
2013-10-14 21:13         ` Theodore Ts'o
2013-09-10 20:48   ` Geert Uytterhoeven
2013-09-10 21:14     ` Theodore Ts'o
2013-09-11  6:49       ` Stephan Mueller
2013-09-12 11:59         ` Geert Uytterhoeven
2013-09-12 12:08           ` Stephan Mueller
2013-09-12 12:15             ` Geert Uytterhoeven
2013-09-12 12:35               ` Stephan Mueller
2013-09-12 12:47                 ` Geert Uytterhoeven
2013-09-12 12:57                   ` Stephan Mueller
2013-09-12 21:18               ` Jörn Engel
2013-09-13 11:33             ` Thorsten Glaser
2013-09-12 14:25           ` Theodore Ts'o
2013-09-10 19:38 ` John Stultz
2013-09-10 19:44   ` John Stultz
2013-09-10 19:47   ` Stephan Mueller
2013-09-10 20:35     ` John Stultz
2013-09-10 20:38   ` Theodore Ts'o
2013-09-10 20:46     ` John Stultz
2013-09-10 21:10       ` Theodore Ts'o
2013-09-10 22:08         ` John Stultz
2013-09-10 22:33           ` Theodore Ts'o
2013-09-11  0:31             ` John Stultz
2013-09-11  0:50               ` Theodore Ts'o
2013-09-11  1:14                 ` John Stultz
2013-09-12 20:46                 ` H. Peter Anvin
2013-09-12 21:07                 ` Jörn Engel
2013-09-12 23:31                   ` Theodore Ts'o
2013-09-12 23:35                     ` Jörn Engel
2013-09-13  0:00                       ` Jörn Engel
2013-09-16 15:40                     ` [PATCH,RFC] random: make fast_mix() honor its name Jörn Engel
2013-09-21 21:25                       ` Theodore Ts'o
2013-09-21 21:41                         ` Theodore Ts'o
2013-09-22  3:05                           ` Theodore Ts'o
2013-09-22 21:01                             ` Jörg-Volker Peetz
2013-09-22 21:27                               ` Theodore Ts'o
2013-09-22 20:53                                 ` Jörn Engel
2013-09-22 23:36                                   ` Theodore Ts'o
2013-09-23  0:16                                     ` Jörn Engel
2013-09-23  2:43                                       ` Theodore Ts'o
2013-09-23 15:02                                         ` Jörn Engel
2013-09-23  7:39                                 ` Jörg-Volker Peetz
2013-09-22 20:31                           ` Jörn Engel
2013-09-22 20:14                         ` Jörn Engel
2013-09-12 21:31           ` [PATCH] /dev/random: Insufficient of entropy on many architectures Jörn Engel
2013-09-13  5:36             ` Stephan Mueller
2013-09-13 11:54               ` Thorsten Glaser
2013-09-13 19:29                 ` Theodore Ts'o
2013-09-13 15:26               ` Jörn Engel
2013-09-13 18:59               ` Theodore Ts'o
2013-09-15 11:12                 ` Stephan Mueller

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=20130910182524.GE29237@thunk.org \
    --to=tytso@mit.edu \
    --cc=dave.taht@bufferbloat.net \
    --cc=linux-kernel@vger.kernel.org \
    --cc=smueller@chronox.de \
    /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.