linux-kernel - Re: [BUG] SLOB's krealloc() seems bust

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <200810081611.30897.nickpiggin@yahoo.com.au>
Date:	Wed, 8 Oct 2008 16:11:30 +1100
From:	Nick Piggin <nickpiggin@...oo.com.au>
To:	Matt Mackall <mpm@...enic.com>
Cc:	Pekka Enberg <penberg@...helsinki.fi>,
	Peter Zijlstra <a.p.zijlstra@...llo.nl>,
	Christoph Lameter <cl@...ux-foundation.org>,
	linux-mm <linux-mm@...ck.org>,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	Ingo Molnar <mingo@...e.hu>,
	linux-kernel <linux-kernel@...r.kernel.org>,
	akpm <akpm@...uxfoundation.org>
Subject: Re: [BUG] SLOB's krealloc() seems bust

On Wednesday 08 October 2008 15:54, Nick Piggin wrote:
> On Wednesday 08 October 2008 15:46, Matt Mackall wrote:
> > On Wed, 2008-10-08 at 15:22 +1100, Nick Piggin wrote:
> > > On Wednesday 08 October 2008 10:08, Matt Mackall wrote:

> > > > diff -r 5e32b09a1b2b mm/slob.c
> > > > --- a/mm/slob.c	Fri Oct 03 14:04:43 2008 -0500
> > > > +++ b/mm/slob.c	Tue Oct 07 18:05:15 2008 -0500
> > > > @@ -514,9 +514,11 @@
> > > >  		return 0;
> > > >
> > > >  	sp = (struct slob_page *)virt_to_page(block);
> > > > -	if (slob_page(sp))
> > > > -		return ((slob_t *)block - 1)->units + SLOB_UNIT;
> > > > -	else
> > > > +	if (slob_page(sp)) {
> > > > +		int align = max(ARCH_KMALLOC_MINALIGN, ARCH_SLAB_MINALIGN);
> > > > +		unsigned int *m = (unsigned int *)(block - align);
> > > > +		return SLOB_UNITS(*m); /* round up */
> > > > +	} else
> > > >  		return sp->page.private;
> > > >  }
> > >
> > > Yes, I came up with nearly the same patch before reading this
> > >
> > > --- linux-2.6/mm/slob.c 2008-10-08 14:43:17.000000000 +1100
> > > +++ suth/mm/slob.c      2008-10-08 15:11:06.000000000 +1100
> > > @@ -514,9 +514,11 @@ size_t ksize(const void *block)
> > >                 return 0;
> > >
> > >         sp = (struct slob_page *)virt_to_page(block);
> > > -       if (slob_page(sp))
> > > -               return (((slob_t *)block - 1)->units - 1) * SLOB_UNIT;
> > > -       else
> > > +       if (slob_page(sp)) {
> > > +               int align = max(ARCH_KMALLOC_MINALIGN,
> > > ARCH_SLAB_MINALIGN); +               unsigned int *m = (unsigned int
> > > *)(block - align); +               return *m + align;
> > > +       } else
> > >                 return sp->page.private;
> > >  }
> > >
> > > However, mine is lifted directly from kfree, wheras you do something a
> > > bit different. Hmm, ksize arguably could be used to find the underlying
> > > allocated slab size in order to use a little bit more than we'd asked
> > > for. So probably we should really just `return *m` (don't round up or
> > > add any padding).
> >
> > Huh? ksize should report how much space is available in the buffer. If
> > we request 33 bytes from SLUB and it gives us 64, ksize reports 64. If
> > we request 33 bytes from SLOB and it gives us 34, we should report 34.
>
> Oh.. hmm yeah right, I didn't realise what you were doing there.
> OK, so your patch looks good to me then (provided it is diffed against
> the previous one, for Linus).

OK, no, that's why I got confused. SLOB_UNITS will round you up to the
next SLOB_UNIT. You'd then have to multiply by SLOB_UNIT to get back to
bytes.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/