diff options
author | Andrew Morton <akpm@linux-foundation.org> | 2008-09-02 17:36:14 -0400 |
---|---|---|
committer | Linus Torvalds <torvalds@linux-foundation.org> | 2008-09-02 22:21:40 -0400 |
commit | 8b76f46a2db29407fed66cf4aca19d61b3dcb3e1 (patch) | |
tree | f908592be7c11065b56573722a71dc468817b925 /drivers | |
parent | 9d3593574702ae1899e23a1535da1ac71f928042 (diff) |
drivers/char/random.c: fix a race which can lead to a bogus BUG()
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>
Cc: <stable@kernel.org> [2.6.26.x]
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Diffstat (limited to 'drivers')
-rw-r--r-- | drivers/char/random.c | 19 |
1 files changed, 10 insertions, 9 deletions
diff --git a/drivers/char/random.c b/drivers/char/random.c index 1838aa3d24fe..7ce1ac4baa6d 100644 --- a/drivers/char/random.c +++ b/drivers/char/random.c | |||
@@ -407,7 +407,7 @@ struct entropy_store { | |||
407 | /* read-write data: */ | 407 | /* read-write data: */ |
408 | spinlock_t lock; | 408 | spinlock_t lock; |
409 | unsigned add_ptr; | 409 | unsigned add_ptr; |
410 | int entropy_count; | 410 | int entropy_count; /* Must at no time exceed ->POOLBITS! */ |
411 | int input_rotate; | 411 | int input_rotate; |
412 | }; | 412 | }; |
413 | 413 | ||
@@ -520,6 +520,7 @@ static void mix_pool_bytes(struct entropy_store *r, const void *in, int bytes) | |||
520 | static void credit_entropy_bits(struct entropy_store *r, int nbits) | 520 | static void credit_entropy_bits(struct entropy_store *r, int nbits) |
521 | { | 521 | { |
522 | unsigned long flags; | 522 | unsigned long flags; |
523 | int entropy_count; | ||
523 | 524 | ||
524 | if (!nbits) | 525 | if (!nbits) |
525 | return; | 526 | return; |
@@ -527,20 +528,20 @@ static void credit_entropy_bits(struct entropy_store *r, int nbits) | |||
527 | spin_lock_irqsave(&r->lock, flags); | 528 | spin_lock_irqsave(&r->lock, flags); |
528 | 529 | ||
529 | DEBUG_ENT("added %d entropy credits to %s\n", nbits, r->name); | 530 | DEBUG_ENT("added %d entropy credits to %s\n", nbits, r->name); |
530 | r->entropy_count += nbits; | 531 | entropy_count = r->entropy_count; |
531 | if (r->entropy_count < 0) { | 532 | entropy_count += nbits; |
533 | if (entropy_count < 0) { | ||
532 | DEBUG_ENT("negative entropy/overflow\n"); | 534 | DEBUG_ENT("negative entropy/overflow\n"); |
533 | r->entropy_count = 0; | 535 | entropy_count = 0; |
534 | } else if (r->entropy_count > r->poolinfo->POOLBITS) | 536 | } else if (entropy_count > r->poolinfo->POOLBITS) |
535 | r->entropy_count = r->poolinfo->POOLBITS; | 537 | entropy_count = r->poolinfo->POOLBITS; |
538 | r->entropy_count = entropy_count; | ||
536 | 539 | ||
537 | /* should we wake readers? */ | 540 | /* should we wake readers? */ |
538 | if (r == &input_pool && | 541 | if (r == &input_pool && entropy_count >= random_read_wakeup_thresh) { |
539 | r->entropy_count >= random_read_wakeup_thresh) { | ||
540 | wake_up_interruptible(&random_read_wait); | 542 | wake_up_interruptible(&random_read_wait); |
541 | kill_fasync(&fasync, SIGIO, POLL_IN); | 543 | kill_fasync(&fasync, SIGIO, POLL_IN); |
542 | } | 544 | } |
543 | |||
544 | spin_unlock_irqrestore(&r->lock, flags); | 545 | spin_unlock_irqrestore(&r->lock, flags); |
545 | } | 546 | } |
546 | 547 | ||