Patchwork libcacard: don't leak vcard_emul_alloc_arrays mem

login
register
mail settings
Submitter Christophe Fergeau
Date July 4, 2011, 4:10 p.m.
Message ID <1309795843-4969-1-git-send-email-cfergeau@redhat.com>
Download mbox | patch
Permalink /patch/103139/
State New
Headers show

Comments

Christophe Fergeau - July 4, 2011, 4:10 p.m.
vcard_emul_mirror_card and vcard_emul_init use
vcard_emul_alloc_arrays to allocate memory for temporary arrays
which will contain elements that in the end will be used one by
one in cac_card_init. The arrays themselves are never stored
anywhere, they are only used as temporary containers. Hence
the memory that was allocated for these arrays should be freed
after use or they will be leaked.
---
 libcacard/vcard_emul_nss.c |   11 ++++++++++-
 1 files changed, 10 insertions(+), 1 deletions(-)
Christophe Fergeau - July 22, 2011, 11:30 a.m.
Ping?

On Mon, Jul 04, 2011 at 06:10:43PM +0200, Christophe Fergeau wrote:
> vcard_emul_mirror_card and vcard_emul_init use
> vcard_emul_alloc_arrays to allocate memory for temporary arrays
> which will contain elements that in the end will be used one by
> one in cac_card_init. The arrays themselves are never stored
> anywhere, they are only used as temporary containers. Hence
> the memory that was allocated for these arrays should be freed
> after use or they will be leaked.
> ---
>  libcacard/vcard_emul_nss.c |   11 ++++++++++-
>  1 files changed, 10 insertions(+), 1 deletions(-)
> 
> diff --git a/libcacard/vcard_emul_nss.c b/libcacard/vcard_emul_nss.c
> index de324ba..4fee471 100644
> --- a/libcacard/vcard_emul_nss.c
> +++ b/libcacard/vcard_emul_nss.c
> @@ -476,6 +476,7 @@ vcard_emul_mirror_card(VReader *vreader)
>      VCardKey **keys;
>      PK11SlotInfo *slot;
>      PRBool ret;
> +    VCard *card;
>  
>      slot = vcard_emul_reader_get_slot(vreader);
>      if (slot == NULL) {
> @@ -535,7 +536,12 @@ vcard_emul_mirror_card(VReader *vreader)
>      }
>  
>      /* now create the card */
> -    return vcard_emul_make_card(vreader, certs, cert_len, keys, cert_count);
> +    card = vcard_emul_make_card(vreader, certs, cert_len, keys, cert_count);
> +    qemu_free(certs);
> +    qemu_free(cert_len);
> +    qemu_free(keys);
> +
> +    return card;
>  }
>  
>  static VCardEmulType default_card_type = VCARD_EMUL_NONE;
> @@ -820,6 +826,9 @@ vcard_emul_init(const VCardEmulOptions *options)
>              vreader_free(vreader);
>              has_readers = PR_TRUE;
>          }
> +        qemu_free(certs);
> +        qemu_free(cert_len);
> +        qemu_free(keys);
>      }
>  
>      /* if we aren't suppose to use hw, skip looking up hardware tokens */
> -- 
> 1.7.5.4
> 
>
Alon Levy - July 22, 2011, 11:54 a.m.
On Fri, Jul 22, 2011 at 01:30:49PM +0200, Christophe Fergeau wrote:
> Ping?
> 

I'll pick it up in my pull request.

> On Mon, Jul 04, 2011 at 06:10:43PM +0200, Christophe Fergeau wrote:
> > vcard_emul_mirror_card and vcard_emul_init use
> > vcard_emul_alloc_arrays to allocate memory for temporary arrays
> > which will contain elements that in the end will be used one by
> > one in cac_card_init. The arrays themselves are never stored
> > anywhere, they are only used as temporary containers. Hence
> > the memory that was allocated for these arrays should be freed
> > after use or they will be leaked.
> > ---
> >  libcacard/vcard_emul_nss.c |   11 ++++++++++-
> >  1 files changed, 10 insertions(+), 1 deletions(-)
> > 
> > diff --git a/libcacard/vcard_emul_nss.c b/libcacard/vcard_emul_nss.c
> > index de324ba..4fee471 100644
> > --- a/libcacard/vcard_emul_nss.c
> > +++ b/libcacard/vcard_emul_nss.c
> > @@ -476,6 +476,7 @@ vcard_emul_mirror_card(VReader *vreader)
> >      VCardKey **keys;
> >      PK11SlotInfo *slot;
> >      PRBool ret;
> > +    VCard *card;
> >  
> >      slot = vcard_emul_reader_get_slot(vreader);
> >      if (slot == NULL) {
> > @@ -535,7 +536,12 @@ vcard_emul_mirror_card(VReader *vreader)
> >      }
> >  
> >      /* now create the card */
> > -    return vcard_emul_make_card(vreader, certs, cert_len, keys, cert_count);
> > +    card = vcard_emul_make_card(vreader, certs, cert_len, keys, cert_count);
> > +    qemu_free(certs);
> > +    qemu_free(cert_len);
> > +    qemu_free(keys);
> > +
> > +    return card;
> >  }
> >  
> >  static VCardEmulType default_card_type = VCARD_EMUL_NONE;
> > @@ -820,6 +826,9 @@ vcard_emul_init(const VCardEmulOptions *options)
> >              vreader_free(vreader);
> >              has_readers = PR_TRUE;
> >          }
> > +        qemu_free(certs);
> > +        qemu_free(cert_len);
> > +        qemu_free(keys);
> >      }
> >  
> >      /* if we aren't suppose to use hw, skip looking up hardware tokens */
> > -- 
> > 1.7.5.4
> > 
> >

Patch

diff --git a/libcacard/vcard_emul_nss.c b/libcacard/vcard_emul_nss.c
index de324ba..4fee471 100644
--- a/libcacard/vcard_emul_nss.c
+++ b/libcacard/vcard_emul_nss.c
@@ -476,6 +476,7 @@  vcard_emul_mirror_card(VReader *vreader)
     VCardKey **keys;
     PK11SlotInfo *slot;
     PRBool ret;
+    VCard *card;
 
     slot = vcard_emul_reader_get_slot(vreader);
     if (slot == NULL) {
@@ -535,7 +536,12 @@  vcard_emul_mirror_card(VReader *vreader)
     }
 
     /* now create the card */
-    return vcard_emul_make_card(vreader, certs, cert_len, keys, cert_count);
+    card = vcard_emul_make_card(vreader, certs, cert_len, keys, cert_count);
+    qemu_free(certs);
+    qemu_free(cert_len);
+    qemu_free(keys);
+
+    return card;
 }
 
 static VCardEmulType default_card_type = VCARD_EMUL_NONE;
@@ -820,6 +826,9 @@  vcard_emul_init(const VCardEmulOptions *options)
             vreader_free(vreader);
             has_readers = PR_TRUE;
         }
+        qemu_free(certs);
+        qemu_free(cert_len);
+        qemu_free(keys);
     }
 
     /* if we aren't suppose to use hw, skip looking up hardware tokens */