518446 - PK11_DEREncodePublicKey leaks a CERTSubjectPublicKeyInfo

Status: RESOLVED FIXED

(NSS :: Libraries, defect)

Description

[Wan-Teh Chang]

Attachment: Proposed patch

PK11_DEREncodePublicKey creates a CERTSubjectPublicKeyInfo
object but forgets to destroy it.  This is found by running
valgrind:

11:38:29 memcheck_analyze.py [ERROR] Leak_DefinitelyLost
2,195 (36 direct, 2,159 indirect) bytes in 1 blocks are definitely lost in loss record 808 of 819
  calloc (ome/chrome-bot/valgrind-20090715/coregrind/m_replacemalloc/vg_replace_malloc.c:415)
  PR_Calloc (ild/buildd/nspr-4.7.1+1.9/mozilla/nsprpub/pr/src/malloc/prmem.c:474)
  PORT_ZAlloc_Util (ild/buildd/nss-3.12.0.3/mozilla/security/nss/lib/util/secport.c:140)
  PORT_NewArena_Util (ild/buildd/nss-3.12.0.3/mozilla/security/nss/lib/util/secport.c:198)
  SECKEY_CreateSubjectPublicKeyInfo (ild/buildd/nss-3.12.0.3/mozilla/security/nss/lib/cryptohi/seckey.c:1787)
  PK11_DEREncodePublicKey (ild/buildd/nss-3.12.0.3/mozilla/security/nss/lib/pk11wrap/pk11akey.c:1575)

Comment 1

[Wan-Teh Chang]

Attachment: Proposed patch v2

Remove a redundant NULL check.  Just rely on
SECKEY_CreateSubjectPublicKeyInfo, which not
only does a NULL check but also sets the
SEC_ERROR_INVALID_ARGS error code.

Comment 2

[Wan-Teh Chang]

Attachment: Fix similar problems in cmd

Memory leaks are less serious in cmd, so I don't want
to waste your precious time reviewing this patch.  Feel
free to ignore this patch.

Comment 3

[Nelson Bolyard (seldom reads bugmail)]

Wan-Teh, as you know, we have Tinderboxes that run continuous memory leak
tests.  When they find a memory leak, Slavo files a bug on it, and enters 
an entry into the file of "ignored" leak stacks.  

Now I wonder which of the following explains the leak reported in this bug.
a) this is a duplicate of a reported and presently ignored leak.
b) this is a leak that is not detected by our tinderbox leak tests. (why not?)
c) other?

Comment 4

[Wan-Teh Chang]

MXR shows that PK11_DEREncodePublicKey is only used by JSS:
http://mxr.mozilla.org/security/ident?i=PK11_DEREncodePublicKey
http://mxr.mozilla.org/mozilla-central/ident?i=PK11_DEREncodePublicKey

That's why our Tinderboxes can't detect this memory leak.

Comment 5

[Robert Relyea]

Comment on attachment 402455 [details] [diff] [review]
Proposed patch v2

r+ rrelyea

Comment 6

[Robert Relyea]

Comment on attachment 402457 [details] [diff] [review]
Fix similar problems in cmd

r+ rrelyea

Let's fix the command leaks as well.

Comment 7

[Wan-Teh Chang]

I checked in the patches on the NSS trunk (NSS 3.12.5).

Checking in pk11akey.c;
/cvsroot/mozilla/security/nss/lib/pk11wrap/pk11akey.c,v  <--  pk11akey.c
new revision: 1.28; previous revision: 1.27
done

Checking in certutil.c;
/cvsroot/mozilla/security/nss/cmd/certutil/certutil.c,v  <--  certutil.c
new revision: 1.147; previous revision: 1.146
done

Checking in certgen.c;
/cvsroot/mozilla/security/nss/cmd/signtool/certgen.c,v  <--  certgen.c
new revision: 1.14; previous revision: 1.13
done