[PATCH 2/2] Make _gcry_burn_stack use variable length array

[Werner Koch]

On Tue,  3 Sep 2013 19:18, jussi.kivilinna at iki.fi said:

> 64-byte stack buffer instead of burn stack deeper. It's mentioned in GCC
> bugzilla that _gcry_burn_stack is doing wrong thing here [1] and that this
> kind of optimization is allowed.

>From [1]:

  This is also a libgcrypt bug, because clearly if it wants to burn
  some portion of the stack (assuming for security reasons), then
  
  1) if it doesn't prevent tail recursion or tail call to itself, it
  doesn't do what it is trying to do, in particular it keeps overwriting
  with zeros the same portion of memory over and over
  
  2) even if it isn't tail recursion/call optimized, on most targets it
  will still leave gaps on the stack not overwritten
  
  So, all in all, quite questionable way of burning cycles in the crypto
  library.dd

Right, this is a about security.  It is actual a standard practice which
has been around for 20 years or so.

I have not seen a reference to the C specs that explains that
optimization of tail recursion is allowed.  Doubtless this one of those
esoteric interpretations of the standard.  Turning C into a high level
language is a Bad Thing(tm).  People who want that should use a high
level language.  I wonder who long it will take that de-facto safe
assumptions, like CHAR_BIT is 8, will be sacrificed for yet another small
performance optimization.  Do that for C++ but keep hands off the things
which worked well for more than 30 years.  This was not the first case
were stretching the interpretation of the standard opened holes in
existing software.  Right there is a standard, but there is also a
common understanding on how certain things are to be interpreted.  The
latter is more important than the wording of standard.

And yes, there might be gaps on the stack.  But for any straight forward
C compiler that would not matter at all.  We merely try our best to not
leave traces of secrets on the stack while being portable.  And while we
are at it: Why is paged out memory still not encrypted by default?

Sorry for the rant.
  
> So lets fix _gcry_burn_stack by using variable length array when VLAs
> are supported by compiler. This should ensure proper stack burning to
> the requested depth and avoid GCC loop optimizations.

Okay, that might work for a while.

> +#ifdef HAVE_VLA
> +    int buflen = (((bytes <= 0) ? 1 : bytes) + 63) & ~63;
> +    char buf[buflen];

Add volatile ?


Shalom-Salam,

   Werner

-- 
Die Gedanken sind frei.  Ausnahmen regelt ein Bundesgesetz.