Messages in this thread Patch in this message | | | Subject | Re: drivers/char/random.c line 728 BUG | From | Matt Mackall <> | Date | Wed, 03 Sep 2008 17:51:37 -0500 |
| |
On Wed, 2008-09-03 at 15:32 -0700, Andrew Morton wrote: > On Wed, 03 Sep 2008 17:12:00 -0500 > Matt Mackall <mpm@selenic.com> wrote: > > > > Could we still apply his patch for the upcoming stable tree and fix it > > > the "right" way at some point in the future? > > > > I'm not sure what the current state of play is here in terms of the > > original patch being pushed to stable and mainline, but my patch is both > > simpler and more correct. If it's not too late, it's the one that should > > go to both places. > > It's a fairly minor thing. The post-this-patch code takes care to > ensure that ->entropy_count never has an illegal value, so it's OK but > aesthetially unpleasing to check its value outside the lock. > > And the post-this-patch code generates less .text, so it's a desirable > thing from that POV too. > > We could/should do both, I guess.
Sure, adding a local variable is fine. Though really, this shouldn't actually improve things, that's just our compiler being sucky. And I think it's a wash in terms of readability.
> I kinda ducked your move-the-BUG > patch because I wasn't in a write-yet-another-changelog mood.
I included one, but it was terse:
Avoid checking lock-protected entropy_count when lock isn't held.
Here's a more expansive version:
Avoid checking lock-protected entropy_count when lock isn't held. This fixes a bug reported by and diagnosed by Aaron Straus.
This fixes a regression introduced 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
Signed-off-by: Matt Mackall <mpm@selenic.com>
diff -r ddae8a8d3c6f drivers/char/random.c --- a/drivers/char/random.c Tue Jul 29 03:07:11 2008 +0000 +++ b/drivers/char/random.c Wed Sep 03 17:43:18 2008 -0500 @@ -726,11 +726,10 @@ { unsigned long flags; - BUG_ON(r->entropy_count > r->poolinfo->POOLBITS); - /* Hold lock while accounting */ spin_lock_irqsave(&r->lock, flags); + BUG_ON(r->entropy_count > r->poolinfo->POOLBITS); DEBUG_ENT("trying to extract %d bits from %s\n", nbytes * 8, r->name);
-- Mathematics is the supreme nostalgia of our time. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |