Am Mittwoch, 18. März 2015, 11:56:43 schrieb Daniel Borkmann:
Hi Daniel,
>On 03/18/2015 11:50 AM, Hannes Frederic Sowa wrote:
>> On Wed, Mar 18, 2015, at 10:53, mancha wrote:
>>> Hi.
>>>
>>> The kernel RNG introduced memzero_explicit in d4c5efdb9777 to
>>> protect
>>>
>>> memory cleansing against things like dead store optimization:
>>> void memzero_explicit(void *s, size_t count)
>>> {
>>>
>>> memset(s, 0, count);
>>> OPTIMIZER_HIDE_VAR(s);
>>>
>>> }
>>>
>>> OPTIMIZER_HIDE_VAR, introduced in fe8c8a126806 to protect
>>> crypto_memneq>>
>>> against timing analysis, is defined when using gcc as:
>>> #define OPTIMIZER_HIDE_VAR(var) __asm__ ("" : "=r" (var) : "0"
>>> (var))
>>>
>>> My tests with gcc 4.8.2 on x86 find it insufficient to prevent gcc
>>> from optimizing out memset (i.e. secrets remain in memory).
>>>
>>> Two things that do work:
>>> __asm__ __volatile__ ("" : "=r" (var) : "0" (var))
>>
>> You are correct, volatile signature should be added to
>> OPTIMIZER_HIDE_VAR. Because we use an output variable "=r", gcc is
>> allowed to check if it is needed and may remove the asm statement.
>> Another option would be to just use var as an input variable - asm
>> blocks without output variables are always considered being volatile
>> by gcc.
>>
>> Can you send a patch?
>>
>> I don't think it is security critical, as Daniel pointed out, the
>> call
>> will happen because the function is an external call to the crypto
>> functions, thus the compiler has to flush memory on return.
>
>Just had a look.
>
>$ 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
>[...]
> 0xffffffff814a5000 <+304>: sub %r15,%rbx
> 0xffffffff814a5003 <+307>: jne 0xffffffff814a4f80
><extract_entropy+176> 0xffffffff814a5009 <+313>: mov %r12,%rdi
> 0xffffffff814a500c <+316>: mov $0xa,%esi
> 0xffffffff814a5011 <+321>: callq 0xffffffff813a18b0
><memzero_explicit> 0xffffffff814a5016 <+326>: mov -0x48(%rbp),%rax
>[...]
>
>I would be fine with __volatile__.
Are we sure that simply adding a __volatile__ works in any case? I just
did a test with a simple user space app:
static inline void memset_secure(void *s, int c, size_t n)
{
memset(s, c, n);
//__asm__ __volatile__("": : :"memory");
__asm__ __volatile__("" : "=r" (s) : "0" (s));
}
int main(int argc, char *argv[])
{
#define BUFLEN 20
char buf[BUFLEN];
snprintf(buf, (BUFLEN - 1), "teststring\n");
printf("%s", buf);
memset_secure(buf, 0, BUFLEN);
}
When using the discussed code of __asm__ __volatile__("" : "=r" (s) :
"0" (s)); I do not find the code implementing memset(0) in objdump.
Only when I enable the memory barrier, I see the following (when
compiling with -O2):
objdump -d memset_secure:
...
0000000000400440 <main>:
...
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
...
>
>Thanks a lot mancha, could you send a patch?
>
>Best,
>Daniel
>--
>To unsubscribe from this list: send the line "unsubscribe linux-crypto"
>in the body of a message to majordomo@xxxxxxxxxxxxxxx
>More majordomo info at http://vger.kernel.org/majordomo-info.html
Ciao
Stephan
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
