Re: [RFC][PATCH] Make cryptoapi non-optional?

[Andrew Morton <akpm@osdl.org>]

Matt Mackall <mpm@selenic.com> wrote:
>
> a) extract_entropy (pool->lock)
> 
> ...
> 
>  [There was also a cute sleeping problem here with random_read.
>  random_read used old-style open-coded sleeping, marked itself
>  TASK_INTERRUPTIBLE, then called extract_entropy, which would do a
>  conditional reschedule, and fall asleep until the next wake up,
>  despite having enough entropy to fulfill the request.]

OK.  There was no description of all this in your initial patch, so I don't
know which patches solve these problems.

> ...
>  [By the way, whoever did the workqueue conversion for 2.5 changed this
>  code to wakeup the processing worker when the sample pool was half
>  full rather on every sample but got the test not quite right.

That would have been me.  The context switch rate due to
add_disk_randomness() was much too high (1000/sec iirc), so that was an
attempt to reduce the wakeup rate.

What we really should try to do is to not pass the work to keventd at all:
just do the entropy addition right there in interrupt context, with
appropriate locking.


> f) change_poolsize (queued for resend)

Queued for resend because I just didn't know what to do with the patches.

I do not understand the random driver, and judging from the past couple of
days discussions I'm not likely to.  I don't know how to test it and I
doubt if anyone else is testing it in sufficient depth.

And, to add to my dilemma, the random driver is security-related and
something which scary propellor-headed types get all emotional about :)

So getting these changes in will be hard.  It would help a lot if they
could be presented individually and that Ted, Jamie and/or other interested
parties were to review and ack them.

> g) urandom starves/races random (queued for resend)
> 
>  Readers of /dev/urandom and get_random_bytes (both nonblock) pull from
>  the same pool as /dev/random readers and without limit. As there are
>  numerous users of get_random_bytes as pointed out above, /dev/random
>  readers can easily be starved (and previously, race on wakeup), even
>  by remote readers. This is rather a problem for the classic
>  entropy-source-limited headless web server which may very well be
>  trying to use both in, for example, a departmental certificate
>  authority.
> 
>  My solution is to clean up the pool creation code and add a second
>  output pool for nonblocking users. The pool reseeding logic is
>  cleaned up to address a bunch of corner cases and has a low watermark
>  parameter so that the nonblocking users can avoid draining the input
>  pool entirely. The current default is to not let nonblocking readers
>  draw the input pool below the point where blocking readers can do two
>  catastrophic reseeds.
> 
>  The cleanup of the pool code lets this easily become per_cpu output
>  pools for the non-blocking readers with about 10 lines of code if the
>  above-mentioned contention is an issue. I haven't tried this yet, but
>  I already did per_cpu for the cryptoapi stuff and it should be about
>  the same.
> 
>  We could go completely lockless for the nonblocking pool also, but
>  that would require some code duplication.

hrm.  That's one which I actually understood.  There is yet hope.