338214 - Reference leak in selfserv in FIPS140-2 mode

Status: RESOLVED FIXED

(NSS :: Libraries, defect, P1)

Description

[Julien Pierre]

The following 2 SSL tests are producing core files due to reference leaks assertions when running softoken in FIPS140-2 mode :

TLS Request don't require client auth (client does not provide auth)

Stress SSL3 RC4 128 with MD5

The stacks are :

(dbx) w
  [1] __lwp_kill(0x1, 0x6), at 0xfec00717
  [2] raise(0x6), at 0xfebaced3
  [3] abort(0x8046998, 0x806932c, 0xfeeed0b4, 0xfed6301f, 0x80466b0, 0xfee6f2f9), at 0xfeb90969
=>[4] PR_Assert(s = 0xfeed7278 "secmod_PrivateModuleCount == 0", file = 0xfeed7298 "pk11util.c", ln = 120), line 538 in "prlog.c"
  [5] SECMOD_Shutdown(), line 120 in "pk11util.c"
  [6] NSS_Shutdown(), line 792 in "nssinit.c"
  [7] main(argc = 16, argv = 0x80467e8), line 2048 in "selfserv.c"
(dbx)

Comment 1

[Julien Pierre]

The actual tests that dump core are not the ones reported, due to bug 338226 .

One core happens during the shutdown of selfserv at the end of the coverage tests. I don't know yet which client test is the trigger for this bug.

The other core happens during the stress test, but it is actually in the SSL2 stress test, when the server shuts down. As previously mentioned, the SSL2 ciphers don't work in FIPS mode.

I ran the server in single-threaded mode (I reduced MIN_THREADS to 1 in selfserv) so I'm confident this is not caused by a race. I believe it is probably related to SSL2. I will try to run things again without any SSL2 tests to see if both cores go away or only the stress test one.

Comment 2

[Julien Pierre]

Attachment: Fix slot leak in PK11_CreateContextByRawKey

Comment 3

[Nelson Bolyard (seldom reads bugmail)]

Comment on attachment 222300 [details] [diff] [review]
Fix slot leak in PK11_CreateContextByRawKey

Good catch.  r=nelson

Comment 4

[Wan-Teh Chang]

Comment on attachment 222300 [details] [diff] [review]
Fix slot leak in PK11_CreateContextByRawKey

r=wtc.  Please make the following two changes when
you check in.

>-    PK11_FreeSymKey(symKey);
>-    PK11_FreeSlot(slot);
>+    loser:

In NSS we don't indent labels.  Please put "loser:" at the
beginning of that line.

>+    if (slot) {
>+        PK11_FreeSlot(slot);
>+    }

Coverity will report that slot cannot possibly be NULL at
this point so the test is not necessary.  You can either
remove the test, or change the remaining "return NULL;"
statement in this function to "goto loser;".

Comment 5

[Julien Pierre]

Attachment: As checked in

Thanks for the quick reviews.

Tip:

Checking in pk11cxt.c;
/cvsroot/mozilla/security/nss/lib/pk11wrap/pk11cxt.c,v  <--  pk11cxt.c
new revision: 1.4; previous revision: 1.3
done

NSS_3_11_BRANCH :

Checking in pk11cxt.c;
/cvsroot/mozilla/security/nss/lib/pk11wrap/pk11cxt.c,v  <--  pk11cxt.c
new revision: 1.2.28.2; previous revision: 1.2.28.1
done