The default bug view has changed. See this FAQ.

NSS_3_12_RC1 crashes in passwordmgr tests

RESOLVED FIXED in 3.12

Status

NSS
Libraries
P1
normal
RESOLVED FIXED
9 years ago
9 years ago

People

(Reporter: kaie, Assigned: Robert Relyea)

Tracking

(Depends on: 1 bug, Blocks: 1 bug, {regression})

trunk
3.12
regression
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments, 2 obsolete attachments)

(Reporter)

Description

9 years ago
NSS_3_12_RC1 crashes when executing Mozilla's passwordmgr unit tests.
(Reporter)

Updated

9 years ago
Blocks: 426681
(Reporter)

Comment 1

9 years ago
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 ***
(Reporter)

Comment 2

9 years ago
Created attachment 314271 [details]
stack
(Reporter)

Comment 3

9 years ago
#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}}
(Reporter)

Comment 4

9 years ago
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
(Assignee)

Comment 5

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

bob

Comment 6

9 years ago
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.
(Assignee)

Comment 7

9 years ago
Created attachment 314371 [details] [diff] [review]
NSS test case to reporduce the problem
Attachment #314371 - Flags: review?
(Assignee)

Comment 8

9 years ago
Created attachment 314372 [details] [diff] [review]
patch to resolve the issue
Attachment #314372 - Flags: review?(kengert)
(Assignee)

Comment 9

9 years ago
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
(Assignee)

Comment 10

9 years ago
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

9 years ago
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.
(Reporter)

Comment 12

9 years ago
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.
(Assignee)

Updated

9 years ago
Attachment #314371 - Flags: review? → review?(slavomir.katuscak)
(Reporter)

Comment 13

9 years ago
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
(Assignee)

Comment 14

9 years ago
Created attachment 316893 [details] [diff] [review]
Fix the test case.
Attachment #314371 - Attachment is obsolete: true
Attachment #316893 - Flags: review?(slavomir.katuscak)
Attachment #314371 - Flags: review?(slavomir.katuscak)
(Reporter)

Updated

9 years ago
Attachment #316893 - Attachment is patch: true
Attachment #316893 - Attachment mime type: application/octet-stream → text/plain
(Assignee)

Comment 15

9 years ago
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

9 years ago
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-

Comment 17

9 years ago
Created attachment 316994 [details] [diff] [review]
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.
Attachment #316893 - Attachment is obsolete: true
Attachment #316994 - Flags: review?(rrelyea)
(Reporter)

Comment 18

9 years ago
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+
(Assignee)

Comment 19

9 years ago
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+

Comment 20

9 years ago
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.
Blocks: 436376
(Reporter)

Comment 22

9 years ago
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.
(Assignee)

Comment 23

9 years ago
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
Last Resolved: 9 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.