Closed Bug 427706 Opened 13 years ago Closed 13 years ago

NSS_3_12_RC1 crashes in passwordmgr tests

Categories

(NSS :: Libraries, defect, P1)

defect

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: KaiE, Assigned: rrelyea)

References

(Depends on 1 open bug, Blocks 1 open bug)

Details

(Keywords: regression)

Attachments

(3 files, 2 obsolete files)

NSS_3_12_RC1 crashes when executing Mozilla's passwordmgr unit tests.
Blocks: 426681
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 ***
Attached file stack
#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}}
This crash has been introduced by bug 423093.

I tested reverting file pk11sdr.c to revision 1.19, and it fixes the crash.
Depends on: 423093
Let's go with 1.19  In the NSS RC1 tag.

bob
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.
Attachment #314371 - Flags: review?
Attachment #314372 - Flags: review?(kengert)
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
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
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.
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.
Attachment #314371 - Flags: review? → review?(slavomir.katuscak)
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.
Priority: -- → P1
Target Milestone: --- → 3.12
Attached patch Fix the test case. (obsolete) — Splinter Review
Attachment #314371 - Attachment is obsolete: true
Attachment #316893 - Flags: review?(slavomir.katuscak)
Attachment #314371 - Flags: review?(slavomir.katuscak)
Attachment #316893 - Attachment is patch: true
Attachment #316893 - Attachment mime type: application/octet-stream → text/plain
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 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.
Attachment #316893 - Flags: review?(slavomir.katuscak) → review-
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.
Attachment #316893 - Attachment is obsolete: true
Attachment #316994 - Flags: review?(rrelyea)
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
Attachment #314372 - Flags: review?(kengert) → review+
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
Attachment #316994 - Flags: review?(rrelyea) → review+
Checking in sdr.sh;
/cvsroot/mozilla/security/nss/tests/sdr/sdr.sh,v  <--  sdr.sh
new revision: 1.8; previous revision: 1.7
done
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.
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.
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
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.
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.