Re: [address@hidden: Help with entropy?]

[Michael Casadevall]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Thanks for going through it.
On Jun 20, 2007, at 5:31 PM, Thomas Schwinge wrote:

> Hello!
>
> I just browsed (a bit) through tschwinge's Sammelsurium and found the
> following links and things which may be interesting for you:
> <http://download.ibofobi.dk/hurd-entropy/gnumach-entropy.diff.bz2>.
> Furthermore, I also have an mbox file with 166 old emails of  
> discussions
> on Hurd mailing lists, emails about gathering entropy in the kernel  
> and
> translators for `/dev/{,u}random', as well as source code for some
> translators.  I think I can publish that stuff somewhere.
>
>
> From: Michael Casadevall <sonicmctails@gmail.com>
> > My problem is when the buffer
> > is completely drained (like when I use GPG to generate a key), it
> > fails to refill.
>
> Has this problem been resolved now?  As a test case I wouldn't use  
> `gpg',
> but a simple ``od --width=1 -t x1 < /dev/entropy''.
>
>
> | Index: i386/i386at/conf.c
>
> | + extern int	entropyopen(), entropyclose(), entropyread(),  
> entropygetstat();
>
> Just include <device/entropy.h> instead.

Most of the other entries in that file use the extern, hence why I  
did, but I'll change that now.

> | Index: linux/configfrag.ac
>
> | +     [if [ x"$enable_kmsg" != xno ]; then]
>
> s/kmsg/entropy
Yeah. Oops :-)

> | entropy.h
>
> | #define ENTROPYBUFSIZE sizeof(unsigned long)*128
>
> Should this really be defined in here?  I'd put it into  
> `entropy.c', as
> the buffer size is internal business and is of no interest to the  
> user of
> the entropy device interface.
>
> | /* typedef struct {
> |   unsigned long ticks;
> |   int data;
> | } entropy_data; */
>
> Likewise.

Fair enough. I thought about putting it as configurable setting  
through autoconf.
> | entropy.c
>
> | /*
> |  * Mach Kernel - Entropy Generating Device
> |  * Copyright (C) 2007 Free Software Foundation.
>
> Did you originally copy this file from `device/kmsg.c'?  If yes,  
> then you
> should retain the original copyright lines.
No code was directly copied from kmsg.c. I had it open though and  
used it as a template to figure out what I was doing. kmsg.c does  
have the copyright assigned to the FSF, I can easily add the  
additional copyright notices
> | /*
> |  * The following is from linux's random.c, and is used
> |  * under the GPL
> |  *
> |  * Look there for a full explination on what these numbers
> |  * mean and how this works
> |  */
> |
> | static struct poolinfo {
> |       int poolwords;
> |       int tap1, tap2, tap3, tap4, tap5;
> |   } poolinfo_table [] = {
> |     /* x^128 + x^103 + x^76 + x^51 +x^25 + x + 1 -- 105 */
> |     { 128,    103,    76,     51,     25,     1 },
>
> (Used in `entropy_putchar'.)  Albeit impressive, why do we want to  
> have
> this machinery inside the kernel?  Can't we put that into userspace  
> and
> have the kernel just export the raw entropy bits we gather from  
> devices
> or interrupts?

After a long talk with antrik, I'm moving it into a translator (I'm  
pretty much ripping streamio to shreads to implement this), if I  
remove all the twisting code, can we get this merged into mach?

> | io_return_t entropyopen(dev_t dev, int flag, io_req_t ior) {
> |  /* Lock the function so we don't get a race condition */
> |   simple_lock(&entropy_lock);
> |
> |   /* Check to see if we've initalized entropy */
> |   if(entropy_init_done == 0) {
> |         entropyinit();
> |   }
> |
> |   entropy_in_use = 1;
> |
> |   /* We're done, unlock, and return success */
> |   simple_unlock(&entropy_lock);
> |   return D_SUCCESS;
> | }
>
> You're allowing for more then one user of the `entropy' device.  Is  
> this
> intentional?  As soon as we have real translators sitting on
> `/dev/{,u}random' / `/dev/entropy' it isn't anymore, I think.

For whatever reason, someone might attach a second random translator.  
Better be safe then sorry because this code could blow up wonderfully  
if multiple users tried to use it at the same time.

> | void entropy_putchar(int c) {
>
> |   /* Its possible we MIGHT get here before the device is opened */
> |   /* so just make sure we're initalized before doing anythign!  */
> |
> |   if(!entropy_init_done) {
> |     entropyinit();
> |   }
>
> As this will definitely happen as soon as the first interrupt  
> occurs (or
> whatever), why not just avoid this
> calling-`entropyinit'-if-not-already-done dance which is used all over
> the place and just call the function once, soon enough?  ``Soon  
> enough''
> may be difficult (or impossible?), but I'd nevertheless just call  
> it once
> and then have at this place here just a ``if (! entropy_init_done)
> return;'' instead.

I thought about that, but init won't be called until the device is  
opened, which could be considerable time, and it would miss all the  
input data from before the device was initialized. We could simply  
stick the entropyinit somewhere else in the kernel though so that  
wouldn't be a problem.

> I did not yet check your ring-buffering magic (the thing Olaf  
> pointed out
> in his email, for example); I'll do that as soon as you say that your
> code is ready for inclusion.
This works the way its supposed to; I just went through all the code  
and made sure of it. A second look is always welcomed.

> Regards,
>  Thomas

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.6 (Darwin)

iD8DBQFGeqrIV/FBeJ6jsZQRAqGpAJ4q/pvFLXN9fKZC+/h3lNoFN2/r0QCfQKGu
NU3tEMkplj8dc8Xxj/XnrcI=
=+biA
-----END PGP SIGNATURE-----