Re: [Qemu-devel] [PATCH for-2.9 v2] virtio-crypto: zeroize the key material before free

[Gonglei (Arei)]

Hi,

>
> Subject: Re: [Qemu-devel] [PATCH for-2.9 v2] virtio-crypto: zeroize the key
> material before free
> 
> On 08.12.2016 16:23, Eric Blake wrote:
> > On 12/07/2016 08:28 PM, Gonglei (Arei) wrote:
> >
> >>> As far as I'm aware, other projects usually have a special memset
> >>> variation for doing this. That is because compilers may choose to
> >>> "optimize" memset(p, ...) + free(p) to just the free(p). Having a
> >>
> >> Actually, I googled this, but I didn't find a definite answer. And
> >>
> >> The Linux kernel uses kzfree instead of memset + kfree
> (mm/slab_common.c).
> 
> Well, I personally don't mind whether we use a custom zfree() or a
> custom memset_s() + free(). I only mind that this patch actually always
> does what it is intended to do.
> 
Yes, but why linux kernel to think about the compiler optimization for
memset for sensitive data?

> > If we're worried about cleaning things without allowing the compiler a
> > chance to optimize, then writing our own qemu_zfree() wrapper may indeed
> > make sense.  But that won't cover the case in Daniel's earlier patch
> > (referenced elsewhere in this thread), as that was zeroizing stack
> > memory (before it went out of scope) rather than heap memory (before
> > free).  So you'd still need some sort of 'write this memory no matter
> > what' primitive that would be directly usable on stack memory and
> > indirectly used as part of the qemu_zfree() wrapper.
> >
If we wrap a secure_memset(), then both stack memory and heap
memory can use it.

Pls see:

http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1381.pdf

But the secure_memset() is not as efficient as possible due to the nature of 
the volatile
type qualifier preventing the compiler from optimizing the code at all. 

It will cause huge performance regression at hot path of data plane...

> > But I wouldn't worry about it for now, unless someone proves we actually
> > have a compiler optimizing away the cleanups.
> 
> It's true that we don't have to worry about it *now*, but the approach
> of waiting until the compiler breaks it does not seem right to me.
> 
> If at some point some compiler recognizes g_free() as being the same
> function as free() and thus optimizes memset() + g_free(), we simply
> won't notice because nobody will keep track of the generated assembly
> output all the time.
> 
> There is a reason C11 introduced memset_s(), and it is this.
> 
...does it make sense if we introduce secure_memset() now ? 

Waiting for your feedback. Thanks!


Regards,
-Gonglei