Re: [musl] Compiler support for erasure of sensitive data

[Rich Felker <dalias at libc dot org>]

On Wed, Sep 09, 2015 at 12:36:01PM -0400, Zack Weinberg wrote:
> The first, simpler problem is strictly optimization.  explicit_bzero
> can be optimized to memset followed by a vacuous use of the memory
> region (generating no machine instructions, but preventing the stores
> from being deleted as dead); this is valuable because the sensitive
> data is often small and fixed-size, so the memset can in turn be
> replaced by inline code.  (This also facilitates implementation of
> -D_FORTIFY_SOURCE checks for explicit_bzero.)  Again looking at
> libressl, 92 of those 152 uses are improved by a crude version of this
> optimization:
> 
>     void explicit_bzero(void *, size_t);
>     extern inline __attribute__((gnu_inline, always_inline))
>     void explicit_bzero_constn(void *ptr, size_t len)
>     {
>       typedef struct {char x[len];} memblk;
>       memset(ptr, 0, len);
>       asm("" : : "m" (*(memblk __attribute__((may_alias)) *)ptr));
>     }
>     #define explicit_bzero(s, n)                          \
>       (__extension__(__builtin_constant_p(n) && (n) > 0   \
>                      ? explicit_bzero_constn(s, n)        \
>                      : explicit_bzero(s, n)))
> 
> I call this "crude" because it only works in GCC, when compiling C,
> and when the length parameter is compile-time constant.  GCC issues no
> error for this code when 'len' is not compile-time constant, but it is
> not documented to work reliably.  When compiling C++, GCC does not
> accept a structure containing an array whose size is not *lexically*
> constant; even if the body of explicit_bzero_constn is moved into the
> macro so that the whole thing is guarded by __builtin_constant_p,
> using explicit_bzero with a non-constant size will cause a compile
> error.  The same is true for Clang whether compiling C or C++.
> 
> This problem could be solved with a very simple feature addition:
> 
>     extern inline __attribute__((gnu_inline, always_inline))
>     void explicit_bzero(void *ptr, size_t len)
>     {
>       memset(ptr, 0, len);
>       __builtin_use_memory(ptr, len);
>     }

You're making this harder than it needs to be. The "m" constraint is
the wrong thing to use here. Simply use:

	__asm__(""::"r"(ptr):"memory");

The memory constraint implies that the asm can read or write any
memory that's reachable by it. The lack of output constraints implies
__volatile__ which is also needed.

Rich