Re: [PATCH] crypto: serpent: improve __serpent_setkey with UBSAN

[Herbert Xu]

On Mon, Jul 31, 2017 at 10:43:55PM +0200, Arnd Bergmann wrote:
> When UBSAN is enabled, we get a very large stack frame for
> __serpent_setkey, when the register allocator ends up using more registers
> than it has, and has to spill temporary values to the stack. The code
> was originally optimized for in-order x86-32 CPU implementations using
> older compilers, but it now runs into a highly suboptimal case on all
> CPU architectures, as seen by this warning:
> 
> crypto/serpent_generic.c: In function '__serpent_setkey':
> crypto/serpent_generic.c:436:1: error: the frame size of 2720 bytes is larger than 2048 bytes [-Werror=frame-larger-than=]
> 
> Disabling -fsanitize=alignment would avoid that warning, presumably the
> option turns off a optimization step that is required for getting the
> register allocation right, but there is no easy way to do that on gcc-7
> (gcc-8 introduces a function attribute for this).
> 
> I tried to figure out a way to modify the source code instead, and noticed
> that the two stages of the setkey() function (keyiter and sbox) each are
> fine by themselves, but not when combined into one function. Splitting
> out the entire sbox into a separate function also happens to work fine
> with all compilers I tried (arm, arm64 and x86).
> 
> The setkey function uses a strange way to handle offsets into the key
> array, using both negative and positive index values, as well as adjusting
> the array pointer back and forth. I have checked that this actually
> makes no difference to modern compilers, but I left that untouched
> to make the patch easier to review and to keep the code closer to
> the reference implementation.
> 
> Link: https://patchwork.kernel.org/patch/9189575/
> Signed-off-by: Arnd Bergmann <arnd@xxxxxxxx>

Patch applied.  Thanks.
-- 
Email: Herbert Xu <herbert@xxxxxxxxxxxxxxxxxxx>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt