[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH for-2.8] virtio-crypto: zeroize the key material
From: |
Gonglei (Arei) |
Subject: |
Re: [Qemu-devel] [PATCH for-2.8] virtio-crypto: zeroize the key material before free |
Date: |
Wed, 7 Dec 2016 00:57:57 +0000 |
>
> From: Stefan Hajnoczi [mailto:address@hidden
> Sent: Wednesday, December 07, 2016 4:18 AM
> To: Eric Blake
> Cc: Michael S. Tsirkin; Gonglei (Arei); address@hidden; Daniel P.
> Berrange
> Subject: Re: [Qemu-devel] [PATCH for-2.8] virtio-crypto: zeroize the key
> material before free
>
> 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.
>
It's fair, let me send V2 for 2.9 with clearer commit message.
Thank you, guys~
Regards,
-Gonglei