svn commit: r273958 - head/sys/dev/random

[Konstantin Belousov]

On Sun, Nov 02, 2014 at 07:24:13PM +0000, Mark R V Murray wrote:
> 
> > On 2 Nov 2014, at 19:20, Konstantin Belousov <kostikbel at gmail.com> wrote:
> > 
> > On Sun, Nov 02, 2014 at 11:05:27AM -0800, Adrian Chadd wrote:
> >> [snip all the conversation]
> >> 
> >> Ok. There's still a problem that I can trigger by trying to Ctrl-C a
> >> process that's blocked reading for randomness. I'll try to chase up
> >> more details about and file a PR about it.
> >> 
> >> The unfortunate part is that the kernel side stack trace of the
> >> offending / hung process isn't currently helpful. :(
> >> 
> > 
> >> From what I see, signals are essentially ignored in the read code.
> > See random_adaptors.c:random_adaptor_read():
> > 
> > 		/* Sleep instead of going into a spin-frenzy */
> > 		tsleep(&random_adaptor, PUSER | PCATCH, "block", hz/10);
> > 
> > The error which would indicate the signal catch, is dropped.  Also,
> > unbound sleep does not drop random_adaptor_lock, which means that
> > you cannot module which could provide some more randomness for you,
> > while any thread is stuck in read loop.
> 
> Hi
> 
> I don???t quite follow what you mean, but it sounds like you understand
> the problem. Could you please explain with a bit more detail?

Which problem ? There are two.

One is the Adrian' complain. tsleep(9) catches signals, and return
EINTR/ERESTART when catched.  Typical driver code checks for the
errors from {t,m}sleep(9) and aborts the operation if error is
returned.  I.e. you should do
	error = tsleep(...);
	if (error != 0) {
		abort the loop;
		return to caller;
	}
The fine detail is that for the case when read has already partially
progressed, i.e. something was copied out to uio, the error must
not be returned, but short read performed instead.

This leads to another question about the code in random_adapter_read():
if ra_read method sleeps, it must handle the signals as well, return
error, and the second loop which perorms ra_read/uiomove should be
aborted as well.  Again, error from either ra_read or uiomove(9)
must result in short read if something was already copied to uio.
Currently, there is no error returned by ra_read (or it is ignored),
and error from uiomove always returned, even if something was already
copied.

Second problem is that random_adaptor_lock is owned while tsleep()
is called (or whatever sleep primitive is used inside ra_read).  If
platform could only provide randomness through some hw, and module
is loaded while thread is blocked, module cannot register, while
reading thread cannot make progress.