All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Theodore Ts'o" <tytso@mit.edu>
To: George Spelvin <linux@horizon.com>
Cc: ahferroin7@gmail.com, andi@firstfloor.org, jepler@unpythonic.net,
	linux-kernel@vger.kernel.org, linux@rasmusvillemoes.dk
Subject: Re: Updated scalable urandom patchkit
Date: Mon, 12 Oct 2015 00:05:00 -0400	[thread overview]
Message-ID: <20151012040500.GD5341@thunk.org> (raw)
In-Reply-To: <20151012001601.18155.qmail@ns.horizon.com>

On Sun, Oct 11, 2015 at 08:16:01PM -0400, George Spelvin wrote:
> 
> I'm not thrilled with incrementing the pointer from i to len, but mixing
> at positions i+k to i+k+len.  The whole LFSR scheme relies on a regular
> pass structure.

That part I'm not worried about.  We still have a regular pass
structure --- since for each CPU, we are still iterating over the pool
in a regular fashion.

> How about this instead: drop the hashed offset, and instead let each
> writer do an atomic_add_return on the index, then iterate over the
> "reserved" slots.  Concurrent additions will at least do non-overlapping
> writes until the numer equals the pool size.

One atomic operation per byte that we're mixing in?  That's quite
expensive.

> Personally, I hate the input_rotate.  It's not that it's harmful, just
> that it doesn't do much good compared to the cost; for the number of cycles
> and context space required there are more efficient mixing operations.

The input_rotate is useful for the input pool, for things like
keyboard code so we can make sure they enter the pool at more points
than just the low bits of each word.  But for the output pools, it
really doesn't make any sense.  And we are getting to the point where
we may end up having different mixing algorithms for the nonblocking
pool, and in that case I have absolutely no trouble dropping the
input_rotate part of the mixing algorithm for the non-blocking pool.

> Or a small machine with a couple of concurrent /dev/urandom abusers.
> Remember, it's globally readable, so it has to be resistance to malicious
> abuse.

One of the other ways we could solve this is by hanging a struct off
the task structure, and if we detect that we have a /dev/urandom
abuser, we give that process its own urandom pool, so any pissing that
it does will be in its own pool.  (So to speak.)

Most processes don't actually use that much randomness, and I'm not
that worried about in-kernel users of the nonblocking pool.  Even with
the most exec-heavy workload, setting up a new exec image is
heavyweight enough that you're really not going to be contending on
the lock.  I also have trouble with someone spending $$$$ on a system
with 1K cpu cores and wasting all of their CPU power with running
shell scripts that fork and exec a lot.  :-)

The reality is that most processes don't use /dev/urandom or
getrandom(2) at all, and those that do, many of them only use it
sparingly.  So maybe the right answer is to do something simple which
takes care of the abusers.

> You can add rather than XOR, and we have atomic add primitives.

Atomic-add primitives aren't portable either.  The representation
isn't guaranteed to be 32-bits, and some platforms an atomic int is
only 24-bits wide (the top 8 bits being used for locking purposes).

> There are several possible solutions that don't need separate pools
> (including separate add-back pools, with a shared seeded pool that
> is never touched by add-back), so I don't think it's necessary to
> give up yet.

Hmm, maybe.  I'm a bit worried about the amount of complexity that
this entails, and the reality is that the urandom pool or pools don't
provide anything other than cryptogaphic randomness.

At this point, I wonder if it might not be simpler to restrict the
current nonblocking pool to kernel users, and for userspace users, the
first time a process reads from /dev/urandom or calls getrandom(2), we
create for them a ChaCha20 CRNG, which hangs off of the task
structure.  This would require about 72 bytes of state per process,
but normally very few processes are reading from /dev/urandom or
calling getrandom(2) from userspace.

The CRNG would be initialized from the non-blocking pool, and is
reseeded after, say, 2**24 cranks or five minutes.  It's essentially
an OpenBSD-style arc4random in the kernel.  (Arguably the right answer
is to put arc4random in libc, where it can automatically handle
forks/clones/pthread automatically, but it seems pretty clear *that*
train has left a long time ago.)

I have a feeling this may be less code and complexity, and it nicely
handles the case where we have a /dev/urandom abuser who feels that
they want to call /dev/urandom in a tight loop, even on a 4 socket
Xeon system.  :-)

							- Ted

  reply	other threads:[~2015-10-12  4:05 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-10-10 18:45 Updated scalable urandom patchkit George Spelvin
2015-10-11  2:31 ` Theodore Ts'o
2015-10-11  2:53   ` Theodore Ts'o
2015-10-11  4:35   ` George Spelvin
2015-10-11 22:25     ` Theodore Ts'o
2015-10-12  0:16       ` George Spelvin
2015-10-12  4:05         ` Theodore Ts'o [this message]
2015-10-12  7:49           ` George Spelvin
2015-10-12 13:54             ` Theodore Ts'o
2015-10-12 20:30               ` George Spelvin
2015-10-12 20:34                 ` George Spelvin
2015-10-13  2:46                 ` Theodore Ts'o
2015-10-13  3:50                   ` Raymond Jennings
2015-10-13  7:50                     ` George Spelvin
2015-10-13  6:24                   ` George Spelvin
2015-10-13 16:20                   ` Andi Kleen
2015-10-13 21:10                     ` George Spelvin
2015-10-14  2:15                       ` Andi Kleen
2015-10-16  5:28                         ` [RFC PATCH 0/4] Alternate sclable urandom patchset George Spelvin
2015-10-16  5:29                           ` [RFC PATCH 1/4] random: Reduce stack usage in _xfer_secondary_pool George Spelvin
2015-10-16  5:30                           ` [RFC PATCH 2/4] random: Remove two unused arguments from extract_entropy() George Spelvin
2015-10-16  5:33                           ` [RFC PATCH 3/4] random: Only do mixback once per read George Spelvin
2015-10-16  6:12                             ` kbuild test robot
2015-10-16  8:11                               ` George Spelvin
2015-10-16  6:23                             ` kbuild test robot
2015-10-16  5:34                           ` [RFC PATCH 4/4] random: Make non-blocking mixback non-blocking George Spelvin
2015-10-21  8:27                       ` Updated scalable urandom patchkit George Spelvin
2015-10-21 11:47                         ` Andi Kleen
2015-10-21 18:10                           ` George Spelvin
  -- strict thread matches above, loose matches on Subject: below --
2015-09-24 17:19 Andi Kleen

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=20151012040500.GD5341@thunk.org \
    --to=tytso@mit.edu \
    --cc=ahferroin7@gmail.com \
    --cc=andi@firstfloor.org \
    --cc=jepler@unpythonic.net \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@horizon.com \
    --cc=linux@rasmusvillemoes.dk \
    /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.