Rework random blocking
Subsystems (3)
v2 cover Rework random blocking
This makes two major semantic changes to Linux's random APIs:
It adds getentropy(..., GRND_INSECURE). This causes getentropy to always return *something*. There is no guarantee whatsoever that the result will be cryptographically random or even unique, but the kernel will give the best quality random output it can. The name is a big hint: the resulting output is INSECURE.
The purpose of this is to allow programs that genuinely want best-effort entropy to get it without resorting to /dev/urandom. Plenty of programs do this because they need to do *something* during boot and they can't afford to wait. Calling it "INSECURE" is probably the best we can do to discourage using this API for things that need security.
This series also removes the blocking pool and makes /dev/random work just like getentropy(..., 0) and makes GRND_RANDOM a no-op. I believe that Linux's blocking pool has outlived its usefulness. Linux's CRNG generates output that is good enough to use even for key generation. The blocking pool is not stronger in any material way, and keeping it around requires a lot of infrastructure of dubious value.
This series should not break any existing programs. /dev/urandom is unchanged. /dev/random will still block just after booting, but it will block less than it used to. getentropy() with existing flags will return output that is, for practical purposes, just as strong as before.
Changes from v1:
- Rebased to v5.3. No other changes.
Andy Lutomirski (7):
random: Don't wake crng_init_wait when crng_init == 1
random: Add GRND_INSECURE to return best-effort non-cryptographic
bytes
random: Ignore GRND_RANDOM in getentropy(2)
random: Make /dev/random be almost like /dev/urandom
random: Remove the blocking pool
random: Delete code to pull data into pools
random: Remove kernel.random.read_wakeup_threshold
drivers/char/random.c | 234 ++++-------------------------------- include/uapi/linux/random.h | 4 +- 2 files changed, 27 insertions(+), 211 deletions(-)
Signature · 1 line
2.21.0
PATCH v2 1/7 random: Don't wake crng_init_wait when crng_init == 1
crng_init_wait is only used to wayt for crng_init to be set to 2, so there's no point to waking it when crng_init is set to 1. Remove the unnecessary wake_up_interruptible() call.
--- drivers/char/random.c | 1 - 1 file changed, 1 deletion(-)
drivers/char/random.c +0 -1
@@ -946,7 +946,6 @@ static int crng_fast_load(const char *cp, size_t len)if (crng_init_cnt >=CRNG_INIT_CNT_THRESH ) {invalidate_batched_entropy (); crng_init =1 ; -wake_up_interruptible (&crng_init_wait);pr_notice ("random: fast init done\n" ); }return 1 ;
Signature · 1 line
2.21.0
PATCH v2 2/7 random: Add GRND_INSECURE to return best-effort non-cryptographic bytes
In reply to Andy Lutomirski--- drivers/char/random.c | 11 +++++++++-- include/uapi/linux/random.h | 2 ++ 2 files changed, 11 insertions(+), 2 deletions(-)
drivers/char/random.c +9 -2
@@ -2122,7 +2122,14 @@ SYSCALL_DEFINE3(getrandom, char __user *, buf, size_t, count, {int ret; -if (flags & ~(GRND_NONBLOCK |GRND_RANDOM )) +if (flags & ~(GRND_NONBLOCK |GRND_RANDOM |GRND_INSECURE )) +return -EINVAL ; + +/* +* Requesting insecure and blocking randomness at the same time makes +* no sense. +*/ +if ((flags & (GRND_INSECURE |GRND_RANDOM )) == (GRND_INSECURE |GRND_RANDOM ))return -EINVAL ;if (count >INT_MAX )
@@ -2131,7 +2138,7 @@ SYSCALL_DEFINE3(getrandom, char __user *, buf, size_t, count,if (flags &GRND_RANDOM )return _random_read (flags &GRND_NONBLOCK , buf, count); -if (!crng_ready ()) { +if (!(flags &GRND_INSECURE ) && !crng_ready ()) {if (flags &GRND_NONBLOCK )return -EAGAIN ; ret =wait_for_random_bytes ();
include/uapi/linux/random.h +2 -0
@@ -49,8 +49,10 @@ struct rand_pool_info {* * GRND_NONBLOCK Don't block and return EAGAIN instead * GRND_RANDOM Use the /dev/random pool instead of /dev/urandom +* GRND_INSECURE Return non-cryptographic random bytes */ #define GRND_NONBLOCK 0x0001#define GRND_RANDOM 0x0002 +#define GRND_INSECURE 0x0004#endif /* _UAPI_LINUX_RANDOM_H */
Signature · 1 line
2.21.0
PATCH v2 3/7 random: Ignore GRND_RANDOM in getentropy(2)
In reply to Andy LutomirskiThe separate blocking pool is going away. Start by ignoring GRND_RANDOM in getentropy(2).
This should not materially break any API. Any code that worked without this change should work at least as well with this change.
--- drivers/char/random.c | 3 --- include/uapi/linux/random.h | 2 +- 2 files changed, 1 insertion(+), 4 deletions(-)
drivers/char/random.c +0 -3
@@ -2135,9 +2135,6 @@ SYSCALL_DEFINE3(getrandom, char __user *, buf, size_t, count,if (count >INT_MAX ) count =INT_MAX ; -if (flags &GRND_RANDOM ) -return _random_read (flags &GRND_NONBLOCK , buf, count); -if (!(flags &GRND_INSECURE ) && !crng_ready ()) {if (flags &GRND_NONBLOCK )return -EAGAIN ;
include/uapi/linux/random.h +1 -1
@@ -48,7 +48,7 @@ struct rand_pool_info {* Flags for getrandom(2) * * GRND_NONBLOCK Don't block and return EAGAIN instead -* GRND_RANDOM Use the /dev/random pool instead of /dev/urandom +* GRND_RANDOM No effect * GRND_INSECURE Return non-cryptographic random bytes */ #define GRND_NONBLOCK 0x0001
Signature · 1 line
2.21.0
PATCH v2 4/7 random: Make /dev/random be almost like /dev/urandom
In reply to Andy LutomirskiThis patch changes the read semantics of /dev/random to be the same as /dev/urandom except that reads will block until the CRNG is ready.
None of the cleanups that this enables have been done yet. As a result, this gives a warning about an unused function.
--- drivers/char/random.c | 55 +++++++++++-------------------------------- 1 file changed, 14 insertions(+), 41 deletions(-)
drivers/char/random.c +14 -41
@@ -354,7 +354,6 @@#define INPUT_POOL_WORDS (1 << (INPUT_POOL_SHIFT-5))#define OUTPUT_POOL_SHIFT 10#define OUTPUT_POOL_WORDS (1 << (OUTPUT_POOL_SHIFT-5)) -#define SEC_XFER_SIZE 512#define EXTRACT_SIZE 10
@@ -803,7 +802,6 @@ static void credit_entropy_bits(struct entropy_store *r, int nbits)if (entropy_bits >= random_read_wakeup_bits &&wq_has_sleeper (&random_read_wait)) {wake_up_interruptible (&random_read_wait); -kill_fasync (&fasync,SIGIO ,POLL_IN ); }/* If the input pool is getting full, and the blocking * pool has room, send some entropy to the blocking
@@ -1031,6 +1029,7 @@ static void crng_reseed(struct crng_state *crng, struct entropy_store *r) crng_init =2 ;process_random_ready_list ();wake_up_interruptible (&crng_init_wait); +kill_fasync (&fasync,SIGIO ,POLL_IN );pr_notice ("random: crng init done\n" );if (unseeded_warning.missed) { pr_notice("random: %d get_random_xx warning(s) missed "
@@ -1921,43 +1920,6 @@ void rand_initialize_disk(struct gendisk *disk) }#endif -static ssize_t -_random_read (int nonblock,char __user *buf,size_t nbytes) -{ -ssize_t n; - -if (nbytes ==0 ) -return 0 ; - - 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 (nonblock) -return -EAGAIN ; - -wait_event_interruptible (random_read_wait, - blocking_pool.initialized && - (ENTROPY_BITS (&input_pool) >= random_read_wakeup_bits)); -if (signal_pending (current)) -return -ERESTARTSYS ; - } -} - -static ssize_t -random_read (struct file *file,char __user *buf,size_t nbytes,loff_t *ppos) -{ -return _random_read (file->f_flags &O_NONBLOCK , buf, nbytes); -} -static ssize_t urandom_read (struct file *file,char __user *buf,size_t nbytes,loff_t *ppos) {
@@ -1981,15 +1943,26 @@ urandom_read(struct file *file, char __user *buf, size_t nbytes, loff_t *ppos)return ret; } +static ssize_t +random_read (struct file *file,char __user *buf,size_t nbytes,loff_t *ppos) +{ +int ret; + + ret =wait_for_random_bytes (); +if (ret !=0 ) +return ret; +return urandom_read (file, buf, nbytes, ppos); +} +static __poll_t random_poll (struct file *file,poll_table * wait) {__poll_t mask; -poll_wait (file, &random_read_wait, wait); +poll_wait (file, &crng_init_wait, wait);poll_wait (file, &random_write_wait, wait); mask =0 ; -if (ENTROPY_BITS (&input_pool) >= random_read_wakeup_bits) +if (crng_ready ()) mask |=EPOLLIN |EPOLLRDNORM ;if (ENTROPY_BITS (&input_pool) < random_write_wakeup_bits) mask |=EPOLLOUT |EPOLLWRNORM ;
Signature · 1 line
2.21.0
PATCH v2 5/7 random: Remove the blocking pool
In reply to Andy LutomirskiThere is no longer any interface to read data from the blocking pool, so remove it.
This enables quite a bit of code deletion, much of which will be done in subsequent patches.
--- drivers/char/random.c | 106 ------------------------------------------ 1 file changed, 106 deletions(-)
drivers/char/random.c +0 -106
@@ -470,7 +470,6 @@ static const struct poolinfo {/* * Static global variables */ -static DECLARE_WAIT_QUEUE_HEAD (random_read_wait);static DECLARE_WAIT_QUEUE_HEAD (random_write_wait);static struct fasync_struct *fasync;
@@ -530,7 +529,6 @@ struct entropy_store {__u32 *pool;const char *name;struct entropy_store *pull; -struct work_struct push_work;/* read-write data: */ unsigned long last_pulled;
@@ -549,9 +547,7 @@ static ssize_t _extract_entropy(struct entropy_store *r, void *buf, size_t nbytes, int fips);static void crng_reseed (struct crng_state *crng,struct entropy_store *r); -static void push_to_pool (struct work_struct *work); static __u32 input_pool_data[INPUT_POOL_WORDS] __latent_entropy; -static __u32 blocking_pool_data[OUTPUT_POOL_WORDS] __latent_entropy;static struct entropy_store input_pool = { .poolinfo = &poolinfo_table[0 ],
@@ -560,16 +556,6 @@ static struct entropy_store input_pool = { .pool = input_pool_data }; -static struct entropy_store blocking_pool = { - .poolinfo = &poolinfo_table[1 ], - .name ="blocking" , - .pull = &input_pool, - .lock =__SPIN_LOCK_UNLOCKED (blocking_pool.lock), - .pool = blocking_pool_data, - .push_work =__WORK_INITIALIZER (blocking_pool.push_work, - push_to_pool), -}; -static __u32 const twist_table[8 ] = {0x00000000 ,0x3b6e20c8 ,0x76dc4190 ,0x4db26158 ,0xedb88320 ,0xd6d6a3e8 ,0x9b64c2b0 ,0xa00ae278 };
@@ -765,15 +751,11 @@ static void credit_entropy_bits(struct entropy_store *r, int nbits) entropy_count =0 ; }else if (entropy_count > pool_size) entropy_count = pool_size; -if ((r == &blocking_pool) && !r->initialized && - (entropy_count >>ENTROPY_SHIFT ) >128 ) - has_initialized =1 ;if (cmpxchg (&r->entropy_count, orig, entropy_count) != orig) goto retry;if (has_initialized) { r->initialized =1 ; -wake_up_interruptible (&random_read_wait);kill_fasync (&fasync,SIGIO ,POLL_IN ); }
@@ -782,7 +764,6 @@ static void credit_entropy_bits(struct entropy_store *r, int nbits)if (r == &input_pool) {int entropy_bits = entropy_count >>ENTROPY_SHIFT ; -struct entropy_store *other = &blocking_pool;if (crng_init <2 ) {if (entropy_bits <128 )
@@ -790,27 +771,6 @@ static void credit_entropy_bits(struct entropy_store *r, int nbits)crng_reseed (&primary_crng, r); entropy_bits = r->entropy_count >>ENTROPY_SHIFT ; } - -/* initialize the blocking pool if necessary */ -if (entropy_bits >= random_read_wakeup_bits && - !other->initialized) { -schedule_work (&other->push_work); -return ; - } - -/* should we wake readers? */ -if (entropy_bits >= random_read_wakeup_bits && -wq_has_sleeper (&random_read_wait)) { -wake_up_interruptible (&random_read_wait); - } -/* If the input pool is getting full, and the blocking -* pool has room, send some entropy to the blocking -* pool. -*/ -if (!work_pending (&other->push_work) && - (ENTROPY_BITS (r) >6 * r->poolinfo->poolbytes) && - (ENTROPY_BITS (other) <=6 * other->poolinfo->poolbytes)) -schedule_work (&other->push_work); } }
@@ -1422,22 +1382,6 @@ static void _xfer_secondary_pool(struct entropy_store *r, size_t nbytes)credit_entropy_bits (r, bytes*8 ); } -/* -* Used as a workqueue function so that when the input pool is getting -* full, we can "spill over" some entropy to the output pools. That -* way the output pools can store some of the excess entropy instead -* of letting it go to waste. -*/ -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_bits/8 ); -trace_push_to_pool (r->name, r->entropy_count >>ENTROPY_SHIFT , - r->pull->entropy_count >>ENTROPY_SHIFT ); -} -/* * This function decides how many bytes to actually take from the * given pool, and also debits the entropy count accordingly.
@@ -1616,54 +1560,6 @@ static ssize_t extract_entropy(struct entropy_store *r, void *buf,return _extract_entropy (r, buf, nbytes, fips_enabled); } -/* -* 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) -{ -ssize_t ret =0 , i; -__u8 tmp[EXTRACT_SIZE ]; -int large_request = (nbytes >256 ); - -trace_extract_entropy_user (r->name, nbytes,ENTROPY_BITS (r), _RET_IP_); -if (!r->initialized && r->pull) { -xfer_secondary_pool (r,ENTROPY_BITS (r->pull)/8 ); -if (!r->initialized) -return 0 ; - } -xfer_secondary_pool (r, nbytes); - nbytes =account (r, nbytes,0 ,0 ); - -while (nbytes) { -if (large_request &&need_resched ()) { -if (signal_pending (current)) { -if (ret ==0 ) - ret = -ERESTARTSYS ; -break ; - } -schedule (); - } - -extract_buf (r, tmp); - i =min_t (int, nbytes,EXTRACT_SIZE ); -if (copy_to_user (buf, tmp, i)) { - ret = -EFAULT ; -break ; - } - - nbytes -= i; - buf += i; - ret += i; - } - -/* Wipe data just returned from memory */ -memzero_explicit (tmp,sizeof (tmp)); - -return ret; -} -#define warn_unseeded_randomness (previous) \ _warn_unseeded_randomness(__func__, (void *) _RET_IP_, (previous))
@@ -1893,7 +1789,6 @@ static void __init init_std_data(struct entropy_store *r)int __init rand_initialize (void ) {init_std_data (&input_pool); -init_std_data (&blocking_pool);crng_initialize (&primary_crng); crng_global_init_time = jiffies;if (ratelimit_disable) {
@@ -2053,7 +1948,6 @@ static long random_ioctl(struct file *f, unsigned int cmd, unsigned long arg)if (!capable (CAP_SYS_ADMIN ))return -EPERM ; input_pool.entropy_count =0 ; - blocking_pool.entropy_count =0 ;return 0 ;case RNDRESEEDCRNG :if (!capable (CAP_SYS_ADMIN ))
Signature · 1 line
2.21.0
PATCH v2 6/7 random: Delete code to pull data into pools
In reply to Andy LutomirskiThere is no pool that pulls, so it was just dead code.
--- drivers/char/random.c | 40 ---------------------------------------- 1 file changed, 40 deletions(-)
drivers/char/random.c +0 -40
@@ -528,10 +528,8 @@ struct entropy_store {const struct poolinfo *poolinfo;__u32 *pool;const char *name; -struct entropy_store *pull;/* read-write data: */ -unsigned long last_pulled;spinlock_t lock;unsigned short add_ptr;unsigned short input_rotate;
@@ -1347,41 +1345,6 @@ EXPORT_SYMBOL_GPL(add_disk_randomness);* *********************************************************************/ -/* -* This utility inline function is responsible for transferring entropy -* from the primary pool to the secondary extraction pool. We make -* sure we pull enough for a 'catastrophic reseed'. -*/ -static void _xfer_secondary_pool (struct entropy_store *r,size_t nbytes); -static void xfer_secondary_pool (struct entropy_store *r,size_t nbytes) -{ -if (!r->pull || - r->entropy_count >= (nbytes << (ENTROPY_SHIFT +3 )) || - r->entropy_count > r->poolinfo->poolfracbits) -return ; - -_xfer_secondary_pool (r, nbytes); -} - -static void _xfer_secondary_pool (struct entropy_store *r,size_t nbytes) -{ -__u32 tmp[OUTPUT_POOL_WORDS ]; - -int bytes = nbytes; - -/* 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_bits /8 ,0 ); -mix_pool_bytes (r, tmp, bytes); -credit_entropy_bits (r, bytes*8 ); -} -/* * This function decides how many bytes to actually take from the * given pool, and also debits the entropy count accordingly.
@@ -1545,7 +1508,6 @@ static ssize_t extract_entropy(struct entropy_store *r, void *buf,spin_unlock_irqrestore (&r->lock, flags);trace_extract_entropy (r->name,EXTRACT_SIZE ,ENTROPY_BITS (r), _RET_IP_); -xfer_secondary_pool (r,EXTRACT_SIZE );extract_buf (r, tmp);spin_lock_irqsave (&r->lock, flags);memcpy (r->last_data, tmp,EXTRACT_SIZE );
@@ -1554,7 +1516,6 @@ static ssize_t extract_entropy(struct entropy_store *r, void *buf, }trace_extract_entropy (r->name, nbytes,ENTROPY_BITS (r), _RET_IP_); -xfer_secondary_pool (r, nbytes); nbytes =account (r, nbytes, min, reserved);return _extract_entropy (r, buf, nbytes, fips_enabled);
@@ -1765,7 +1726,6 @@ static void __init init_std_data(struct entropy_store *r)ktime_t now =ktime_get_real ();unsigned long rv; - r->last_pulled = jiffies;mix_pool_bytes (r, &now,sizeof (now));for (i = r->poolinfo->poolbytes; i >0 ; i -=sizeof (rv)) { if (!arch_get_random_seed_long(&rv) &&
Signature · 1 line
2.21.0
PATCH v2 7/7 random: Remove kernel.random.read_wakeup_threshold
In reply to Andy LutomirskiIt has no effect any more, so remove it. We can revert this if there is some user code that expects to be able to set this sysctl.
--- drivers/char/random.c | 18 +----------------- 1 file changed, 1 insertion(+), 17 deletions(-)
drivers/char/random.c +1 -17
@@ -369,12 +369,6 @@#define ENTROPY_SHIFT 3#define ENTROPY_BITS (r) ((r)->entropy_count >> ENTROPY_SHIFT) -/* -* 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_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
@@ -1982,8 +1976,7 @@ SYSCALL_DEFINE3(getrandom, char __user *, buf, size_t, count,#include <linux/sysctl.h> -static int min_read_thresh =8 , min_write_thresh; -static int max_read_thresh =OUTPUT_POOL_WORDS *32 ; +static int min_write_thresh;static int max_write_thresh =INPUT_POOL_WORDS *32 ;static int random_min_urandom_seed =60 ;static char sysctl_bootid[16 ];
@@ -2058,15 +2051,6 @@ struct ctl_table random_table[] = { .proc_handler = proc_do_entropy, .data = &input_pool.entropy_count, }, - { - .procname = "read_wakeup_threshold", - .data = &random_read_wakeup_bits, - .maxlen = sizeof(int), - .mode = 0644, - .proc_handler = proc_dointvec_minmax, - .extra1 = &min_read_thresh, - .extra2 = &max_read_thresh, - }, { .procname = "write_wakeup_threshold", .data = &random_write_wakeup_bits,
Signature · 1 line
2.21.0
Re: [PATCH v2 7/7] random: Remove kernel.random.read_wakeup_threshold
Quoted 9 lines · Andy Lutomirski
It has no effect any more, so remove it. We can revert this if there is some user code that expects to be able to set this sysctl.
Signed-off-by Andy Lutomirski <[email protected]>--- drivers/char/random.c | 18 +----------------- 1 file changed, 1 insertion(+), 17 deletions(-)drivers/char/random.c +0
-0
[...]
drivers/char/random.c struct ctl_table random_table[] = { · line 2062
Quoted 2 lines
drivers/char/random.c +0
-2- { - .procname = "read_wakeup_threshold",
There's a line in bin_random_table in kernel/sysctl_binary.c that refers to this sysctl, that should probably also be deleted?
Re: [PATCH v2 7/7] random: Remove kernel.random.read_wakeup_threshold
Quoted 9 lines · Jann Horn
Quoted 8 lines · Andy Lutomirski
It has no effect any more, so remove it. We can revert this if there is some user code that expects to be able to set this sysctl.
Signed-off-by Andy Lutomirski <[email protected]>--- drivers/char/random.c | 18 +----------------- 1 file changed, 1 insertion(+), 17 deletions(-)
Quoted 7 lines
Quoted 1 line
drivers/char/random.c +0
-0[...]
drivers/char/random.c struct ctl_table random_table[] = { · line 2062
Quoted 2 lines
drivers/char/random.c +0
-2- { - .procname = "read_wakeup_threshold",There's a line in
bin_random_tableinkernel/sysctl_binary.cthat refers to this sysctl, that should probably also be deleted?
I think it should be safe to leave in kernel/sysctl_binary.c
This reminds me. I think we may finally be at a point where we can remove practically all of kernel/sysctl_binary.c
I need to double check but last I looked no distro enables COINFIG_SYSCTL_SYSCALL anymore. Ubunutu was the last distro I know of that enabled it, and I think it has been a year or more since Ubuntu disabled CONFIG_SYSCTL_SYSCALL.
Eric
Quoted 2 lines · Andy Lutomirski
This makes two major semantic changes to Linux's random APIs:
Hmm. I have at least two issues in here that need fixing:
- getrandom() can warn. Whoops.
- Repeatedly calling getentropy in GRND_INSECURE mode will interfere
will CRNG init.
v3 will fix these issues.