linux-kernel - Re: [BUG/PATCH] kernel RNG and its secrets

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: Windows password security audit tool. GUI, reports in PDF.

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

Date:	Wed, 18 Mar 2015 17:14:02 +0000
From:	mancha <mancha1@...o.com>
To:	Stephan Mueller <smueller@...onox.de>
Cc:	Hannes Frederic Sowa <hannes@...essinduktion.org>,
	Daniel Borkmann <daniel@...earbox.net>, tytso@....edu,
	linux-kernel@...r.kernel.org, linux-crypto@...r.kernel.org,
	herbert@...dor.apana.org.au, dborkman@...hat.com,
	cesarb@...arb.eti.br
Subject: Re: [BUG/PATCH] kernel RNG and its secrets

On Wed, Mar 18, 2015 at 05:02:01PM +0100, Stephan Mueller wrote:
> Am Mittwoch, 18. März 2015, 16:09:34 schrieb Hannes Frederic Sowa:
> 
> Hi Hannes,
> 
> >On Wed, Mar 18, 2015, at 13:42, Daniel Borkmann wrote:
> >> On 03/18/2015 01:20 PM, Stephan Mueller wrote:
> >> > Am Mittwoch, 18. März 2015, 13:19:07 schrieb Hannes Frederic Sowa:
> >> >>>> My proposal would be to add a
> >> >>>> 
> >> >>>> #define OPTIMIZER_HIDE_MEM(ptr, len) __asm__ __volatile__ ("" :
> >> >>>> :
> >> >>>> "m"(
> >> >>>> ({ struct { u8 b[len]; } *p = (void *)ptr ; *p; }) )
> >> >>>> 
> >> >>>> and use this in the code function.
> >> >>>> 
> >> >>>> This is documented in gcc manual 6.43.2.5.
> >> >>> 
> >> >>> That one adds the zeroization instructuctions. But now there are
> >> >>> much
> >> >>> more than with the barrier.
> >> >>> 
> >> >>>    400469:       48 c7 04 24 00 00 00    movq   $0x0,(%rsp)
> >> >>>    400470:       00
> >> >>>    400471:       48 c7 44 24 08 00 00    movq   $0x0,0x8(%rsp)
> >> >>>    400478:       00 00
> >> >>>    40047a:       c7 44 24 10 00 00 00    movl   $0x0,0x10(%rsp)
> >> >>>    400481:       00
> >> >>>    400482:       48 c7 44 24 20 00 00    movq   $0x0,0x20(%rsp)
> >> >>>    400489:       00 00
> >> >>>    40048b:       48 c7 44 24 28 00 00    movq   $0x0,0x28(%rsp)
> >> >>>    400492:       00 00
> >> >>>    400494:       c7 44 24 30 00 00 00    movl   $0x0,0x30(%rsp)
> >> >>>    40049b:       00
> >> >>> 
> >> >>> Any ideas?
> >> >> 
> >> >> Hmm, correct definition of u8?
> >> > 
> >> > I use unsigned char
> >> > 
> >> >> Which version of gcc do you use? I can't see any difference if I
> >> >> compile your example at -O2.
> >> > 
> >> > gcc-Version 4.9.2 20150212 (Red Hat 4.9.2-6) (GCC)
> >
> >Well, was an error on my side, I see the same behavior.
> >
> >> I can see the same with the gcc version I previously posted. So
> >> it clears the 20 bytes from your example (movq, movq, movl) at
> >> two locations, presumably buf[] and b[].
> >
> >Yes, it looks like that. The reservation on the stack changes, too.
> >
> >Seems like just using barrier() is the best and easiest option.
> 
> Would you prepare a patch for that?
> >
> >Thanks,
> >Hannes
> 
> 
> Ciao
> Stephan

Hi.

Patch 0001 fixes the dead store issue in memzero_explicit().

However, if the idea is to use barrier() instead of OPTIMIZER_HIDE_VAR()
in crypto_memneq() as well, then patch 0002 is the one to use. Please
review and keep in mind my analysis was limited to memzero_explicit().

Cesar, were there reasons you didn't use the gcc version of barrier()
for crypto_memneq()?

Please let me know if I can be of any further assistance.

--mancha

View attachment "0001-memzero_explicit.patch" of type "text/plain" (727 bytes)

View attachment "0002-OPTIMIZER_HIDE_VAR_purge.patch" of type "text/plain" (6107 bytes)

Content of type "application/pgp-signature" skipped