[PATCH -crypto] lib: memzero_explicit: use barrier instead of OPTIMIZER_HIDE_VAR

[PATCH -crypto] lib: memzero_explicit: use barrier instead of OPTIMIZER_HIDE_VAR

[Daniel Borkmann]

From: mancha security <mancha1@zoho.com>

OPTIMIZER_HIDE_VAR(), as defined when using gcc, is insufficient to
ensure protection from dead store optimization.

For the random driver and crypto drivers, calls are emitted ...

  $ gdb vmlinux
  (gdb) disassemble memzero_explicit
  Dump of assembler code for function memzero_explicit:
    0xffffffff813a18b0 <+0>:	push   %rbp
    0xffffffff813a18b1 <+1>:	mov    %rsi,%rdx
    0xffffffff813a18b4 <+4>:	xor    %esi,%esi
    0xffffffff813a18b6 <+6>:	mov    %rsp,%rbp
    0xffffffff813a18b9 <+9>:	callq  0xffffffff813a7120 <memset>
    0xffffffff813a18be <+14>:	pop    %rbp
    0xffffffff813a18bf <+15>:	retq
  End of assembler dump.

  (gdb) disassemble extract_entropy
  [...]
    0xffffffff814a5009 <+313>:	mov    %r12,%rdi
    0xffffffff814a500c <+316>:	mov    $0xa,%esi
    0xffffffff814a5011 <+321>:	callq  0xffffffff813a18b0 <memzero_explicit>
    0xffffffff814a5016 <+326>:	mov    -0x48(%rbp),%rax
  [...]

... but in case in future we might use facilities such as LTO, then
OPTIMIZER_HIDE_VAR() is not sufficient to protect gcc from a possible
eviction of the memset(). We have to use a compiler barrier instead.

Minimal test example when we assume memzero_explicit() would *not* be
a call, but would have been *inlined* instead:

  static inline void memzero_explicit(void *s, size_t count)
  {
    memset(s, 0, count);
    <foo>
  }

  int main(void)
  {
    char buff[20];

    snprintf(buff, sizeof(buff) - 1, "test");
    printf("%s", buff);

    memzero_explicit(buff, sizeof(buff));
    return 0;
  }

With <foo> := OPTIMIZER_HIDE_VAR():

  (gdb) disassemble main
  Dump of assembler code for function main:
  [...]
   0x0000000000400464 <+36>:	callq  0x400410 <printf@plt>
   0x0000000000400469 <+41>:	xor    %eax,%eax
   0x000000000040046b <+43>:	add    $0x28,%rsp
   0x000000000040046f <+47>:	retq
  End of assembler dump.

With <foo> := barrier():

  (gdb) disassemble main
  Dump of assembler code for function main:
  [...]
   0x0000000000400464 <+36>:	callq  0x400410 <printf@plt>
   0x0000000000400469 <+41>:	movq   $0x0,(%rsp)
   0x0000000000400471 <+49>:	movq   $0x0,0x8(%rsp)
   0x000000000040047a <+58>:	movl   $0x0,0x10(%rsp)
   0x0000000000400482 <+66>:	xor    %eax,%eax
   0x0000000000400484 <+68>:	add    $0x28,%rsp
   0x0000000000400488 <+72>:	retq
  End of assembler dump.

As can be seen, movq, movq, movl are being emitted inlined
via memset().

Reference: http://thread.gmane.org/gmane.linux.kernel.cryptoapi/13764/
Fixes: d4c5efdb9777 ("random: add and use memzero_explicit() for clearing data")
Cc: Hannes Frederic Sowa <hannes@stressinduktion.org>
Cc: Stephan Mueller <smueller@chronox.de>
Cc: Theodore Ts'o <tytso@mit.edu>
Signed-off-by: mancha security <mancha1@zoho.com>
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
---
 Sending to Herbert as crypto/random are the main users.
 Based against -crypto tree. Thanks!

 lib/string.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/string.c b/lib/string.c
index ce81aae..a579201 100644
--- a/lib/string.c
+++ b/lib/string.c
@@ -607,7 +607,7 @@ EXPORT_SYMBOL(memset);
 void memzero_explicit(void *s, size_t count)
 {
 	memset(s, 0, count);
-	OPTIMIZER_HIDE_VAR(s);
+	barrier();
 }
 EXPORT_SYMBOL(memzero_explicit);
 
-- 
1.9.3

Re: [PATCH -crypto] lib: memzero_explicit: use barrier instead of OPTIMIZER_HIDE_VAR

[Hannes Frederic Sowa]

On Wed, Mar 18, 2015, at 18:47, Daniel Borkmann wrote:
> From: mancha security <mancha1@zoho.com>
> 
> OPTIMIZER_HIDE_VAR(), as defined when using gcc, is insufficient to
> ensure protection from dead store optimization.
> 
> For the random driver and crypto drivers, calls are emitted ...
> 
>   $ gdb vmlinux
>   (gdb) disassemble memzero_explicit
>   Dump of assembler code for function memzero_explicit:
>     0xffffffff813a18b0 <+0>:        push   %rbp
>     0xffffffff813a18b1 <+1>:        mov    %rsi,%rdx
>     0xffffffff813a18b4 <+4>:        xor    %esi,%esi
>     0xffffffff813a18b6 <+6>:        mov    %rsp,%rbp
>     0xffffffff813a18b9 <+9>:        callq  0xffffffff813a7120 <memset>
>     0xffffffff813a18be <+14>:       pop    %rbp
>     0xffffffff813a18bf <+15>:       retq
>   End of assembler dump.
> 
>   (gdb) disassemble extract_entropy
>   [...]
>     0xffffffff814a5009 <+313>:      mov    %r12,%rdi
>     0xffffffff814a500c <+316>:      mov    $0xa,%esi
>     0xffffffff814a5011 <+321>:      callq  0xffffffff813a18b0
>     <memzero_explicit>
>     0xffffffff814a5016 <+326>:      mov    -0x48(%rbp),%rax
>   [...]
> 
> ... but in case in future we might use facilities such as LTO, then
> OPTIMIZER_HIDE_VAR() is not sufficient to protect gcc from a possible
> eviction of the memset(). We have to use a compiler barrier instead.
> 
> Minimal test example when we assume memzero_explicit() would *not* be
> a call, but would have been *inlined* instead:
> 
>   static inline void memzero_explicit(void *s, size_t count)
>   {
>     memset(s, 0, count);
>     <foo>
>   }
> 
>   int main(void)
>   {
>     char buff[20];
> 
>     snprintf(buff, sizeof(buff) - 1, "test");
>     printf("%s", buff);
> 
>     memzero_explicit(buff, sizeof(buff));
>     return 0;
>   }
> 
> With <foo> := OPTIMIZER_HIDE_VAR():
> 
>   (gdb) disassemble main
>   Dump of assembler code for function main:
>   [...]
>    0x0000000000400464 <+36>:       callq  0x400410 <printf@plt>
>    0x0000000000400469 <+41>:       xor    %eax,%eax
>    0x000000000040046b <+43>:       add    $0x28,%rsp
>    0x000000000040046f <+47>:       retq
>   End of assembler dump.
> 
> With <foo> := barrier():
> 
>   (gdb) disassemble main
>   Dump of assembler code for function main:
>   [...]
>    0x0000000000400464 <+36>:       callq  0x400410 <printf@plt>
>    0x0000000000400469 <+41>:       movq   $0x0,(%rsp)
>    0x0000000000400471 <+49>:       movq   $0x0,0x8(%rsp)
>    0x000000000040047a <+58>:       movl   $0x0,0x10(%rsp)
>    0x0000000000400482 <+66>:       xor    %eax,%eax
>    0x0000000000400484 <+68>:       add    $0x28,%rsp
>    0x0000000000400488 <+72>:       retq
>   End of assembler dump.
> 
> As can be seen, movq, movq, movl are being emitted inlined
> via memset().
> 
> Reference: http://thread.gmane.org/gmane.linux.kernel.cryptoapi/13764/
> Fixes: d4c5efdb9777 ("random: add and use memzero_explicit() for clearing
> data")
> Cc: Hannes Frederic Sowa <hannes@stressinduktion.org>
> Cc: Stephan Mueller <smueller@chronox.de>
> Cc: Theodore Ts'o <tytso@mit.edu>
> Signed-off-by: mancha security <mancha1@zoho.com>
> Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
> ---
>  Sending to Herbert as crypto/random are the main users.
>  Based against -crypto tree. Thanks!

Acked-by: Hannes Frederic Sowa <hannes@stressinduktion.org>

Still checking on how to realize the test. Thanks!

Re: [PATCH -crypto] lib: memzero_explicit: use barrier instead of OPTIMIZER_HIDE_VAR

[Stephan Mueller]

Am Mittwoch, 18. März 2015, 18:47:25 schrieb Daniel Borkmann:

Hi Daniel,

> From: mancha security <mancha1@zoho.com>
> 
> OPTIMIZER_HIDE_VAR(), as defined when using gcc, is insufficient to
> ensure protection from dead store optimization.
> 
> For the random driver and crypto drivers, calls are emitted ...
> 
>   $ gdb vmlinux
>   (gdb) disassemble memzero_explicit
>   Dump of assembler code for function memzero_explicit:
>     0xffffffff813a18b0 <+0>:	push   %rbp
>     0xffffffff813a18b1 <+1>:	mov    %rsi,%rdx
>     0xffffffff813a18b4 <+4>:	xor    %esi,%esi
>     0xffffffff813a18b6 <+6>:	mov    %rsp,%rbp
>     0xffffffff813a18b9 <+9>:	callq  0xffffffff813a7120 <memset>
>     0xffffffff813a18be <+14>:	pop    %rbp
>     0xffffffff813a18bf <+15>:	retq
>   End of assembler dump.
> 
>   (gdb) disassemble extract_entropy
>   [...]
>     0xffffffff814a5009 <+313>:	mov    %r12,%rdi
>     0xffffffff814a500c <+316>:	mov    $0xa,%esi
>     0xffffffff814a5011 <+321>:	callq  0xffffffff813a18b0 
<memzero_explicit>
>     0xffffffff814a5016 <+326>:	mov    -0x48(%rbp),%rax
>   [...]
> 
> ... but in case in future we might use facilities such as LTO, then
> OPTIMIZER_HIDE_VAR() is not sufficient to protect gcc from a possible
> eviction of the memset(). We have to use a compiler barrier instead.
> 
> Minimal test example when we assume memzero_explicit() would *not* be
> a call, but would have been *inlined* instead:
> 
>   static inline void memzero_explicit(void *s, size_t count)
>   {
>     memset(s, 0, count);
>     <foo>
>   }
> 
>   int main(void)
>   {
>     char buff[20];
> 
>     snprintf(buff, sizeof(buff) - 1, "test");
>     printf("%s", buff);
> 
>     memzero_explicit(buff, sizeof(buff));
>     return 0;
>   }
> 
> With <foo> := OPTIMIZER_HIDE_VAR():
> 
>   (gdb) disassemble main
>   Dump of assembler code for function main:
>   [...]
>    0x0000000000400464 <+36>:	callq  0x400410 <printf@plt>
>    0x0000000000400469 <+41>:	xor    %eax,%eax
>    0x000000000040046b <+43>:	add    $0x28,%rsp
>    0x000000000040046f <+47>:	retq
>   End of assembler dump.
> 
> With <foo> := barrier():
> 
>   (gdb) disassemble main
>   Dump of assembler code for function main:
>   [...]
>    0x0000000000400464 <+36>:	callq  0x400410 <printf@plt>
>    0x0000000000400469 <+41>:	movq   $0x0,(%rsp)
>    0x0000000000400471 <+49>:	movq   $0x0,0x8(%rsp)
>    0x000000000040047a <+58>:	movl   $0x0,0x10(%rsp)
>    0x0000000000400482 <+66>:	xor    %eax,%eax
>    0x0000000000400484 <+68>:	add    $0x28,%rsp
>    0x0000000000400488 <+72>:	retq
>   End of assembler dump.
> 
> As can be seen, movq, movq, movl are being emitted inlined
> via memset().
> 
> Reference: http://thread.gmane.org/gmane.linux.kernel.cryptoapi/13764/
> Fixes: d4c5efdb9777 ("random: add and use memzero_explicit() for clearing
> data") Cc: Hannes Frederic Sowa <hannes@stressinduktion.org>
> Cc: Stephan Mueller <smueller@chronox.de>
> Cc: Theodore Ts'o <tytso@mit.edu>
> Signed-off-by: mancha security <mancha1@zoho.com>
> Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
> ---
>  Sending to Herbert as crypto/random are the main users.
>  Based against -crypto tree. Thanks!
> 
>  lib/string.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/lib/string.c b/lib/string.c
> index ce81aae..a579201 100644
> --- a/lib/string.c
> +++ b/lib/string.c
> @@ -607,7 +607,7 @@ EXPORT_SYMBOL(memset);
>  void memzero_explicit(void *s, size_t count)
>  {
>  	memset(s, 0, count);
> -	OPTIMIZER_HIDE_VAR(s);
> +	barrier();
>  }
>  EXPORT_SYMBOL(memzero_explicit);

Acked-by: Stephan Mueller <smueller@chronox.de>

-- 
Ciao
Stephan

Re: [PATCH -crypto] lib: memzero_explicit: use barrier instead of OPTIMIZER_HIDE_VAR

[Herbert Xu]

On Wed, Mar 18, 2015 at 06:47:25PM +0100, Daniel Borkmann wrote:
> From: mancha security <mancha1@zoho.com>
> 
> OPTIMIZER_HIDE_VAR(), as defined when using gcc, is insufficient to
> ensure protection from dead store optimization.

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