Message ID | 20090430130542.GF6900@wotan.suse.de (mailing list archive) |
---|---|
State | Not Applicable, archived |
Headers | show |
Hi Nick, On Thu, 30 Apr 2009 15:05:42 +0200 Nick Piggin <npiggin@suse.de> wrote: > > Hmm, this might do it. The following code now passes some stress testing > in a userspace harness wheras before it did not (and was obviously wrong). Indeed that allows it to boot fine. Thanks. Tested-by: Stephen Rothwell <sfr@canb.auug.org.au>
On Thu, Apr 30, 2009 at 03:05:42PM +0200, Nick Piggin wrote: [...] > --- > SLQB: fix dumb early allocation cache > > The dumb early allocation cache had a bug where it could allow allocation > to go past the end of a page, which could cause crashes or random memory > corruption. Fix this and simplify the logic. > > Signed-off-by: Nick Piggin <npiggin@suse.de> > --- > mm/slqb.c | 19 +++++++++++-------- > 1 file changed, 11 insertions(+), 8 deletions(-) > > Index: linux-2.6/mm/slqb.c > =================================================================== > --- linux-2.6.orig/mm/slqb.c > +++ linux-2.6/mm/slqb.c > @@ -2185,8 +2185,11 @@ static void *kmem_cache_dyn_array_alloc( > { > size_t size = sizeof(void *) * ids; > > + BUG_ON(!size); > + > if (unlikely(!slab_is_available())) { > static void *nextmem; > + static size_t nextleft; > void *ret; > > /* > @@ -2194,16 +2197,16 @@ static void *kmem_cache_dyn_array_alloc( > * never get freed by definition so we can do it rather > * simply. > */ > - if (!nextmem) { > - nextmem = alloc_pages_exact(size, GFP_KERNEL); > - if (!nextmem) > - return NULL; > + if (size > nextleft) { > + nextmem = alloc_pages_exact(size, GFP_KERNEL); > + if (!nextmem) > + return NULL; Cosmetic issue: spaces instead of tabs are used on these three lines. > + nextleft = roundup(size, PAGE_SIZE); > } > + > ret = nextmem; > - nextmem = (void *)((unsigned long)ret + size); > - if ((unsigned long)ret >> PAGE_SHIFT != > - (unsigned long)nextmem >> PAGE_SHIFT) > - nextmem = NULL; > + nextleft -= size; > + nextmem += size; > memset(ret, 0, size); > return ret; > } else {
On Fri, May 01, 2009 at 12:00:33AM +1000, Stephen Rothwell wrote: > Hi Nick, > > On Thu, 30 Apr 2009 15:05:42 +0200 Nick Piggin <npiggin@suse.de> wrote: > > > > Hmm, this might do it. The following code now passes some stress testing > > in a userspace harness wheras before it did not (and was obviously wrong). > > Indeed that allows it to boot fine. Thanks. > > Tested-by: Stephen Rothwell <sfr@canb.auug.org.au> Great, thanks for reporting and testing. This one is especially important because it is basically scribbling on random memory :( Pekka, please apply. Thanks, Nick
On Thu, 2009-04-30 at 18:10 +0400, Anton Vorontsov wrote: > > @@ -2194,16 +2197,16 @@ static void *kmem_cache_dyn_array_alloc( > > * never get freed by definition so we can do it rather > > * simply. > > */ > > - if (!nextmem) { > > - nextmem = alloc_pages_exact(size, GFP_KERNEL); > > - if (!nextmem) > > - return NULL; > > + if (size > nextleft) { > > + nextmem = alloc_pages_exact(size, GFP_KERNEL); > > + if (!nextmem) > > + return NULL; > > Cosmetic issue: spaces instead of tabs are used on these > three lines. I fixed that up. Thanks!
On Thu, 2009-04-30 at 16:10 +0200, Nick Piggin wrote: > On Fri, May 01, 2009 at 12:00:33AM +1000, Stephen Rothwell wrote: > > Hi Nick, > > > > On Thu, 30 Apr 2009 15:05:42 +0200 Nick Piggin <npiggin@suse.de> wrote: > > > > > > Hmm, this might do it. The following code now passes some stress testing > > > in a userspace harness wheras before it did not (and was obviously wrong). > > > > Indeed that allows it to boot fine. Thanks. > > > > Tested-by: Stephen Rothwell <sfr@canb.auug.org.au> > > Great, thanks for reporting and testing. This one is especially > important because it is basically scribbling on random memory > :( Pekka, please apply. Applied, thanks!
Index: linux-2.6/mm/slqb.c =================================================================== --- linux-2.6.orig/mm/slqb.c +++ linux-2.6/mm/slqb.c @@ -2185,8 +2185,11 @@ static void *kmem_cache_dyn_array_alloc( { size_t size = sizeof(void *) * ids; + BUG_ON(!size); + if (unlikely(!slab_is_available())) { static void *nextmem; + static size_t nextleft; void *ret; /* @@ -2194,16 +2197,16 @@ static void *kmem_cache_dyn_array_alloc( * never get freed by definition so we can do it rather * simply. */ - if (!nextmem) { - nextmem = alloc_pages_exact(size, GFP_KERNEL); - if (!nextmem) - return NULL; + if (size > nextleft) { + nextmem = alloc_pages_exact(size, GFP_KERNEL); + if (!nextmem) + return NULL; + nextleft = roundup(size, PAGE_SIZE); } + ret = nextmem; - nextmem = (void *)((unsigned long)ret + size); - if ((unsigned long)ret >> PAGE_SHIFT != - (unsigned long)nextmem >> PAGE_SHIFT) - nextmem = NULL; + nextleft -= size; + nextmem += size; memset(ret, 0, size); return ret; } else {