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

[Stefan Hajnoczi]

On Tue, Dec 06, 2016 at 01:30:12PM -0600, Eric Blake wrote:
> 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).

Okay.  I am not merging this for QEMU 2.8.0-rc3, it should go through
Michael Tsirkin's tree.

Stefan

signature.asc
Description: PGP signature