[PATCH] crypto: sha1-powerpc: little-endian support

[PATCH] crypto: sha1-powerpc: little-endian support

[Marcelo Cerri]

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(-)

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

Re: [PATCH] crypto: sha1-powerpc: little-endian support

[Paulo Flabiano Smorigo]

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
> 

-- 
Paulo Flabiano Smorigo
IBM Linux Technology Center

Re: [PATCH] crypto: sha1-powerpc: little-endian support

[Marcelo Cerri]

[-- Attachment #1: Type: text/plain, Size: 1476 bytes --]

Hi Herbert,

Any thoughts on this one?

-- 
Regards,
Marcelo

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>
> ---
>  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
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

Re: [PATCH] crypto: sha1-powerpc: little-endian support

[Herbert Xu]

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,
-- 
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

Re: [PATCH] crypto: sha1-powerpc: little-endian support

[Marcelo Cerri]

[-- Attachment #1: Type: text/plain, Size: 586 bytes --]

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

-- 
Regards,
Marcelo

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

Re: [PATCH] crypto: sha1-powerpc: little-endian support

[Herbert Xu]

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.
-- 
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

Re: [PATCH] crypto: sha1-powerpc: little-endian support

[Michael Ellerman]

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

Re: [PATCH] crypto: sha1-powerpc: little-endian support

[Marcelo Cerri]

[-- Attachment #1: Type: text/plain, Size: 984 bytes --]

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.

-- 
Regards,
Marcelo

On Tue, Oct 04, 2016 at 05:23:16PM +1100, Michael Ellerman wrote:
> 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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]