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

Andrew Morton
2003-08-16T05:02:34Z

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.