427706 - NSS_3_12_RC1 crashes in passwordmgr tests

Status: RESOLVED FIXED

(NSS :: Libraries, defect, P1)

Description

[Kai Engert [:KaiE:]]

NSS_3_12_RC1 crashes when executing Mozilla's passwordmgr unit tests.

Comment 1

[Kai Engert [:KaiE:]]

The failing script:

http://lxr.mozilla.org/seamonkey/source/toolkit/components/passwordmgr/test/unit/test_storage_legacy_1.js


*** glibc detected *** ../../../../dist/bin/xpcshell: free(): invalid pointer: 0x0aa43b24 ***

Comment 2

[Kai Engert [:KaiE:]]

Attachment: stack

Comment 3

[Kai Engert [:KaiE:]]

#9  0x00eff99b in PK11SDR_Decrypt (data=0xbf919ec8, result=0xbf919ebc, cx=0xaa43b38) at pk11sdr.c:387
387                         SECITEM_ZfreeItem(&sdrResult.data, PR_FALSE);
(gdb) l
382                     if (possibleResult.data) {
383                         /* this is unlikely but possible. If we hit this condition,
384                          * we have no way of knowing which possibility to prefer.
385                          * in this case we just match the key the application
386                          * thought was the right one */
387                         SECITEM_ZfreeItem(&sdrResult.data, PR_FALSE);
388                     } else {
389                         possibleResult = sdrResult.data;
390                     }
391                 }
(gdb) print sdrResult
$1 = {keyid = {type = siBuffer, data = 0xaa43afc "�", len = 16}, alg = {algorithm = {type = siBuffer,
      data = 0xaa43b10 "*\206H\206�\r\003\a\004\b��\212]�\035��\004\b", len = 8}, parameters = {type = siBuffer,
      data = 0xaa43b18 "\004\b��\212]�\035��\004\b", len = 10}}, data = {type = siBuffer, data = 0xaa43b24 "", len = 8}}

Comment 4

[Kai Engert [:KaiE:]]

This crash has been introduced by bug 423093.

I tested reverting file pk11sdr.c to revision 1.19, and it fixes the crash.

Comment 5

[Robert Relyea]

Let's go with 1.19  In the NSS RC1 tag.

bob

Comment 6

[Christophe Ravel]

The tag NSS_3_12_RC1 was already created. We should not change it.
I'd suggest that we tag the tree as NSS_3_12_RC2 (with pk11sdr.c reverted to what it was in rev 1.19).

We also need to reopen bug 423093.

Comment 7

[Robert Relyea]

Attachment: NSS test case to reporduce the problem

Comment 8

[Robert Relyea]

Attachment: patch to resolve the issue

Comment 9

[Robert Relyea]

I think the  best way to resolve the mozilla issue is to include 3.19 in NSS.

call int RC2.
We can add this batch to what will become NSS 3.12.1

bob

Comment 10

[Robert Relyea]

ok, I've created a new NSS_3_12_RC2 tag, christophe,can you verify that it's correct. I've reverted pk11sdr.c to revision 1.19 by checking the reverted code in (1.21). I also bumped the build number in nss.h


bob

Comment 11

[Christophe Ravel]

To be more consistent, you could also have changed the version in softkver.h to the same version as in nss.h. But now that the tag is done, it is not worth changing. RC2 is not a true release candidate since we still have to fix bug 423093. We'll have to take care of the softoken version for the next RC.

Comment 12

[Kai Engert [:KaiE:]]

I applied Bob's patches and ran a local build and the test suite.
I got 4 SDR failures.
I've sent email to Bob with more details.

Comment 13

[Kai Engert [:KaiE:]]

The following test always fails on my system:

sdr.sh: Decrypt - Value 2
sdrtest -d . -i /home/kaie/moz/nss/head/mozilla/tests_results/security/localhost.19/tests.v2.24047 -t The quick brown fox jumped over the lazy dog
sdr.sh: #5: Decrypt - Value 2  - FAILED


In the previous comment I had reported 4 failures. I think this is because all.sh runs all tests 4 times.

Comment 14

[Robert Relyea]

Attachment: Fix the test case.

Comment 15

[Robert Relyea]

Kai, could you run the tests again with the new testcase.
Also, be sure to apply the patch to version 1.20, not 1.21 (which has the whole set of code backed out).

bob

Comment 16

[Slavomir Katuscak]

Comment on attachment 316893 [details] [diff] [review]
Fix the test case.

Latest test will not work, using -o instead of -i. Also I see some mess in naming of variables and output messages, I prepared new patch for this.

Comment 17

[Slavomir Katuscak]

Attachment: Test case + some cleanup.

Patch based on Bob's patch. 

Changes:
1. Fixed -o -> -i.
2. Using unused T1 string and changed SMALLPAD to T3 to have the same naming for similar tests (if there is a reason for naming SMALLPAD, feel free to add comment to my patch). 
3. Changed HTML messages to unique format (3 similar tests had 3 completely different messages) and fixed confusing " ' in echo messages to match following command.

Comment 18

[Kai Engert [:KaiE:]]

Comment on attachment 314372 [details] [diff] [review]
patch to resolve the issue

When I combine this patch with cvs version 1.20 and test it with attachment 316994 [details] [diff] [review], then it works.

r=kengert

Comment 19

[Robert Relyea]

Comment on attachment 316994 [details] [diff] [review]
Test case + some cleanup.

Good catch slavo...

please fix the -o in the echo statement for VALUE3 to be -i before you check in.

bob

Comment 20

[Slavomir Katuscak]

Checking in sdr.sh;
/cvsroot/mozilla/security/nss/tests/sdr/sdr.sh,v  <--  sdr.sh
new revision: 1.8; previous revision: 1.7
done

Comment 21

[Nelson Bolyard (seldom reads bugmail)]

IS this bug fixed?
Did the fix get into 3.12.0 RC4?  
Or will it first appear in 3.12.1 ?
If the latter, please update the target fix milestone for this bug.

Comment 22

[Kai Engert [:KaiE:]]

Nelson, we did what was said in comment 4, 5 and 6.
The RC 2 tag was created with the regression culprit backed out.
The crash was fixed.

In order to fix the bad patch (from bug 423093), in preparation for landing it again, a patch attached to this bug has been proposed, it's attachment 314372 [details] [diff] [review].

But apparently the original patch + incremental patch have not yet been checked in to trunk.

I think, for clarity, the incremental patch (attachment 314372 [details] [diff] [review]) should get moved over to bug 423093, and this bug (the crasher) should get closed.

Comment 23

[Robert Relyea]

checked into 3.12.1
Checking in pk11sdr.c;
/cvsroot/mozilla/security/nss/lib/pk11wrap/pk11sdr.c,v  <--  pk11sdr.c
new revision: 1.22; previous revision: 1.21
done

Comment 24

[Nelson Bolyard (seldom reads bugmail)]

I agree with Kai,
This bug (the crash) was fixed in 3.12.0.
As a result of the above checkin, Bug 423093 should be marked fixed
in 3.12.1.