From: mancha security <[email protected]>
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 <[email protected]>
Cc: Stephan Mueller <[email protected]>
Cc: Theodore Ts'o <[email protected]>
Signed-off-by: mancha security <[email protected]>
Signed-off-by: Daniel Borkmann <[email protected]>
---
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
On Wed, Mar 18, 2015, at 18:47, Daniel Borkmann wrote:
> From: mancha security <[email protected]>
>
> 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 <[email protected]>
> Cc: Stephan Mueller <[email protected]>
> Cc: Theodore Ts'o <[email protected]>
> Signed-off-by: mancha security <[email protected]>
> Signed-off-by: Daniel Borkmann <[email protected]>
> ---
> Sending to Herbert as crypto/random are the main users.
> Based against -crypto tree. Thanks!
Acked-by: Hannes Frederic Sowa <[email protected]>
Still checking on how to realize the test. Thanks!
Am Mittwoch, 18. M?rz 2015, 18:47:25 schrieb Daniel Borkmann:
Hi Daniel,
> From: mancha security <[email protected]>
>
> 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 <[email protected]>
> Cc: Stephan Mueller <[email protected]>
> Cc: Theodore Ts'o <[email protected]>
> Signed-off-by: mancha security <[email protected]>
> Signed-off-by: Daniel Borkmann <[email protected]>
> ---
> 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 <[email protected]>
--
Ciao
Stephan
On Wed, Mar 18, 2015 at 06:47:25PM +0100, Daniel Borkmann wrote:
> From: mancha security <[email protected]>
>
> OPTIMIZER_HIDE_VAR(), as defined when using gcc, is insufficient to
> ensure protection from dead store optimization.
Patch applied. Thanks!
--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt