qemu-devel
[Top][All Lists]
Advanced

[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: Stefan Hajnoczi
Subject: Re: [Qemu-devel] [PATCH for-2.8] virtio-crypto: zeroize the key material before free
Date: Tue, 6 Dec 2016 17:33:37 +0000
User-agent: Mutt/1.7.1 (2016-10-04)

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.
> > 
> > Signed-off-by: Gonglei <address@hidden>
> > ---
> >  hw/virtio/virtio-crypto.c | 13 ++++++++++++-
> >  1 file changed, 12 insertions(+), 1 deletion(-)
> > 
> > diff --git a/hw/virtio/virtio-crypto.c b/hw/virtio/virtio-crypto.c
> > index 2f2467e..ecb19b6 100644
> > --- a/hw/virtio/virtio-crypto.c
> > +++ b/hw/virtio/virtio-crypto.c
> > @@ -337,7 +337,18 @@ static void virtio_crypto_free_request(VirtIOCryptoReq 
> > *req)
> >  {
> >      if (req) {
> >          if (req->flags == CRYPTODEV_BACKEND_ALG_SYM) {
> > -            g_free(req->u.sym_op_info);
> > +            size_t max_len;
> > +            CryptoDevBackendSymOpInfo *op_info = req->u.sym_op_info;
> > +
> > +            max_len = op_info->iv_len +
> > +                      op_info->aad_len +
> > +                      op_info->src_len +
> > +                      op_info->dst_len +
> > +                      op_info->digest_result_len;
> > +
> > +            /* 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).

Prevents sensitive information from being exposed by accident later in
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.

Perhaps the comment should be more explicit but this patch seems
reasonable.

Reviewed-by: Stefan Hajnoczi <address@hidden>

Attachment: signature.asc
Description: PGP signature


reply via email to

[Prev in Thread] Current Thread [Next in Thread]