Re: [PATCH v6 1/5] random: Blocking API for accessing nonblocking_pool
From: Stephan Mueller <hidden>
Date: 2015-05-18 05:32:51
Also in:
lkml
Subsystem:
random number driver, the rest · Maintainers:
"Theodore Ts'o", Jason A. Donenfeld, Linus Torvalds
Am Freitag, 15. Mai 2015, 14:46:26 schrieb Herbert Xu: Hi Herbert,
On Wed, May 13, 2015 at 09:54:41PM +0200, Stephan Mueller wrote:quoted
/* + * Equivalent function to get_random_bytes with the difference that this + * function blocks the request until the nonblocking_pool is initialized. + */ +void get_blocking_random_bytes(void *buf, int nbytes) +{ + if (unlikely(nonblocking_pool.initialized == 0)) + wait_event_interruptible(urandom_init_wait, + nonblocking_pool.initialized); + extract_entropy(&nonblocking_pool, buf, nbytes, 0, 0);So what if the wait was interrupted? You are going to extract entropy from an empty pool. Anyway, you still haven't addressed my primary concern with this model which is the potential for dead-lock. Sleeping for an open period of time like this in a work queue is bad form. It may also lead to dead-locks if whatever you're waiting for happened to use the same work thread. That's why I think you should simply provide a function and data pointer which random.c can then stash onto a list to call when the pool is ready.
Thanks for the hint to the list. Before handing in another formal patch, may i ask for checking the following approach? I would think that this one should cover your concerns.
diff --git a/drivers/char/random.c b/drivers/char/random.c
index 9cd6968..9bc2a57 100644
--- a/drivers/char/random.c
+++ b/drivers/char/random.c@@ -409,6 +409,19 @@ static DECLARE_WAIT_QUEUE_HEAD(random_write_wait); static DECLARE_WAIT_QUEUE_HEAD(urandom_init_wait); static struct fasync_struct *fasync; +static LIST_HEAD(random_wait_list); +static DEFINE_MUTEX(random_wait_list_mutex); +struct random_work { + struct list_head list; + struct work_struct rw_work; + void *rw_buf; + int rw_len; + void *rw_private; + void (*rw_cb)(void *buf, int buflen, + void *private); +}; +static void process_random_waiters(void); + /********************************************************************** * * OS independent entropy store. Here are the functions which handle
@@ -660,6 +673,7 @@ retry: r->entropy_total = 0; if (r == &nonblocking_pool) { prandom_reseed_late(); + process_random_waiters(); wake_up_interruptible(&urandom_init_wait); pr_notice("random: %s pool is initialized\n", r->name); }
@@ -1778,3 +1792,64 @@ void add_hwgenerator_randomness(const char *buffer, size_t count, credit_entropy_bits(poolp, entropy); } EXPORT_SYMBOL_GPL(add_hwgenerator_randomness); + +static void process_random_waiters(void) +{ + struct random_work *rw = NULL; + + mutex_lock(&random_wait_list_mutex); + while (!list_empty(&random_wait_list)) { + rw = list_first_entry(&random_wait_list, struct random_work, + list); + list_del(&rw->list); + schedule_work(&rw->rw_work); + } + mutex_unlock(&random_wait_list_mutex); +} + +static void get_blocking_random_bytes_work(struct work_struct *work) +{ + struct random_work *rw = container_of(work, struct random_work, + rw_work); + + get_random_bytes(rw->rw_buf, rw->rw_len); + rw->rw_cb(rw->rw_buf, rw->rw_len, rw->rw_private); + kfree(rw); +} + +/* + * Equivalent function to get_random_bytes with the difference that this + * function blocks the request until the nonblocking_pool is initialized. + */ +int get_blocking_random_bytes_cb(void *buf, int nbytes, void *private, + void (*cb)(void *buf, int buflen, + void *private)) +{ + struct random_work *rw = NULL; + int ret = 0; + + mutex_lock(&random_wait_list_mutex); + list_for_each_entry(rw, &random_wait_list, list) + if (buf == rw->rw_buf) + goto out; + + rw = kmalloc(sizeof(struct random_work), GFP_KERNEL); + if (!rw) { + ret = -ENOMEM; + goto out; + } + INIT_WORK(&rw->rw_work, get_blocking_random_bytes_work); + rw->rw_buf = buf; + rw->rw_len = nbytes; + rw->rw_private = private; + rw->rw_cb = cb; + list_add_tail(&rw->list, &random_wait_list); + +out: + mutex_unlock(&random_wait_list_mutex); + if (nonblocking_pool.initialized) + process_random_waiters(); + + return ret; +} +EXPORT_SYMBOL(get_blocking_random_bytes_cb);
diff --git a/include/linux/random.h b/include/linux/random.h
index b05856e..b57525f 100644
--- a/include/linux/random.h
+++ b/include/linux/random.h@@ -15,6 +15,9 @@ extern void add_interrupt_randomness(int irq, int irq_flags); extern void get_random_bytes(void *buf, int nbytes); extern void get_random_bytes_arch(void *buf, int nbytes); +extern int get_blocking_random_bytes_cb(void *buf, int nbytes, void *private, + void (*cb)(void *buf, int buflen, + void *private)); void generate_random_uuid(unsigned char uuid_out[16]); extern int random_int_secret_init(void);
--
2.1.0
--
Ciao
Stephan