Re: [PATCH RFC] explicit_bzero, again

[Zack Weinberg <zackw at panix dot com>]

On Fri, Aug 21, 2015 at 10:29 AM, Florian Weimer <fweimer@redhat.com> wrote:
> On 08/21/2015 04:24 PM, Zack Weinberg wrote:
>> On Fri, Aug 21, 2015 at 2:37 AM, Florian Weimer <fweimer@redhat.com> wrote:
>>> On 08/16/2015 06:43 PM, Zack Weinberg wrote:
>>>> +@strong{Warning:} The compiler is free to make additional copies of
>>>> +any object, or parts of it, in temporary storage areas (such as
>>>> +registers and ``scratch'' stack space).  @code{explicit_bzero} does
>>>> +not guarantee that temporary copies of sensitive data are destroyed.
>>>
>>> Perhaps you should add that explicit_bzero can create the copy which it
>>> is about to overwrite, leaving the original untouched.
...
> Oh, if you find it surprising, it is certainly worth documenting.
> Here's an example:
>
>   <https://gcc.gnu.org/ml/gcc-help/2014-10/msg00071.html>
>
> Any variable which is not already addressable for another reason is
> likely to trigger such behavior.

Thanks for the example.  I will add this to the manual in the next
revision of the patch.

>>> A partial
>>> countermeasure could be a barrier with register clobbers for as many
>>> caller-saved registers as possible.
...
>> are you saying that this barrier should be part of
>> explicit_bzero itself, or something the application needs to do?
>
> Calling explicit_bzero should take care of that, I think.  I'm not
> completely sure how to achieve that.  It might be sufficient to put the
> barrier into the implementation, or it might have to be in a wrapper in
> a header file.

    asm ("" ::: /* all call-preserved registers */)

won't actually *do* anything to erase old values.  If they are live,
they will get spilled to the stack (another copy!) and pulled back in
when needed.  If they are dead, the compiler will just shrug and move
on.  OK, so we have to actually emit assembly instructions to clear
call-preserved registers.  That could be a substantial deoptimization,
particularly in cases where there are several calls to explicit_bzero
in a row (e.g. my changes to crypt/)  and on architectures with large
register files.  For ia64 (is that still supported?) I'm not sure it's
*possible* to write an appropriate assembly insert because how do I
even know how many registers are live in the current call frame?

I could see adding a *separate* macro (I think it has to be a macro) like so

    #define clear_register_file() do { \
        __asm__("xorl %eax, %eax" ::: "eax") \
        __asm__("xorl %ebx, %ebx" ::: "ebx") \
        __asm__("xorl %ecx, %ecx" ::: "ecx") \
        __asm__("xorl %edx, %edx" ::: "edx") \
        __asm__("xorl %esi, %esi" ::: "esi") \
        __asm__("xorl %edi, %edi" ::: "edi") \
        __asm__("xorl %ebp, %ebp" ::: "ebp") \
        /* ... etc etc FPU and vector registers */ \
    } while (0)

You'd use this once right before returning from a function that
manipulated sensitive data.  It would still be a pretty hefty
deoptimization, and it wouldn't do anything about scratch stack slots.
I actually think scratch stack slots are a bigger concern; I've heard
about a lot more unintended data leaks via memory than via the
register file.

All in all I feel that the Right Thing is to not worry about this
beyond mentioning it in documentation -- again, the situation is not
worse than what people are hand-rolling right now -- and put the
effort into adding compiler intrinsics that will let us do the job
_properly_.

zw