Re: [Qemu-devel] [PATCH 05/13] qcrypto-luks: clear the masterkey and password before freeing them always

[Daniel P . Berrangé]

On Sun, Aug 25, 2019 at 06:31:02PM +0300, Maxim Levitsky wrote:
> On Thu, 2019-08-22 at 13:56 +0300, Maxim Levitsky wrote:
> > On Thu, 2019-08-22 at 11:49 +0100, Daniel P. Berrangé wrote:
> > > On Tue, Aug 20, 2019 at 08:12:51PM +0200, Max Reitz wrote:
> > > > On 14.08.19 22:22, Maxim Levitsky wrote:
> > > > > While there are other places where these are still stored in memory,
> > > > > this is still one less key material area that can be sniffed with
> > > > > various side channel attacks
> > > > > 
> > > > > 
> > > > > 
> > > > 
> > > > (Many empty lines here)
> > > > 
> > > > > Signed-off-by: Maxim Levitsky <address@hidden>
> > > > > ---
> > > > >  crypto/block-luks.c | 52 
> > > > > ++++++++++++++++++++++++++++++++++++++-------
> > > > >  1 file changed, 44 insertions(+), 8 deletions(-)
> > > > 
> > > > Wouldn’t it make sense to introduce a dedicated function for this?
> > > 
> > > Yes, it would.
> > > 
> > > In fact I have a series pending which bumps min glib and introduces
> > > use of auto-free functions in this code.
> > > 
> > > It would be desirable to have a autp-free func for memset+free
> > > so we can just declare the variable
> > > 
> > >    q_autowipefree char *password = NULL;
> > > 
> > > and have it result in memset+free
> > > 
> > 
> > That is perfect.
> > When do you think you could post the series so that I could rebase
> > on top of it?
> 
> 
> I am thinking that I will keep my patch as is, just so that code is
> consistent in memsetting the secrets (even though as Nir pointed out,
> that these will be probably optimized away anyway).
> And then when you send your patch you will just remove all
> of these memsets.

I'm fine with you continuing to use memset, since this is a pre-existing
problem in the code that you are not making worse. We'll figure out the
fix separately.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|