| Message ID | 1474659116-4689-1-git-send-email-marcelo.cerri@canonical.com (mailing list archive) |
|---|---|
| State | Accepted |
| Delegated to: | Herbert Xu |
| Headers | show |
Fri, Sep 23, 2016 at 04:31:56PM -0300, Marcelo Cerri wrote: > The driver does not handle endianness properly when loading the input > data. Indeed. I tested in both endianesses and it's working fine. Thanks! Herbert, can we go ahead with this fix? > > Signed-off-by: Marcelo Cerri <marcelo.cerri@canonical.com> > --- > arch/powerpc/crypto/sha1-powerpc-asm.S | 13 +++++++++++-- > 1 file changed, 11 insertions(+), 2 deletions(-) > > diff --git a/arch/powerpc/crypto/sha1-powerpc-asm.S b/arch/powerpc/crypto/sha1-powerpc-asm.S > index 125e165..82ddc9b 100644 > --- a/arch/powerpc/crypto/sha1-powerpc-asm.S > +++ b/arch/powerpc/crypto/sha1-powerpc-asm.S > @@ -7,6 +7,15 @@ > #include <asm/ppc_asm.h> > #include <asm/asm-offsets.h> > > +#ifdef __BIG_ENDIAN__ > +#define LWZ(rt, d, ra) \ > + lwz rt,d(ra) > +#else > +#define LWZ(rt, d, ra) \ > + li rt,d; \ > + lwbrx rt,rt,ra > +#endif > + > /* > * We roll the registers for T, A, B, C, D, E around on each > * iteration; T on iteration t is A on iteration t+1, and so on. > @@ -23,7 +32,7 @@ > #define W(t) (((t)%16)+16) > > #define LOADW(t) \ > - lwz W(t),(t)*4(r4) > + LWZ(W(t),(t)*4,r4) > > #define STEPD0_LOAD(t) \ > andc r0,RD(t),RB(t); \ > @@ -33,7 +42,7 @@ > add r0,RE(t),r15; \ > add RT(t),RT(t),r6; \ > add r14,r0,W(t); \ > - lwz W((t)+4),((t)+4)*4(r4); \ > + LWZ(W((t)+4),((t)+4)*4,r4); \ > rotlwi RB(t),RB(t),30; \ > add RT(t),RT(t),r14 > > -- > 2.7.4 >
Hi Herbert, Any thoughts on this one?
On Wed, Sep 28, 2016 at 10:15:51AM -0300, Marcelo Cerri wrote: > Hi Herbert, > > Any thoughts on this one? Can this patch wait until the next merge window? On the broken platforms it should just fail the self-test, right? Cheers,
On Wed, Sep 28, 2016 at 09:20:15PM +0800, Herbert Xu wrote: > On Wed, Sep 28, 2016 at 10:15:51AM -0300, Marcelo Cerri wrote: > > Hi Herbert, > > > > Any thoughts on this one? > > Can this patch wait until the next merge window? On the broken > platforms it should just fail the self-test, right? Yes. It fails on any LE platform (including Ubuntu and RHEL 7.1). > > Cheers, > -- > Email: Herbert Xu <herbert@gondor.apana.org.au> > Home Page: http://gondor.apana.org.au/~herbert/ > PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
On Fri, Sep 23, 2016 at 04:31:56PM -0300, Marcelo Cerri wrote: > The driver does not handle endianness properly when loading the input > data. > > Signed-off-by: Marcelo Cerri <marcelo.cerri@canonical.com> Patch applied. Thanks.
Marcelo Cerri <marcelo.cerri@canonical.com> writes: > [ Unknown signature status ] > On Wed, Sep 28, 2016 at 09:20:15PM +0800, Herbert Xu wrote: >> On Wed, Sep 28, 2016 at 10:15:51AM -0300, Marcelo Cerri wrote: >> > Hi Herbert, >> > >> > Any thoughts on this one? >> >> Can this patch wait until the next merge window? On the broken >> platforms it should just fail the self-test, right? > > Yes. It fails on any LE platform (including Ubuntu and RHEL 7.1). How are you testing this? I thought I was running the crypto tests but I've never seen this fail. cheers -- To unsubscribe from this list: send the line "unsubscribe linux-crypto" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Michael, On Ubuntu, CRYPTO_MANAGER_DISABLE_TESTS is set by default. So I had to disable this config in order to make sha1-powerpc fail in the crypto API tests. However, even with tests disabled, any usage of sha1-powerpc should result in incorrect results.
diff --git a/arch/powerpc/crypto/sha1-powerpc-asm.S b/arch/powerpc/crypto/sha1-powerpc-asm.S index 125e165..82ddc9b 100644 --- a/arch/powerpc/crypto/sha1-powerpc-asm.S +++ b/arch/powerpc/crypto/sha1-powerpc-asm.S @@ -7,6 +7,15 @@ #include <asm/ppc_asm.h> #include <asm/asm-offsets.h> +#ifdef __BIG_ENDIAN__ +#define LWZ(rt, d, ra) \ + lwz rt,d(ra) +#else +#define LWZ(rt, d, ra) \ + li rt,d; \ + lwbrx rt,rt,ra +#endif + /* * We roll the registers for T, A, B, C, D, E around on each * iteration; T on iteration t is A on iteration t+1, and so on. @@ -23,7 +32,7 @@ #define W(t) (((t)%16)+16) #define LOADW(t) \ - lwz W(t),(t)*4(r4) + LWZ(W(t),(t)*4,r4) #define STEPD0_LOAD(t) \ andc r0,RD(t),RB(t); \ @@ -33,7 +42,7 @@ add r0,RE(t),r15; \ add RT(t),RT(t),r6; \ add r14,r0,W(t); \ - lwz W((t)+4),((t)+4)*4(r4); \ + LWZ(W((t)+4),((t)+4)*4,r4); \ rotlwi RB(t),RB(t),30; \ add RT(t),RT(t),r14
The driver does not handle endianness properly when loading the input data. Signed-off-by: Marcelo Cerri <marcelo.cerri@canonical.com> --- arch/powerpc/crypto/sha1-powerpc-asm.S | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-)