From 0b053c9518292705736329a8fe20ef4686ffc8e9 Mon Sep 17 00:00:00 2001 From: mancha security Date: Wed, 18 Mar 2015 18:47:25 +0100 Subject: [PATCH] lib: memzero_explicit: use barrier instead of OPTIMIZER_HIDE_VAR 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 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 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); } int main(void) { char buff[20]; snprintf(buff, sizeof(buff) - 1, "test"); printf("%s", buff); memzero_explicit(buff, sizeof(buff)); return 0; } With := OPTIMIZER_HIDE_VAR(): (gdb) disassemble main Dump of assembler code for function main: [...] 0x0000000000400464 <+36>: callq 0x400410 0x0000000000400469 <+41>: xor %eax,%eax 0x000000000040046b <+43>: add $0x28,%rsp 0x000000000040046f <+47>: retq End of assembler dump. With := barrier(): (gdb) disassemble main Dump of assembler code for function main: [...] 0x0000000000400464 <+36>: callq 0x400410 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: Theodore Ts'o Signed-off-by: mancha security Signed-off-by: Daniel Borkmann Acked-by: Hannes Frederic Sowa Acked-by: Stephan Mueller Signed-off-by: Herbert Xu --- lib/string.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/string.c b/lib/string.c index ce81aaec3839..a5792019193c 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);