• mancha security's avatar
    lib: memzero_explicit: use barrier instead of OPTIMIZER_HIDE_VAR · 99c5da5e
    mancha security authored
    commit 0b053c95 upstream.
    
    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: d4c5efdb ("random: add and use memzero_explicit() for clearing data")
    Cc: Theodore Ts'o <tytso@mit.edu>
    Signed-off-by: default avatarmancha security <mancha1@zoho.com>
    Signed-off-by: default avatarDaniel Borkmann <daniel@iogearbox.net>
    Acked-by: default avatarHannes Frederic Sowa <hannes@stressinduktion.org>
    Acked-by: default avatarStephan Mueller <smueller@chronox.de>
    Signed-off-by: default avatarHerbert Xu <herbert@gondor.apana.org.au>
    Signed-off-by: default avatarKamal Mostafa <kamal@canonical.com>
    99c5da5e
string.c 16.6 KB