lkml.org 
[lkml]   [2008]   [Sep]   [3]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
Subject[patch 16/42] drivers/char/random.c: fix a race which can lead to a bogus BUG()
2.6.26-stable review patch.  If anyone has any objections, please let us know.

------------------

From: Andrew Morton <akpm@linux-foundation.org>

commit 8b76f46a2db29407fed66cf4aca19d61b3dcb3e1 upstream

Fix a bug reported by and diagnosed by Aaron Straus.

This is a regression intruduced into 2.6.26 by

commit adc782dae6c4c0f6fb679a48a544cfbcd79ae3dc
Author: Matt Mackall <mpm@selenic.com>
Date: Tue Apr 29 01:03:07 2008 -0700

random: simplify and rename credit_entropy_store

credit_entropy_bits() does:

spin_lock_irqsave(&r->lock, flags);
...
if (r->entropy_count > r->poolinfo->POOLBITS)
r->entropy_count = r->poolinfo->POOLBITS;

so there is a time window in which this BUG_ON():

static size_t account(struct entropy_store *r, size_t nbytes, int min,
int reserved)
{
unsigned long flags;

BUG_ON(r->entropy_count > r->poolinfo->POOLBITS);

/* Hold lock while accounting */
spin_lock_irqsave(&r->lock, flags);

can trigger.

We could fix this by moving the assertion inside the lock, but it seems
safer and saner to revert to the old behaviour wherein
entropy_store.entropy_count at no time exceeds
entropy_store.poolinfo->POOLBITS.

Reported-by: Aaron Straus <aaron@merfinllc.com>
Cc: Matt Mackall <mpm@selenic.com>
Cc: Theodore Ts'o <tytso@mit.edu>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>

---
drivers/char/random.c | 19 ++++++++++---------
1 file changed, 10 insertions(+), 9 deletions(-)

--- a/drivers/char/random.c
+++ b/drivers/char/random.c
@@ -406,7 +406,7 @@ struct entropy_store {
/* read-write data: */
spinlock_t lock;
unsigned add_ptr;
- int entropy_count;
+ int entropy_count; /* Must at no time exceed ->POOLBITS! */
int input_rotate;
};

@@ -519,6 +519,7 @@ static void mix_pool_bytes(struct entrop
static void credit_entropy_bits(struct entropy_store *r, int nbits)
{
unsigned long flags;
+ int entropy_count;

if (!nbits)
return;
@@ -526,20 +527,20 @@ static void credit_entropy_bits(struct e
spin_lock_irqsave(&r->lock, flags);

DEBUG_ENT("added %d entropy credits to %s\n", nbits, r->name);
- r->entropy_count += nbits;
- if (r->entropy_count < 0) {
+ entropy_count = r->entropy_count;
+ entropy_count += nbits;
+ if (entropy_count < 0) {
DEBUG_ENT("negative entropy/overflow\n");
- r->entropy_count = 0;
- } else if (r->entropy_count > r->poolinfo->POOLBITS)
- r->entropy_count = r->poolinfo->POOLBITS;
+ entropy_count = 0;
+ } else if (entropy_count > r->poolinfo->POOLBITS)
+ entropy_count = r->poolinfo->POOLBITS;
+ r->entropy_count = entropy_count;

/* should we wake readers? */
- if (r == &input_pool &&
- r->entropy_count >= random_read_wakeup_thresh) {
+ if (r == &input_pool && entropy_count >= random_read_wakeup_thresh) {
wake_up_interruptible(&random_read_wait);
kill_fasync(&fasync, SIGIO, POLL_IN);
}
-
spin_unlock_irqrestore(&r->lock, flags);
}

--


\
 
 \ /
  Last update: 2008-09-03 19:43    [W:0.316 / U:0.308 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site