Re: locking against myself panic (cprng_strongreseed, filt_rndread)

[Taylor R Campbell <campbell+netbsd-tech-kern%mumble.net@localhost>]

> Date: Fri, 10 Nov 2017 13:58:51 +0100
> From: Edgar Fuß <ef%math.uni-bonn.de@localhost>
> 
> Short: Your patch seems to work.

Pullup request submitted:
https://releng.netbsd.org/cgi-bin/req-6.cgi?show=1512

> Long: I don't quite understand the behaviour, but at least, it doesn't panic.
> 
> I wrote a test programm (my first encounter with kqueue, so probably wrong) 
> to exercise EVFILT_READ.
> I created a named pipe and it did approximately what I expected (on start, 
> it hung in fiford, which I didn't expect).
> I wanted to test in on /dev/urandom, it read 512 bytes, then timed out 
> (program wrote dots) and paniced (same traceback as originally) after 
> about 15 of them.
> On /dev/random, it read 512 bytes, wrote dots and didn't panic (at least 
> not for 20 seconds). On a second run on /dev/random, it read 512 bytes 
> and paniced immediately after.

There are multiple compounding bugs here.  Here's how it works:

1. - When you register an EVFILT_READ event for /dev/random, it draws
     a PRNG seed from the entropy pool (`depleting it'), and records
     that _if_ entropy is ever added to the pool then the waiter
     should be notified.

   - Same for /dev/urandom, except it doesn't draw a new seed if there
     already was a per-CPU PRNG initialized.

   (Note that the caller blocks if there already is entropy in the
   pool -- the event only waits for _adding_ entropy to the pool.
   This is an independent bug that was also fixed in netbsd-7.)

2. When someone later adds entropy to the pool (e.g., uvm fault, or
   network packet arrival, or disk seek, or `echo htthththttthtthh >
   /dev/random'), cprng_strong_reseed gets called.

3. cprng_strong_reseed does a locking dance with a pair of cyclic
   mutexes (also gone in netbsd-7).  If it wins the dance, it calls
   cprng_strong_doreseed with c->reseed.mtx and c->mtx held.

4. cprng_strong_doreseed calls selnotify, which notifies all the
   kqueue events by...

5. ...calling filt_rndread, which calls cprng_strong_ready, which
   acquires c->mtx again --> panic.

The patch changes cprng_strong_ready so that it requires the caller to
hold c->mtx (which was necessary anyway for the caller to make
sensible decisions on the basis of invariants preserved by c->mtx),
and enables two ways to call filt_rndread:

- with c->mtx not held, indicated by having NOTE_SUBMIT clear, when
  the waiter is registering the kevent and about to sleep; and
- with c->mtx held, indicated by having NOTE_SUBMIT set, when the
  event happens.

(This kind of crazy NOTE_SUBMIT protocol turns out to be the standard
way to write kqueue filters.  Go figure.)

> With the patch, it read behaved identically on the first run; on the second, 
> it read about twenty 512-byte chunks and continued writing dots (if I 
> remember correctly). In any case, I couldn't make it panic.

Right -- all of the _other_ bugs in netbsd-6 /dev/random are still
there.  All that the patch changed is the instapanic on adding entropy
to the pool when someone is waiting with kqueue to read from
/dev/u?random.