Hi - Looks nice, minor comments below.
On 2016-02-04 at 04:01 ron minnich <[email protected]> wrote: > This is a series starting with import the code, formatting it, > getting it to build, and changing the device. > > > The following changes since commit > 47341e4900dead535ab786d887607f8f8e2ea194: > > New and easy strace framework. (2016-02-03 18:31:54 -0500) > > are available in the git repository at: > > [email protected]:rminnich/akaros newrandom > From 3666b7600e0a986e64a7f961674d32aa2fae1cb0 Mon Sep 17 00:00:00 2001 > From: "Ronald G. Minnich" <[email protected]> > Date: Wed, 3 Feb 2016 19:09:02 -0800 > Subject: Add a random device; remove old genrandom junk; remove random from > #cons > diff --git a/kern/drivers/dev/random.c b/kern/drivers/dev/random.c > new file mode 100644 > index 000000000000..230b7a57495d > --- /dev/null > +++ b/kern/drivers/dev/random.c > @@ -0,0 +1,241 @@ > +/* > + * copyright here. > + */ This needs fixed. AFAIK, urandomread looks like it's from another project and the other random bits at the end are from the original random.c file. Though I think we can delete those. > +#include <vfs.h> > +#include <kfs.h> > +#include <slab.h> > +#include <kmalloc.h> > +#include <kref.h> > +#include <string.h> > +#include <stdio.h> > +#include <assert.h> > +#include <error.h> > +#include <cpio.h> > +#include <pmap.h> > +#include <smp.h> > +#include <ip.h> > +#include <random/fortuna.h> > + > +static qlock_t rl; > +static spinlock_t tap_lock; > +static int len; > + > +/* While the rule *used to be* that urandom did not block, this rng is fast > enough > + * that we don't think we will need that distinction. I'm not sure anything > will > + * every really block. But some things may want a select-like interface > anyway. */ checkpatch complained about this: WARNING: line over 80 characters #156: FILE: kern/drivers/dev/random.c:24: +/* While the rule *used to be* that urandom did not block, this rng is fast enough WARNING: line over 80 characters #158: FILE: kern/drivers/dev/random.c:26: + * every really block. But some things may want a select-like interface anyway. */ ERROR: trailing whitespace #163: FILE: kern/drivers/dev/random.c:31: + * hardware entropy source. $ > +struct fdtap_slist data_tap; > + > +/* > + * Add entropy. This is not currently used but we might want to hook it into > a > + * hardware entropy source. > + */ > +void random_add(void *xp) > +{ > + ERRSTACK(1); > + > + if (waserror()) { > + qunlock(&rl); > + nexterror(); > + } > + > + qlock(&rl); For these funcs, do we want to qlock before the waserror? With the understanding that waserror undoes things done before it (in the simpler cases)? waserror() can handle either style, so I'm fine either way. > + fortuna_add_entropy(xp, sizeof(xp)); > + qunlock(&rl); > + > + poperror(); > +} > + > +/* > + * consume random bytes > + */ > +static uint32_t _randomread(void *xp, uint32_t n) > +{ > + ERRSTACK(1); > + > + if (waserror()) { > + qunlock(&rl); > + nexterror(); > + } > + > + qlock(&rl); > + fortuna_get_bytes(n, xp); > + qunlock(&rl); > + > + poperror(); > + > + return n; > +} > + > +/** > + * Fast random generator > + **/ > +uint32_t urandomread(void *xp, uint32_t n) > +{ > + ERRSTACK(1); > + uint64_t seed[16]; > + uint8_t *e, *p; > + uint32_t x = 0; > + uint64_t s0; > + uint64_t s1; > + > + if (waserror()) > + nexterror(); I think we don't want a waserror() at all here (and thus the poperror below). If all we do is nexterror, we didn't really do anything. > + > + // The initial seed is from a good random pool. > + _randomread(seed, sizeof(seed)); > + p = xp; > + for (e = p + n; p < e;) { > + s0 = seed[x]; > + s1 = seed[x = (x + 1) & 15]; > + s1 ^= s1 << 31; > + s1 ^= s1 >> 11; > + s0 ^= s0 >> 30; > + *p++ = (seed[x] = s0 ^ s1) * 1181783497276652981LL; > + } > + poperror(); > + return n; > +} > + > +static struct walkqid *randomwalk(struct chan *c, struct chan *nc, char > **name, > + int nname) > +{ > + return devwalk(c, nc, name, nname, randomdir, ARRAY_SIZE(randomdir), > devgen); Checkpatch complaint: WARNING: line over 80 characters #270: FILE: kern/drivers/dev/random.c:138: + return devwalk(c, nc, name, nname, randomdir, ARRAY_SIZE(randomdir), devgen); > +static long randomread(struct chan *c, void *va, long n, int64_t ignored) > +{ > + switch (c->qid.path) { > + case Qdir: > + return devdirread(c, va, n, randomdir, > + ARRAY_SIZE(randomdir), devgen); > + case Qrandom: > + return _randomread(va, n); > + case Qurandom: > + return _randomread(va, n); Should Qurandom be calling urandomread instead of randomread? > + default: > + panic("randomread: qid %d is impossible", c->qid.path); > + } > + return -1; /* not reached */ > +} > +/* This is something used by the TCP stack. > + * I have no idea of why these numbers were chosen. */ > +static uint32_t randn; > + > +static void seedrand(void) > +{ > + ERRSTACK(2); > + if (!waserror()) { > + _randomread((void *)&randn, sizeof(randn)); > + poperror(); > + } > +} > + > +int nrand(int n) > +{ > + if (randn == 0) > + seedrand(); > + randn = randn * 1103515245 + 12345 + read_tsc(); This is nasty crap. I think I hacked in that read_tsc() to give us at least some variation in a busted RNG. I think nrand() is used as a way to get 16 bits of a random number. We can probably change tcp to use urandom or something. It looks like TCP is using nrand for SEQ numbers, so maybe we just give them a number. > + return (randn >> 16) % n; > +} > + > +int rand(void) > +{ > + nrand(1); > + return randn; > +} No one uses this other than a cheap test; I think we can remove all of this crap. > diff --git a/kern/lib/random/fortuna.c b/kern/lib/random/fortuna.c > index 868d59b55618..90c86cff0d04 100644 > --- a/kern/lib/random/fortuna.c > +++ b/kern/lib/random/fortuna.c > @@ -31,9 +31,12 @@ > > #include <arch/arch.h> > #include <time.h> > + > +#include <random/fortuna.h> > #include <random/rijndael.h> > #include <random/sha2.h> > > + > /* > * Why Fortuna-like: There does not seem to be any definitive reference > * on Fortuna in the net. Instead this implementation is based on I think this diff should have been squashed into d111786e69fb ("Get the basic random number generator functions to compile"). It's a good git exercise to do it. (I can help). -- You received this message because you are subscribed to the Google Groups "Akaros" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. To post to this group, send email to [email protected]. For more options, visit https://groups.google.com/d/optout.
