From dfd38750db3cc17a1da3405389a81b430720c2c6 Mon Sep 17 00:00:00 2001 From: Greg Price Date: Fri, 29 Nov 2013 14:58:06 -0500 Subject: [PATCH 01/16] random: fix typos / spelling errors in comments Signed-off-by: Greg Price Signed-off-by: "Theodore Ts'o" --- drivers/char/random.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/char/random.c b/drivers/char/random.c index 429b75bb60e8..b8e44e25bc36 100644 --- a/drivers/char/random.c +++ b/drivers/char/random.c @@ -305,7 +305,7 @@ static int random_read_wakeup_thresh = 64; static int random_write_wakeup_thresh = 28 * OUTPUT_POOL_WORDS; /* - * The minimum number of seconds between urandom pool resending. We + * The minimum number of seconds between urandom pool reseeding. We * do this to limit the amount of entropy that can be drained from the * input pool even if there are heavy demands on /dev/urandom. */ @@ -322,7 +322,7 @@ static int random_min_urandom_seed = 60; * Register. (See M. Matsumoto & Y. Kurita, 1992. Twisted GFSR * generators. ACM Transactions on Modeling and Computer Simulation * 2(3):179-194. Also see M. Matsumoto & Y. Kurita, 1994. Twisted - * GFSR generators II. ACM Transactions on Mdeling and Computer + * GFSR generators II. ACM Transactions on Modeling and Computer * Simulation 4:254-266) * * Thanks to Colin Plumb for suggesting this. @@ -1036,7 +1036,7 @@ static void extract_buf(struct entropy_store *r, __u8 *out) sha_transform(hash.w, (__u8 *)(r->pool + i), workspace); /* - * If we have a architectural hardware random number + * If we have an architectural hardware random number * generator, mix that in, too. */ for (i = 0; i < LONGS(20); i++) { From f22052b2025b78475a60dbe01b66b3120f4faefa Mon Sep 17 00:00:00 2001 From: Greg Price Date: Fri, 29 Nov 2013 14:58:16 -0500 Subject: [PATCH 02/16] random: fix comment on proc_do_uuid There's only one function here now, as uuid_strategy is long gone. Also make the bit about "If accesses via ..." clearer. Signed-off-by: Greg Price Signed-off-by: "Theodore Ts'o" --- drivers/char/random.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/char/random.c b/drivers/char/random.c index b8e44e25bc36..f0eea5fdc1b7 100644 --- a/drivers/char/random.c +++ b/drivers/char/random.c @@ -1512,13 +1512,13 @@ static int max_write_thresh = INPUT_POOL_WORDS * 32; static char sysctl_bootid[16]; /* - * These functions is used to return both the bootid UUID, and random + * This function is used to return both the bootid UUID, and random * UUID. The difference is in whether table->data is NULL; if it is, * then a new UUID is generated and returned to the user. * - * If the user accesses this via the proc interface, it will be returned - * as an ASCII string in the standard UUID format. If accesses via the - * sysctl system call, it is returned as 16 bytes of binary data. + * If the user accesses this via the proc interface, the UUID will be + * returned as an ASCII string in the standard UUID format; if via the + * sysctl system call, as 16 bytes of binary data. */ static int proc_do_uuid(struct ctl_table *table, int write, void __user *buffer, size_t *lenp, loff_t *ppos) From 18e9cea74951b64282964f9625db94c5d5a007bd Mon Sep 17 00:00:00 2001 From: Greg Price Date: Fri, 29 Nov 2013 14:59:45 -0500 Subject: [PATCH 03/16] random: fix description of get_random_bytes After this remark was written, commit d2e7c96af added a use of arch_get_random_long() inside the get_random_bytes codepath. The main point stands, but it needs to be reworded. Signed-off-by: Greg Price Signed-off-by: "Theodore Ts'o" --- drivers/char/random.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/char/random.c b/drivers/char/random.c index f0eea5fdc1b7..fcda7d3937e0 100644 --- a/drivers/char/random.c +++ b/drivers/char/random.c @@ -1170,8 +1170,9 @@ static ssize_t extract_entropy_user(struct entropy_store *r, void __user *buf, /* * This function is the exported kernel interface. It returns some * number of good random numbers, suitable for key generation, seeding - * TCP sequence numbers, etc. It does not use the hw random number - * generator, if available; use get_random_bytes_arch() for that. + * TCP sequence numbers, etc. It does not rely on the hardware random + * number generator. For random bytes direct from the hardware RNG + * (when available), use get_random_bytes_arch(). */ void get_random_bytes(void *buf, int nbytes) { From 12ff3a517ab92b5496c731a3c354caa1f16c569f Mon Sep 17 00:00:00 2001 From: Greg Price Date: Fri, 29 Nov 2013 15:02:33 -0500 Subject: [PATCH 04/16] random: simplify loop in random_read The loop condition never changes until just before a break, so we might as well write it as a constant. Also since a996996dd75a ("random: drop weird m_time/a_time manipulation") we don't do anything after the loop finishes, so the 'break's might as well return directly. Some other simplifications. There should be no change in behavior introduced by this commit. Signed-off-by: Greg Price Signed-off-by: "Theodore Ts'o" --- drivers/char/random.c | 53 +++++++++++++------------------------------ 1 file changed, 16 insertions(+), 37 deletions(-) diff --git a/drivers/char/random.c b/drivers/char/random.c index fcda7d3937e0..fcc2bff8f887 100644 --- a/drivers/char/random.c +++ b/drivers/char/random.c @@ -1285,53 +1285,32 @@ void rand_initialize_disk(struct gendisk *disk) static ssize_t random_read(struct file *file, char __user *buf, size_t nbytes, loff_t *ppos) { - ssize_t n, retval = 0, count = 0; + ssize_t n; if (nbytes == 0) return 0; - while (nbytes > 0) { - n = nbytes; - if (n > SEC_XFER_SIZE) - n = SEC_XFER_SIZE; - - n = extract_entropy_user(&blocking_pool, buf, n); - - if (n < 0) { - retval = n; - break; - } - + nbytes = min_t(size_t, nbytes, SEC_XFER_SIZE); + while (1) { + n = extract_entropy_user(&blocking_pool, buf, nbytes); + if (n < 0) + return n; trace_random_read(n*8, (nbytes-n)*8, ENTROPY_BITS(&blocking_pool), ENTROPY_BITS(&input_pool)); + if (n > 0) + return n; + /* Pool is (near) empty. Maybe wait and retry. */ - if (n == 0) { - if (file->f_flags & O_NONBLOCK) { - retval = -EAGAIN; - break; - } + if (file->f_flags & O_NONBLOCK) + return -EAGAIN; - wait_event_interruptible(random_read_wait, - ENTROPY_BITS(&input_pool) >= - random_read_wakeup_thresh); - - if (signal_pending(current)) { - retval = -ERESTARTSYS; - break; - } - - continue; - } - - count += n; - buf += n; - nbytes -= n; - break; /* This break makes the device work */ - /* like a named pipe */ + wait_event_interruptible(random_read_wait, + ENTROPY_BITS(&input_pool) >= + random_read_wakeup_thresh); + if (signal_pending(current)) + return -ERESTARTSYS; } - - return (count ? count : retval); } static ssize_t From 19fa5be1d92be3112521145bf99f77007abf6b16 Mon Sep 17 00:00:00 2001 From: Greg Price Date: Fri, 29 Nov 2013 15:50:06 -0500 Subject: [PATCH 05/16] random: fix comment on "account" This comment didn't quite keep up as extract_entropy() was split into four functions. Put each bit by the function it describes. Signed-off-by: Greg Price Signed-off-by: "Theodore Ts'o" --- drivers/char/random.c | 31 +++++++++++++++++++++---------- 1 file changed, 21 insertions(+), 10 deletions(-) diff --git a/drivers/char/random.c b/drivers/char/random.c index fcc2bff8f887..2c532a6b0a21 100644 --- a/drivers/char/random.c +++ b/drivers/char/random.c @@ -958,17 +958,9 @@ static void push_to_pool(struct work_struct *work) } /* - * These functions extracts randomness from the "entropy pool", and - * returns it in a buffer. - * - * The min parameter specifies the minimum amount we can pull before - * failing to avoid races that defeat catastrophic reseeding while the - * reserved parameter indicates how much entropy we must leave in the - * pool after each pull to avoid starving other readers. - * - * Note: extract_entropy() assumes that .poolwords is a multiple of 16 words. + * This function decides how many bytes to actually take from the + * given pool, and also debits the entropy count accordingly. */ - static size_t account(struct entropy_store *r, size_t nbytes, int min, int reserved) { @@ -1018,6 +1010,12 @@ retry: return ibytes; } +/* + * This function does the actual extraction for extract_entropy and + * extract_entropy_user. + * + * Note: we assume that .poolwords is a multiple of 16 words. + */ static void extract_buf(struct entropy_store *r, __u8 *out) { int i; @@ -1079,6 +1077,15 @@ static void extract_buf(struct entropy_store *r, __u8 *out) memset(&hash, 0, sizeof(hash)); } +/* + * This function extracts randomness from the "entropy pool", and + * returns it in a buffer. + * + * The min parameter specifies the minimum amount we can pull before + * failing to avoid races that defeat catastrophic reseeding while the + * reserved parameter indicates how much entropy we must leave in the + * pool after each pull to avoid starving other readers. + */ static ssize_t extract_entropy(struct entropy_store *r, void *buf, size_t nbytes, int min, int reserved) { @@ -1129,6 +1136,10 @@ static ssize_t extract_entropy(struct entropy_store *r, void *buf, return ret; } +/* + * This function extracts randomness from the "entropy pool", and + * returns it in a userspace buffer. + */ static ssize_t extract_entropy_user(struct entropy_store *r, void __user *buf, size_t nbytes) { From ee1de406ba6eb1e01f143fe3351e70cc772cc63e Mon Sep 17 00:00:00 2001 From: Greg Price Date: Fri, 29 Nov 2013 15:56:16 -0500 Subject: [PATCH 06/16] random: simplify accounting logic This logic is exactly equivalent to the old logic, but it should be easier to see what it's doing. The equivalence depends on one fact from outside this function: when 'r->limit' is false, 'reserved' is zero. (Well, two facts; the other is that 'reserved' is never negative.) Cc: Jiri Kosina Cc: "H. Peter Anvin" Signed-off-by: Greg Price Signed-off-by: "Theodore Ts'o" --- drivers/char/random.c | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/drivers/char/random.c b/drivers/char/random.c index 2c532a6b0a21..9675821b4b5a 100644 --- a/drivers/char/random.c +++ b/drivers/char/random.c @@ -984,14 +984,10 @@ retry: ibytes = 0; } else { /* If limited, never pull more than available */ - if (r->limit && ibytes + reserved >= have_bytes) - ibytes = have_bytes - reserved; - - if (have_bytes >= ibytes + reserved) - entropy_count -= ibytes << (ENTROPY_SHIFT + 3); - else - entropy_count = reserved << (ENTROPY_SHIFT + 3); - + if (r->limit) + ibytes = min_t(size_t, ibytes, have_bytes - reserved); + entropy_count = max_t(int, 0, + entropy_count - (ibytes << (ENTROPY_SHIFT + 3))); if (cmpxchg(&r->entropy_count, orig, entropy_count) != orig) goto retry; From a58aa4edc6d2e779894b1fa95a2f4de157ff3b3b Mon Sep 17 00:00:00 2001 From: Greg Price Date: Fri, 29 Nov 2013 20:09:37 -0500 Subject: [PATCH 07/16] random: forget lock in lockless accounting The only mutable data accessed here is ->entropy_count, but since 10b3a32d2 ("random: fix accounting race condition") we use cmpxchg to protect our accesses to ->entropy_count here. Drop the use of the lock. Cc: Jiri Kosina Signed-off-by: Greg Price Signed-off-by: "Theodore Ts'o" --- drivers/char/random.c | 4 ---- 1 file changed, 4 deletions(-) diff --git a/drivers/char/random.c b/drivers/char/random.c index 9675821b4b5a..694510af4fcd 100644 --- a/drivers/char/random.c +++ b/drivers/char/random.c @@ -970,9 +970,6 @@ static size_t account(struct entropy_store *r, size_t nbytes, int min, int entropy_count, orig; size_t ibytes; - /* Hold lock while accounting */ - spin_lock_irqsave(&r->lock, flags); - BUG_ON(r->entropy_count > r->poolinfo->poolfracbits); /* Can we pull enough? */ @@ -995,7 +992,6 @@ retry: < random_write_wakeup_thresh) wakeup_write = 1; } - spin_unlock_irqrestore(&r->lock, flags); trace_debit_entropy(r->name, 8 * ibytes); if (wakeup_write) { From 8c2aa3390ebb59cba4495a56557b70ad0575eef5 Mon Sep 17 00:00:00 2001 From: Greg Price Date: Thu, 5 Dec 2013 19:19:29 -0500 Subject: [PATCH 08/16] random: tighten bound on random_read_wakeup_thresh We use this value in a few places other than its literal meaning, in particular in _xfer_secondary_pool() as a minimum number of bits to pull from the input pool at a time into either output pool. It doesn't make sense to pull more bits than the whole size of an output pool. We could and possibly should separate the quantities "how much should the input pool have to have to wake up /dev/random readers" and "how much should we transfer from the input to an output pool at a time", but nobody is likely to be sad they can't set the first quantity to more than 1024 bits, so for now just limit them both. Signed-off-by: Greg Price Signed-off-by: "Theodore Ts'o" --- drivers/char/random.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/char/random.c b/drivers/char/random.c index 694510af4fcd..70b8ebf08edd 100644 --- a/drivers/char/random.c +++ b/drivers/char/random.c @@ -1490,7 +1490,7 @@ EXPORT_SYMBOL(generate_random_uuid); #include static int min_read_thresh = 8, min_write_thresh; -static int max_read_thresh = INPUT_POOL_WORDS * 32; +static int max_read_thresh = OUTPUT_POOL_WORDS * 32; static int max_write_thresh = INPUT_POOL_WORDS * 32; static char sysctl_bootid[16]; From 0fb7a01af5b0cbe5bf365891fc4d186f2caa23f7 Mon Sep 17 00:00:00 2001 From: Greg Price Date: Thu, 5 Dec 2013 19:32:19 -0500 Subject: [PATCH 09/16] random: simplify accounting code With this we handle "reserved" in just one place. As a bonus the code becomes less nested, and the "wakeup_write" flag variable becomes unnecessary. The variable "flags" was already unused. This code behaves identically to the previous version except in two pathological cases that don't occur. If the argument "nbytes" is already less than "min", then we didn't previously enforce "min". If r->limit is false while "reserved" is nonzero, then we previously applied "reserved" in checking whether we had enough bits, even though we don't apply it to actually limit how many we take. The callers of account() never exercise either of these cases. Before the previous commit, it was possible for "nbytes" to be less than "min" if userspace chose a pathological configuration, but no longer. Cc: Jiri Kosina Cc: "H. Peter Anvin" Signed-off-by: Greg Price Signed-off-by: "Theodore Ts'o" --- drivers/char/random.c | 27 ++++++++++----------------- 1 file changed, 10 insertions(+), 17 deletions(-) diff --git a/drivers/char/random.c b/drivers/char/random.c index 70b8ebf08edd..ded4339be8f9 100644 --- a/drivers/char/random.c +++ b/drivers/char/random.c @@ -964,8 +964,6 @@ static void push_to_pool(struct work_struct *work) static size_t account(struct entropy_store *r, size_t nbytes, int min, int reserved) { - unsigned long flags; - int wakeup_write = 0; int have_bytes; int entropy_count, orig; size_t ibytes; @@ -977,24 +975,19 @@ retry: entropy_count = orig = ACCESS_ONCE(r->entropy_count); have_bytes = entropy_count >> (ENTROPY_SHIFT + 3); ibytes = nbytes; - if (have_bytes < min + reserved) { + /* If limited, never pull more than available */ + if (r->limit) + ibytes = min_t(size_t, ibytes, have_bytes - reserved); + if (ibytes < min) ibytes = 0; - } else { - /* If limited, never pull more than available */ - if (r->limit) - ibytes = min_t(size_t, ibytes, have_bytes - reserved); - entropy_count = max_t(int, 0, - entropy_count - (ibytes << (ENTROPY_SHIFT + 3))); - if (cmpxchg(&r->entropy_count, orig, entropy_count) != orig) - goto retry; - - if ((r->entropy_count >> ENTROPY_SHIFT) - < random_write_wakeup_thresh) - wakeup_write = 1; - } + entropy_count = max_t(int, 0, + entropy_count - (ibytes << (ENTROPY_SHIFT + 3))); + if (cmpxchg(&r->entropy_count, orig, entropy_count) != orig) + goto retry; trace_debit_entropy(r->name, 8 * ibytes); - if (wakeup_write) { + if (ibytes && + (r->entropy_count >> ENTROPY_SHIFT) < random_write_wakeup_thresh) { wake_up_interruptible(&random_write_wait); kill_fasync(&fasync, SIGIO, POLL_OUT); } From 7d1b08c40c4f02c119476b29eca9bbc8d98d2a83 Mon Sep 17 00:00:00 2001 From: Greg Price Date: Sat, 7 Dec 2013 09:49:55 -0500 Subject: [PATCH 10/16] random: entropy_bytes is actually bits The variable 'entropy_bytes' is set from an expression that actually counts bits. Fortunately it's also only compared to values that also count bits. Rename it accordingly. Signed-off-by: Greg Price Signed-off-by: "Theodore Ts'o" --- drivers/char/random.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/char/random.c b/drivers/char/random.c index ded4339be8f9..581d806823e9 100644 --- a/drivers/char/random.c +++ b/drivers/char/random.c @@ -666,10 +666,10 @@ retry: r->entropy_total, _RET_IP_); if (r == &input_pool) { - int entropy_bytes = entropy_count >> ENTROPY_SHIFT; + int entropy_bits = entropy_count >> ENTROPY_SHIFT; /* should we wake readers? */ - if (entropy_bytes >= random_read_wakeup_thresh) { + if (entropy_bits >= random_read_wakeup_thresh) { wake_up_interruptible(&random_read_wait); kill_fasync(&fasync, SIGIO, POLL_IN); } @@ -678,7 +678,7 @@ retry: * forth between them, until the output pools are 75% * full. */ - if (entropy_bytes > random_write_wakeup_thresh && + if (entropy_bits > random_write_wakeup_thresh && r->initialized && r->entropy_total >= 2*random_read_wakeup_thresh) { static struct entropy_store *last = &blocking_pool; From 2132a96f66b6b4d865113e7d4cb56d5f7c6e3cdf Mon Sep 17 00:00:00 2001 From: Greg Price Date: Fri, 6 Dec 2013 21:28:03 -0500 Subject: [PATCH 11/16] random: clarify bits/bytes in wakeup thresholds These are a recurring cause of confusion, so rename them to hopefully be clearer. Signed-off-by: Greg Price Signed-off-by: "Theodore Ts'o" --- drivers/char/random.c | 34 +++++++++++++++++----------------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/drivers/char/random.c b/drivers/char/random.c index 581d806823e9..8cc7d6515676 100644 --- a/drivers/char/random.c +++ b/drivers/char/random.c @@ -295,14 +295,14 @@ * The minimum number of bits of entropy before we wake up a read on * /dev/random. Should be enough to do a significant reseed. */ -static int random_read_wakeup_thresh = 64; +static int random_read_wakeup_bits = 64; /* * If the entropy count falls under this number of bits, then we * should wake up processes which are selecting or polling on write * access to /dev/random. */ -static int random_write_wakeup_thresh = 28 * OUTPUT_POOL_WORDS; +static int random_write_wakeup_bits = 28 * OUTPUT_POOL_WORDS; /* * The minimum number of seconds between urandom pool reseeding. We @@ -669,7 +669,7 @@ retry: int entropy_bits = entropy_count >> ENTROPY_SHIFT; /* should we wake readers? */ - if (entropy_bits >= random_read_wakeup_thresh) { + if (entropy_bits >= random_read_wakeup_bits) { wake_up_interruptible(&random_read_wait); kill_fasync(&fasync, SIGIO, POLL_IN); } @@ -678,9 +678,9 @@ retry: * forth between them, until the output pools are 75% * full. */ - if (entropy_bits > random_write_wakeup_thresh && + if (entropy_bits > random_write_wakeup_bits && r->initialized && - r->entropy_total >= 2*random_read_wakeup_thresh) { + r->entropy_total >= 2*random_read_wakeup_bits) { static struct entropy_store *last = &blocking_pool; struct entropy_store *other = &blocking_pool; @@ -924,19 +924,19 @@ static void _xfer_secondary_pool(struct entropy_store *r, size_t nbytes) { __u32 tmp[OUTPUT_POOL_WORDS]; - /* For /dev/random's pool, always leave two wakeup worth's BITS */ - int rsvd = r->limit ? 0 : random_read_wakeup_thresh/4; + /* For /dev/random's pool, always leave two wakeups' worth */ + int rsvd_bytes = r->limit ? 0 : random_read_wakeup_bits / 4; int bytes = nbytes; - /* pull at least as many as BYTES as wakeup BITS */ - bytes = max_t(int, bytes, random_read_wakeup_thresh / 8); + /* pull at least as much as a wakeup */ + bytes = max_t(int, bytes, random_read_wakeup_bits / 8); /* but never more than the buffer size */ bytes = min_t(int, bytes, sizeof(tmp)); trace_xfer_secondary_pool(r->name, bytes * 8, nbytes * 8, ENTROPY_BITS(r), ENTROPY_BITS(r->pull)); bytes = extract_entropy(r->pull, tmp, bytes, - random_read_wakeup_thresh / 8, rsvd); + random_read_wakeup_bits / 8, rsvd_bytes); mix_pool_bytes(r, tmp, bytes, NULL); credit_entropy_bits(r, bytes*8); } @@ -952,7 +952,7 @@ static void push_to_pool(struct work_struct *work) struct entropy_store *r = container_of(work, struct entropy_store, push_work); BUG_ON(!r); - _xfer_secondary_pool(r, random_read_wakeup_thresh/8); + _xfer_secondary_pool(r, random_read_wakeup_bits/8); trace_push_to_pool(r->name, r->entropy_count >> ENTROPY_SHIFT, r->pull->entropy_count >> ENTROPY_SHIFT); } @@ -987,7 +987,7 @@ retry: trace_debit_entropy(r->name, 8 * ibytes); if (ibytes && - (r->entropy_count >> ENTROPY_SHIFT) < random_write_wakeup_thresh) { + (r->entropy_count >> ENTROPY_SHIFT) < random_write_wakeup_bits) { wake_up_interruptible(&random_write_wait); kill_fasync(&fasync, SIGIO, POLL_OUT); } @@ -1303,7 +1303,7 @@ random_read(struct file *file, char __user *buf, size_t nbytes, loff_t *ppos) wait_event_interruptible(random_read_wait, ENTROPY_BITS(&input_pool) >= - random_read_wakeup_thresh); + random_read_wakeup_bits); if (signal_pending(current)) return -ERESTARTSYS; } @@ -1334,9 +1334,9 @@ random_poll(struct file *file, poll_table * wait) poll_wait(file, &random_read_wait, wait); poll_wait(file, &random_write_wait, wait); mask = 0; - if (ENTROPY_BITS(&input_pool) >= random_read_wakeup_thresh) + if (ENTROPY_BITS(&input_pool) >= random_read_wakeup_bits) mask |= POLLIN | POLLRDNORM; - if (ENTROPY_BITS(&input_pool) < random_write_wakeup_thresh) + if (ENTROPY_BITS(&input_pool) < random_write_wakeup_bits) mask |= POLLOUT | POLLWRNORM; return mask; } @@ -1559,7 +1559,7 @@ struct ctl_table random_table[] = { }, { .procname = "read_wakeup_threshold", - .data = &random_read_wakeup_thresh, + .data = &random_read_wakeup_bits, .maxlen = sizeof(int), .mode = 0644, .proc_handler = proc_dointvec_minmax, @@ -1568,7 +1568,7 @@ struct ctl_table random_table[] = { }, { .procname = "write_wakeup_threshold", - .data = &random_write_wakeup_thresh, + .data = &random_write_wakeup_bits, .maxlen = sizeof(int), .mode = 0644, .proc_handler = proc_dointvec_minmax, From 46884442fc5bb81a896f7245bd850fde9b435509 Mon Sep 17 00:00:00 2001 From: Theodore Ts'o Date: Tue, 17 Dec 2013 21:16:39 -0500 Subject: [PATCH 12/16] random: use the architectural HWRNG for the SHA's IV in extract_buf() To help assuage the fears of those who think the NSA can introduce a massive hack into the instruction decode and out of order execution engine in the CPU without hundreds of Intel engineers knowing about it (only one of which woud need to have the conscience and courage of Edward Snowden to spill the beans to the public), use the HWRNG to initialize the SHA starting value, instead of xor'ing it in afterwards. Signed-off-by: "Theodore Ts'o" --- drivers/char/random.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/drivers/char/random.c b/drivers/char/random.c index 8cc7d6515676..d07575c99a5f 100644 --- a/drivers/char/random.c +++ b/drivers/char/random.c @@ -1012,23 +1012,23 @@ static void extract_buf(struct entropy_store *r, __u8 *out) __u8 extract[64]; unsigned long flags; - /* Generate a hash across the pool, 16 words (512 bits) at a time */ - sha_init(hash.w); - spin_lock_irqsave(&r->lock, flags); - for (i = 0; i < r->poolinfo->poolwords; i += 16) - sha_transform(hash.w, (__u8 *)(r->pool + i), workspace); - /* * If we have an architectural hardware random number - * generator, mix that in, too. + * generator, use it for SHA's initial vector */ + sha_init(hash.w); for (i = 0; i < LONGS(20); i++) { unsigned long v; if (!arch_get_random_long(&v)) break; - hash.l[i] ^= v; + hash.l[i] = v; } + /* Generate a hash across the pool, 16 words (512 bits) at a time */ + spin_lock_irqsave(&r->lock, flags); + for (i = 0; i < r->poolinfo->poolwords; i += 16) + sha_transform(hash.w, (__u8 *)(r->pool + i), workspace); + /* * We mix the hash back into the pool to prevent backtracking * attacks (where the attacker knows the state of the pool From d20f78d252778e0fae8f8256e602bd682eb2185c Mon Sep 17 00:00:00 2001 From: "H. Peter Anvin" Date: Mon, 17 Mar 2014 16:36:27 -0700 Subject: [PATCH 13/16] x86, random: Enable the RDSEED instruction Upcoming Intel silicon adds a new RDSEED instruction, which is similar to RDRAND but provides a stronger guarantee: unlike RDRAND, RDSEED will always reseed the PRNG from the true random number source between each read. Thus, the output of RDSEED is guaranteed to be 100% entropic, unlike RDRAND which is only architecturally guaranteed to be 1/512 entropic (although in practice is much more.) The RDSEED instruction takes the same time to execute as RDRAND, but RDSEED unlike RDRAND can legitimately return failure (CF=0) due to entropy exhaustion if too many threads on too many cores are hammering the RDSEED instruction at the same time. Therefore, we have to be more conservative and only use it in places where we can tolerate failures. This patch introduces the primitives arch_get_random_seed_{int,long}() but does not use it yet. Signed-off-by: H. Peter Anvin Reviewed-by: Ingo Molnar Cc: Benjamin Herrenschmidt Cc: Paul Mackerras Cc: Michael Ellerman Signed-off-by: Theodore Ts'o --- arch/powerpc/include/asm/archrandom.h | 9 +++++++ arch/x86/include/asm/archrandom.h | 39 ++++++++++++++++++++++++++- include/linux/random.h | 8 ++++++ 3 files changed, 55 insertions(+), 1 deletion(-) diff --git a/arch/powerpc/include/asm/archrandom.h b/arch/powerpc/include/asm/archrandom.h index d853d163ba47..801beba4e64b 100644 --- a/arch/powerpc/include/asm/archrandom.h +++ b/arch/powerpc/include/asm/archrandom.h @@ -27,6 +27,15 @@ static inline int arch_get_random_int(unsigned int *v) int powernv_get_random_long(unsigned long *v); +static inline int arch_get_random_seed_long(unsigned long *v) +{ + return 0; +} +static inline int arch_get_random_seed_int(unsigned int *v) +{ + return 0; +} + #endif /* CONFIG_ARCH_RANDOM */ #endif /* _ASM_POWERPC_ARCHRANDOM_H */ diff --git a/arch/x86/include/asm/archrandom.h b/arch/x86/include/asm/archrandom.h index e6a92455740e..6ad7f6d3f97f 100644 --- a/arch/x86/include/asm/archrandom.h +++ b/arch/x86/include/asm/archrandom.h @@ -1,7 +1,7 @@ /* * This file is part of the Linux kernel. * - * Copyright (c) 2011, Intel Corporation + * Copyright (c) 2011-2014, Intel Corporation * Authors: Fenghua Yu , * H. Peter Anvin * @@ -31,10 +31,13 @@ #define RDRAND_RETRY_LOOPS 10 #define RDRAND_INT ".byte 0x0f,0xc7,0xf0" +#define RDSEED_INT ".byte 0x0f,0xc7,0xf8" #ifdef CONFIG_X86_64 # define RDRAND_LONG ".byte 0x48,0x0f,0xc7,0xf0" +# define RDSEED_LONG ".byte 0x48,0x0f,0xc7,0xf8" #else # define RDRAND_LONG RDRAND_INT +# define RDSEED_LONG RDSEED_INT #endif #ifdef CONFIG_ARCH_RANDOM @@ -53,6 +56,16 @@ static inline int rdrand_long(unsigned long *v) return ok; } +/* A single attempt at RDSEED */ +static inline bool rdseed_long(unsigned long *v) +{ + unsigned char ok; + asm volatile(RDSEED_LONG "\n\t" + "setc %0" + : "=qm" (ok), "=a" (*v)); + return ok; +} + #define GET_RANDOM(name, type, rdrand, nop) \ static inline int name(type *v) \ { \ @@ -70,16 +83,35 @@ static inline int name(type *v) \ return ok; \ } +#define GET_SEED(name, type, rdseed, nop) \ +static inline int name(type *v) \ +{ \ + unsigned char ok; \ + alternative_io("movb $0, %0\n\t" \ + nop, \ + rdseed "\n\t" \ + "setc %0", \ + X86_FEATURE_RDSEED, \ + ASM_OUTPUT2("=q" (ok), "=a" (*v))); \ + return ok; \ +} + #ifdef CONFIG_X86_64 GET_RANDOM(arch_get_random_long, unsigned long, RDRAND_LONG, ASM_NOP5); GET_RANDOM(arch_get_random_int, unsigned int, RDRAND_INT, ASM_NOP4); +GET_SEED(arch_get_random_seed_long, unsigned long, RDSEED_LONG, ASM_NOP5); +GET_SEED(arch_get_random_seed_int, unsigned int, RDSEED_INT, ASM_NOP4); + #else GET_RANDOM(arch_get_random_long, unsigned long, RDRAND_LONG, ASM_NOP3); GET_RANDOM(arch_get_random_int, unsigned int, RDRAND_INT, ASM_NOP3); +GET_SEED(arch_get_random_seed_long, unsigned long, RDSEED_LONG, ASM_NOP4); +GET_SEED(arch_get_random_seed_int, unsigned int, RDSEED_INT, ASM_NOP4); + #endif /* CONFIG_X86_64 */ #else @@ -89,6 +121,11 @@ static inline int rdrand_long(unsigned long *v) return 0; } +static inline bool rdseed_long(unsigned long *v) +{ + return 0; +} + #endif /* CONFIG_ARCH_RANDOM */ extern void x86_init_rdrand(struct cpuinfo_x86 *c); diff --git a/include/linux/random.h b/include/linux/random.h index 1cfce0e24dbd..c2f08131050d 100644 --- a/include/linux/random.h +++ b/include/linux/random.h @@ -88,6 +88,14 @@ static inline int arch_get_random_int(unsigned int *v) { return 0; } +static inline int arch_get_random_seed_long(unsigned long *v) +{ + return 0; +} +static inline int arch_get_random_seed_int(unsigned int *v) +{ + return 0; +} #endif /* Pseudo random number generator from numerical recipes. */ From 83664a6928a420b5ccfc0cf23ddbfe3634fea271 Mon Sep 17 00:00:00 2001 From: "H. Peter Anvin" Date: Mon, 17 Mar 2014 16:36:28 -0700 Subject: [PATCH 14/16] random: Use arch_get_random_seed*() at init time and once a second Use arch_get_random_seed*() in two places in the Linux random driver (drivers/char/random.c): 1. During entropy pool initialization, use RDSEED in favor of RDRAND, with a fallback to the latter. Entropy exhaustion is unlikely to happen there on physical hardware as the machine is single-threaded at that point, but could happen in a virtual machine. In that case, the fallback to RDRAND will still provide more than adequate entropy pool initialization. 2. Once a second, issue RDSEED and, if successful, feed it to the entropy pool. To ensure an extra layer of security, only credit half the entropy just in case. Suggested-by: Linus Torvalds Reviewed-by: Ingo Molnar Signed-off-by: H. Peter Anvin Signed-off-by: Theodore Ts'o --- drivers/char/random.c | 24 ++++++++++++++++++++---- 1 file changed, 20 insertions(+), 4 deletions(-) diff --git a/drivers/char/random.c b/drivers/char/random.c index d07575c99a5f..a4bea7775e0f 100644 --- a/drivers/char/random.c +++ b/drivers/char/random.c @@ -844,6 +844,8 @@ void add_interrupt_randomness(int irq, int irq_flags) cycles_t cycles = random_get_entropy(); __u32 input[4], c_high, j_high; __u64 ip; + unsigned long seed; + int credit; c_high = (sizeof(cycles) > 4) ? cycles >> 32 : 0; j_high = (sizeof(now) > 4) ? now >> 32 : 0; @@ -862,20 +864,33 @@ void add_interrupt_randomness(int irq, int irq_flags) r = nonblocking_pool.initialized ? &input_pool : &nonblocking_pool; __mix_pool_bytes(r, &fast_pool->pool, sizeof(fast_pool->pool), NULL); + /* * If we don't have a valid cycle counter, and we see * back-to-back timer interrupts, then skip giving credit for - * any entropy. + * any entropy, otherwise credit 1 bit. */ + credit = 1; if (cycles == 0) { if (irq_flags & __IRQF_TIMER) { if (fast_pool->last_timer_intr) - return; + credit = 0; fast_pool->last_timer_intr = 1; } else fast_pool->last_timer_intr = 0; } - credit_entropy_bits(r, 1); + + /* + * If we have architectural seed generator, produce a seed and + * add it to the pool. For the sake of paranoia count it as + * 50% entropic. + */ + if (arch_get_random_seed_long(&seed)) { + __mix_pool_bytes(r, &seed, sizeof(seed), NULL); + credit += sizeof(seed) * 4; + } + + credit_entropy_bits(r, credit); } #ifdef CONFIG_BLOCK @@ -1235,7 +1250,8 @@ static void init_std_data(struct entropy_store *r) r->last_pulled = jiffies; mix_pool_bytes(r, &now, sizeof(now), NULL); for (i = r->poolinfo->poolbytes; i > 0; i -= sizeof(rv)) { - if (!arch_get_random_long(&rv)) + if (!arch_get_random_seed_long(&rv) && + !arch_get_random_long(&rv)) rv = random_get_entropy(); mix_pool_bytes(r, &rv, sizeof(rv), NULL); } From 331c6490c7f10dcf263712e313b7c0bc7fb6d77a Mon Sep 17 00:00:00 2001 From: "H. Peter Anvin" Date: Mon, 17 Mar 2014 16:36:29 -0700 Subject: [PATCH 15/16] random: If we have arch_get_random_seed*(), try it before blocking If we have arch_get_random_seed*(), try to use it for emergency refill of the entropy pool before giving up and blocking on /dev/random. It may or may not work in the moment, but if it does work, it will give the user better service than blocking will. Reviewed-by: Ingo Molnar Signed-off-by: H. Peter Anvin Signed-off-by: Theodore Ts'o --- drivers/char/random.c | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/drivers/char/random.c b/drivers/char/random.c index a4bea7775e0f..c35cee268e13 100644 --- a/drivers/char/random.c +++ b/drivers/char/random.c @@ -1294,6 +1294,34 @@ void rand_initialize_disk(struct gendisk *disk) } #endif +/* + * Attempt an emergency refill using arch_get_random_seed_long(). + * + * As with add_interrupt_randomness() be paranoid and only + * credit the output as 50% entropic. + */ +static int arch_random_refill(void) +{ + const unsigned int nlongs = 64; /* Arbitrary number */ + unsigned int n = 0; + unsigned int i; + unsigned long buf[nlongs]; + + for (i = 0; i < nlongs; i++) { + if (arch_get_random_seed_long(&buf[n])) + n++; + } + + if (n) { + unsigned int rand_bytes = n * sizeof(unsigned long); + + mix_pool_bytes(&input_pool, buf, rand_bytes, NULL); + credit_entropy_bits(&input_pool, rand_bytes*4); + } + + return n; +} + static ssize_t random_read(struct file *file, char __user *buf, size_t nbytes, loff_t *ppos) { @@ -1312,8 +1340,13 @@ random_read(struct file *file, char __user *buf, size_t nbytes, loff_t *ppos) ENTROPY_BITS(&input_pool)); if (n > 0) return n; + /* Pool is (near) empty. Maybe wait and retry. */ + /* First try an emergency refill */ + if (arch_random_refill()) + continue; + if (file->f_flags & O_NONBLOCK) return -EAGAIN; From 7b878d4b48c4e04b936918bb83836a107ba453b3 Mon Sep 17 00:00:00 2001 From: "H. Peter Anvin" Date: Mon, 17 Mar 2014 16:36:30 -0700 Subject: [PATCH 16/16] random: Add arch_has_random[_seed]() Add predicate functions for having arch_get_random[_seed]*(). The only current use is to avoid the loop in arch_random_refill() when arch_get_random_seed_long() is unavailable. Signed-off-by: H. Peter Anvin Cc: Benjamin Herrenschmidt Cc: Paul Mackerras Cc: Michael Ellerman Signed-off-by: Theodore Ts'o --- arch/powerpc/include/asm/archrandom.h | 9 +++++++++ arch/x86/include/asm/archrandom.h | 3 +++ drivers/char/random.c | 3 +++ include/linux/random.h | 8 ++++++++ 4 files changed, 23 insertions(+) diff --git a/arch/powerpc/include/asm/archrandom.h b/arch/powerpc/include/asm/archrandom.h index 801beba4e64b..bde531103638 100644 --- a/arch/powerpc/include/asm/archrandom.h +++ b/arch/powerpc/include/asm/archrandom.h @@ -25,6 +25,11 @@ static inline int arch_get_random_int(unsigned int *v) return rc; } +static inline int arch_has_random(void) +{ + return !!ppc_md.get_random_long; +} + int powernv_get_random_long(unsigned long *v); static inline int arch_get_random_seed_long(unsigned long *v) @@ -35,6 +40,10 @@ static inline int arch_get_random_seed_int(unsigned int *v) { return 0; } +static inline int arch_has_random_seed(void) +{ + return 0; +} #endif /* CONFIG_ARCH_RANDOM */ diff --git a/arch/x86/include/asm/archrandom.h b/arch/x86/include/asm/archrandom.h index 6ad7f6d3f97f..69f1366f1aa3 100644 --- a/arch/x86/include/asm/archrandom.h +++ b/arch/x86/include/asm/archrandom.h @@ -114,6 +114,9 @@ GET_SEED(arch_get_random_seed_int, unsigned int, RDSEED_INT, ASM_NOP4); #endif /* CONFIG_X86_64 */ +#define arch_has_random() static_cpu_has(X86_FEATURE_RDRAND) +#define arch_has_random_seed() static_cpu_has(X86_FEATURE_RDSEED) + #else static inline int rdrand_long(unsigned long *v) diff --git a/drivers/char/random.c b/drivers/char/random.c index c35cee268e13..6b75713d953a 100644 --- a/drivers/char/random.c +++ b/drivers/char/random.c @@ -1307,6 +1307,9 @@ static int arch_random_refill(void) unsigned int i; unsigned long buf[nlongs]; + if (!arch_has_random_seed()) + return 0; + for (i = 0; i < nlongs; i++) { if (arch_get_random_seed_long(&buf[n])) n++; diff --git a/include/linux/random.h b/include/linux/random.h index c2f08131050d..57fbbffd77a0 100644 --- a/include/linux/random.h +++ b/include/linux/random.h @@ -88,6 +88,10 @@ static inline int arch_get_random_int(unsigned int *v) { return 0; } +static inline int arch_has_random(void) +{ + return 0; +} static inline int arch_get_random_seed_long(unsigned long *v) { return 0; @@ -96,6 +100,10 @@ static inline int arch_get_random_seed_int(unsigned int *v) { return 0; } +static inline int arch_has_random_seed(void) +{ + return 0; +} #endif /* Pseudo random number generator from numerical recipes. */