Fwd: [crypto:master 60/60] arch/x86/crypto/ghash-clmulni-intel_glue.c:71:25: sparse: cast to restricted __be64

[Ard Biesheuvel]

Greg,

This pertains to commit 8ceee72808d1 (crypto: ghash-clmulni-intel -
use C implementation for setkey()) that has been pulled by Linus
during the current merge window.

It is missing two things:
- a cc to stable annotation
- a fix for the sparse warning below (change cast from __be64 to __force __be64)

The reason for cc'ing stable on this patch is that it fixes a
potential data corruption issue where the ghash setkey() method uses
SSE registers without calling kernel_fpu_begin() first. This issue was
introduced by 0e1227d356e9b (crypto: ghash - Add PCLMULQDQ accelerated
implementation).

So how would you like to proceed with this? Should I propose a new
patch somewhere?

Regards,
Ard.


---------- Forwarded message ----------
From: Herbert Xu <herbert@xxxxxxxxxxxxxxxxxxx>
Date: 1 April 2014 14:48
Subject: Re: [crypto:master 60/60]
arch/x86/crypto/ghash-clmulni-intel_glue.c:71:25: sparse: cast to
restricted __be64
To: Ard Biesheuvel <ard.biesheuvel@xxxxxxxxxx>
Cc: "linux-crypto@xxxxxxxxxxxxxxx" <linux-crypto@xxxxxxxxxxxxxxx>,
kbuild test robot <fengguang.wu@xxxxxxxxx>


On Tue, Apr 01, 2014 at 02:37:20PM +0200, Ard Biesheuvel wrote:
> On 1 April 2014 13:23, kbuild test robot <fengguang.wu@xxxxxxxxx> wrote:
> > tree:   git://git.kernel.org/pub/scm/linux/kernel/git/herbert/crypto-2.6.git master
> > head:   8ceee72808d1ae3fb191284afc2257a2be964725
> > commit: 8ceee72808d1ae3fb191284afc2257a2be964725 [60/60] crypto: ghash-clmulni-intel - use C implementation for setkey()
> > reproduce: make C=1 CF=-D__CHECK_ENDIAN__
> >
> >
> > sparse warnings: (new ones prefixed by >>)
> >
> >>> arch/x86/crypto/ghash-clmulni-intel_glue.c:71:25: sparse: cast to restricted __be64
> >>> arch/x86/crypto/ghash-clmulni-intel_glue.c:72:25: sparse: cast to restricted __be64
> >
>
> Sigh.
>
> The sparse warnings /without/ the be64 casts are even worse.
>
> The obvious fix is not to use a be128 for the key, as it is obviously
> an opaque type that just represents a byte array.
> So, Herbert, if you prefer, I can rework this patch to use be128
> instead of u128 inside struct ghash_ctx, but it will have some fallout
> throughout the file. Or instead, we cast to '__force __be64',
> basically just telling sparse to shut up ...

I'll add the __force to shut it up.  Thanks!
--
Email: Herbert Xu <herbert@xxxxxxxxxxxxxxxxxxx>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/