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

[Eric Blake]

On 12/06/2016 01:22 PM, Michael S. Tsirkin wrote:
> On Tue, Dec 06, 2016 at 05:33:37PM +0000, Stefan Hajnoczi wrote:
>> On Tue, Dec 06, 2016 at 03:40:49PM +0200, Michael S. Tsirkin wrote:
>>> On Tue, Dec 06, 2016 at 05:29:13PM +0800, Gonglei wrote:
>>>> Zeroize the memory of CryptoDevBackendSymOpInfo structure pointed
>>>> for key material security.
>>>>

>>>> +            /* Zeroize and free request data structure */
>>>> +            memset(op_info, 0, sizeof(*op_info) + max_len);
>>>> +            g_free(op_info);
>>>
>>> Write into memory, then free it?  This looks rather strange. Why are we
>>> doing this?
>>
>> Common practice with sensitive information (key material, passwords,
>> etc).
> 
> Some kind of explanation about what makes this one
> more sensitive than others would be nice.

Even mentioning existing practice would go a long way; see commit 8813800b.

> 
> Also, what makes it 2.8 material? Considering the pointer math
> involved, it's not risk-free.
> 
>> coredumps, memory disclosure bugs when heap memory is reused, etc.
>>
>> Sensitive information is sometimes also held in mlocked pages to prevent
>> it being swapped to disk but that's not being done here.

And existing practice is that we aren't going to be that paranoid at
this time (and yes, I asked Dan that same question on his commit
mentioned above).

>>
>> Perhaps the comment should be more explicit but this patch seems
>> reasonable.
> 
> Right.  One can see memset and free at a glance.
> The comment and the commit log should explain the why,
> not the what.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

signature.asc
Description: OpenPGP digital signature