netdev - Re: [PATCH 1/3] sha512: make it work, undo percpu message schedule

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1326577610.5287.38.camel@edumazet-laptop>
Date:	Sat, 14 Jan 2012 22:46:50 +0100
From:	Eric Dumazet <eric.dumazet@...il.com>
To:	Alexey Dobriyan <adobriyan@...il.com>
Cc:	Herbert Xu <herbert@...dor.apana.org.au>,
	linux-crypto@...r.kernel.org, netdev@...r.kernel.org,
	ken@...elabs.ch, Steffen Klassert <steffen.klassert@...unet.com>,
	security@...nel.org
Subject: Re: [PATCH 1/3] sha512: make it work, undo percpu message schedule

Le samedi 14 janvier 2012 à 21:27 +0300, Alexey Dobriyan a écrit :
> commit f9e2bca6c22d75a289a349f869701214d63b5060
> aka "crypto: sha512 - Move message schedule W[80] to static percpu area"
> created global message schedule area.


> Signed-off-by: Alexey Dobriyan <adobriyan@...il.com>
> Cc: stable@...r.kernel.org
> ---
> 
>  crypto/sha512_generic.c |    6 +-----
>  1 file changed, 1 insertion(+), 5 deletions(-)
> 
> --- a/crypto/sha512_generic.c
> +++ b/crypto/sha512_generic.c
> @@ -21,8 +21,6 @@
>  #include <linux/percpu.h>
>  #include <asm/byteorder.h>
>  
> -static DEFINE_PER_CPU(u64[80], msg_schedule);
> -
>  static inline u64 Ch(u64 x, u64 y, u64 z)
>  {
>          return z ^ (x & (y ^ z));
> @@ -89,7 +87,7 @@ sha512_transform(u64 *state, const u8 *input)
>  	u64 a, b, c, d, e, f, g, h, t1, t2;
>  
>  	int i;
> -	u64 *W = get_cpu_var(msg_schedule);
> +	u64 W[80];
>  
>  	/* load the input */
>          for (i = 0; i < 16; i++)
> @@ -128,8 +126,6 @@ sha512_transform(u64 *state, const u8 *input)
>  
>  	/* erase our data */
>  	a = b = c = d = e = f = g = h = t1 = t2 = 0;
> -	memset(W, 0, sizeof(__get_cpu_var(msg_schedule)));
> -	put_cpu_var(msg_schedule);
>  }
>  
>  static int

Is it just me or are you ignoring what crypto maintainer and others
thought of your patch ?

You are re-introducing a 640 bytes stack array, how comes it can be
really safe ?

This is too risky, and we provided an alternate patch, not just for fun.


--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html