On Wed, Mar 18, 2015, at 13:14, Stephan Mueller wrote: > Am Mittwoch, 18. März 2015, 13:02:12 schrieb Hannes Frederic Sowa: > > Hi Hannes, > > >On Wed, Mar 18, 2015, at 12:09, Stephan Mueller wrote: > >> Am Mittwoch, 18. März 2015, 11:56:43 schrieb Daniel Borkmann: > >> >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)); > >> > >> } > > > >Good point, thanks! > > > >Of course an input or output of s does not force the memory pointed to > >by s being flushed. > > > > > >My proposal would be to add a > > > >#define OPTIMIZER_HIDE_MEM(ptr, len) __asm__ __volatile__ ("" : : "m"( > >({ struct { u8 b[len]; } *p = (void *)ptr ; *p; }) ) > > > >and use this in the code function. > > > >This is documented in gcc manual 6.43.2.5. > > That one adds the zeroization instructuctions. But now there are much > more than with the barrier. > > 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 > 400482: 48 c7 44 24 20 00 00 movq $0x0,0x20(%rsp) > 400489: 00 00 > 40048b: 48 c7 44 24 28 00 00 movq $0x0,0x28(%rsp) > 400492: 00 00 > 400494: c7 44 24 30 00 00 00 movl $0x0,0x30(%rsp) > 40049b: 00 > > Any ideas?
Hmm, correct definition of u8? Which version of gcc do you use? I can't see any difference if I compile your example at -O2. Bye, Hannes -- To unsubscribe from this list: send the line "unsubscribe linux-crypto" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html