Re: [PATCH crypto-2.6] lib: make memzero_explicit more robust against dead store elimination

[mancha security <mancha1@zoho.com>]

[-- Attachment #1: Type: text/plain, Size: 1991 bytes --]

On Wed, Apr 29, 2015 at 04:01:19PM +0200, Daniel Borkmann wrote:
> On 04/29/2015 03:08 PM, mancha security wrote:
> ...
> >By the way, has anyone been able to verify that __memory_barrier
> >provides DSE protection under various optimizations? Unfortunately, I
> >don't have ready access to ICC at the moment or I'd test it myself.
> 
> Never used icc, but it looks like it's free for open source projects;
> I can give it a try, but in case you're faster than I am, feel free
> to post results here.

Time permitting, I'll try setting this up and post my results.

> 
> From what I see based on the code, i.e. after that buggy cleanup
> commit ...
> 
> commit 73679e50820123ebdedc67ebcda4562d1d6e4aba
> Author: Pranith Kumar <bobby.prani@gmail.com>
> Date:   Tue Apr 15 12:05:22 2014 -0400
> 
>     compiler-intel.h: Remove duplicate definition
> 
>     barrier is already defined as __memory_barrier in compiler.h
>     Remove this unnecessary redefinition.
> 
>     Signed-off-by: Pranith Kumar <bobby.prani@gmail.com>
>     Link: http://lkml.kernel.org/r/CAJhHMCAnYPy0%2BqD-1KBnJPLt3XgAjdR12j%2BySSnPgmZcpbE7HQ@mail.gmail.com
>     Signed-off-by: H. Peter Anvin <hpa@linux.intel.com>
> 
> ... it looks like it's currently using the _same_ gcc inline asm
> for the barrier on icc instead of what that commit intended to do.
> 
> So funny enough, we don't actually use __memory_barrier() at the
> moment. ;)
> 
> Nonetheless, having a look might be good.

Nice catch, 73679e50820 is indeed buggy because ICC defines __GNUC__
(unless -no-gcc is used). That should be reverted.

Bug aside, according to [1], ICC does support GNU-style inline asm so
for the purposes of barrier_data(), it would be interesting to see if
it affords better/worse DSE protection compared to __memory_barrier().

--mancha

[1]
https://software.intel.com/sites/products/documentation/doclib/iss/2013/compiler/cpp-lin/GUID-5100C4FC-BC2F-4E36-943A-120CFFFB4285.htm


[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]