Re: [BUG] SLOB breaks Crypto

[Paul Mundt <lethal@linux-sh.org>]

On Tue, May 18, 2010 at 02:20:21PM -0700, David Miller wrote:
> From: Pekka Enberg <penberg@cs.helsinki.fi>
> Date: Wed, 19 May 2010 00:15:46 +0300
> 
> > On Tue, May 18, 2010 at 11:59 PM, David Miller <davem@davemloft.net> wrote:
> >> From: Matt Mackall <mpm@selenic.com>
> >> Date: Tue, 18 May 2010 14:33:55 -0500
> >>
> >>> SLOB honors ARCH_KMALLOC_MINALIGN. If your arch has alignment
> >>> requirements, I recommend you set it.
> >>
> >> I recommend that the alignment provided by the allocator is not
> >> determined by which allocator I happen to have enabled.
> >>
> >> The values and ifdef'ery should be identical in all of our
> >> allocators.
> > 
> > Why? It doesn't make much sense for SLOB, which tries to be as space
> > efficient as possible, as a default. If things break on sparc, it
> > really needs to set ARCH_KMALLOC_MINALIGN as slab default alignment is
> > not something you really want to depend on.
> 
> I think it does make sense to expect that, whatever my architecture
> defines or does not define, I can expect the allocators to provide the
> same minimum alignment guarentee.  Otherwise it is no guarantee at all.
> 
SLAB/SLUB/SLOB all used to have the same BYTES_PER_WORD alignment
guarantee, with SLAB and SLUB having moved away from this to unsigned
long long in b46b8f19 and 47bfdc0d respectively. This was due to mixing
64-bit integers in data structures, which in the SLAB case resulted in
misaligned structures and also broke redzoning (architecture overrides
also disabled it completely). The SLUB change was made a couple of days
earlier for the same structure misalignment reasons (64-bit integers on
32-bit platforms).

The default changes in SLAB/SLUB at least assume that 32-bit
architectures can only address 64-bit values on a 64-bit boundary. While
this is true for most cases, these have always been handled through the
bumping of the architecture minalign values in the past. Indeed, this was
the rationale I had for adding the architecture-specific slab minalign
override in the first place. The kmalloc one on the other hand is largely
just overriden for platforms with DMA requirements -- usually a
cacheline boundary.

> It's already obvious from these reports that such dependencies do
> exist.
> 
These dependencies were then introduced after SLAB/SLUB changed the
rules, suggesting that not enough testing was done.

> So one of two things should happen:
> 
> 1) SLOB conforms to SLAB/SLUB in it's test
> 
> 2) SLAB/SLUB conforms to SLOB in it's test
> 
> And yes this is an either-or, you can't say they are both valid.

I don't see any reason to punish SLOB for the assumptions that SLAB/SLUB
arbitrarily took up, presumably on an architecture that should have
specified its own alignment requirements and simply couldn't be bothered.
Making SLAB redzoning work with arbitrary alignment is another matter
entirely, and something that should probably be revisited.

Anything that assumes more than BYTES_PER_WORD is simply broken and
should be reverted.