Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

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>

authored by

Andrew Morton and committed by
Linus Torvalds
8b76f46a 9d359357

+10 -9
+10 -9
drivers/char/random.c
··· 407 407 /* read-write data: */ 408 408 spinlock_t lock; 409 409 unsigned add_ptr; 410 - int entropy_count; 410 + int entropy_count; /* Must at no time exceed ->POOLBITS! */ 411 411 int input_rotate; 412 412 }; 413 413 ··· 520 520 static void credit_entropy_bits(struct entropy_store *r, int nbits) 521 521 { 522 522 unsigned long flags; 523 + int entropy_count; 523 524 524 525 if (!nbits) 525 526 return; ··· 528 527 spin_lock_irqsave(&r->lock, flags); 529 528 530 529 DEBUG_ENT("added %d entropy credits to %s\n", nbits, r->name); 531 - r->entropy_count += nbits; 532 - if (r->entropy_count < 0) { 530 + entropy_count = r->entropy_count; 531 + entropy_count += nbits; 532 + if (entropy_count < 0) { 533 533 DEBUG_ENT("negative entropy/overflow\n"); 534 - r->entropy_count = 0; 535 - } else if (r->entropy_count > r->poolinfo->POOLBITS) 536 - r->entropy_count = r->poolinfo->POOLBITS; 534 + entropy_count = 0; 535 + } else if (entropy_count > r->poolinfo->POOLBITS) 536 + entropy_count = r->poolinfo->POOLBITS; 537 + r->entropy_count = entropy_count; 537 538 538 539 /* should we wake readers? */ 539 - if (r == &input_pool && 540 - r->entropy_count >= random_read_wakeup_thresh) { 540 + if (r == &input_pool && entropy_count >= random_read_wakeup_thresh) { 541 541 wake_up_interruptible(&random_read_wait); 542 542 kill_fasync(&fasync, SIGIO, POLL_IN); 543 543 } 544 - 545 544 spin_unlock_irqrestore(&r->lock, flags); 546 545 } 547 546