Re: [PATCH] crypto: more robust crypto_memneq

[Cesar Eduardo Barros <cesarb@cesarb.eti.br>]

Em 25-11-2013 13:59, James Yonan escreveu:
> On 24/11/2013 14:12, Cesar Eduardo Barros wrote:
>> Disabling compiler optimizations can be fragile, since a new
>> optimization could be added to -O0 or -Os that breaks the assumptions
>> the code is making.
>>
>> Instead of disabling compiler optimizations, use a dummy inline assembly
>> (based on RELOC_HIDE) to block the problematic kinds of optimization,
>> while still allowing other optimizations to be applied to the code.
>>
>> The dummy inline assembly is added after every OR, and has the
>> accumulator variable as its input and output. The compiler is forced to
>> assume that the dummy inline assembly could both depend on the
>> accumulator variable and change the accumulator variable, so it is
>> forced to compute the value correctly before the inline assembly, and
>> cannot assume anything about its value after the inline assembly.
>>
>> This change should be enough to make crypto_memneq work correctly (with
>> data-independent timing) even if it is inlined at its call sites. That
>> can be done later in a followup patch.
>>
>> Compile-tested on x86_64.
>>
>> Signed-off-by: Cesar Eduardo Barros <cesarb@cesarb.eti.br>
>
> This approach using __asm__ ("" : "=r" (var) : "0" (var)) to try to
> prevent compiler optimizations of var is interesting.

It is not novel; I copied it from RELOC_HIDE.

> I like the fact that it's finer-grained than -Os and doesn't preclude
> inlining.
>
> One concern would be that __asm__ could be optimized out unless
> __volatile__ is present.

It cannot be optimized out, because the __asm__'s output is used. Of 
course, if the __asm__ output is never used, it could be optimized out, 
but that is the desired result in this case (it means the result of 
crypto_memneq is being ignored, so the whole function should be elided).

AFAIK, __volatile__ also has the side effect that it forces ordering 
with everything (this fact is used in the definition of the barrier() 
macro). This level of barrier is not needed for crypto_memneq; ordering 
on "neq" is more than enough.

-- 
Cesar Eduardo Barros
cesarb@cesarb.eti.br