Last Comment Bug 812399 - The fix for bug 641052 was partially lost when bug 753116 was fixed
: The fix for bug 641052 was partially lost when bug 753116 was fixed
: regression
Product: NSS
Classification: Components
Component: Libraries (show other bugs)
: 3.14
: All All
P1 normal (vote)
: 3.14.1
Assigned To: Wan-Teh Chang
: 811928 (view as bug list)
Depends on:
  Show dependency treegraph
Reported: 2012-11-15 18:24 PST by Wan-Teh Chang
Modified: 2012-11-19 12:46 PST (History)
1 user (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---

Patch (4.68 KB, patch)
2012-11-15 18:24 PST, Wan-Teh Chang
rrelyea: review+
Details | Diff | Splinter Review

Description User image Wan-Teh Chang 2012-11-15 18:24:16 PST
Created attachment 682298 [details] [diff] [review]

When Bob fixed bug 753116 (softoken needs to split out common components to util),
he moved lib/softoken/sftkmod.c to lib/util/utilmod.c, and lib/softoken/sftkpars.c
to lib/util/utilpars.c. During that move, my fix for bug 641052 (NSS_NoDB_Init
should not try to open /pkcs11.txt and /secmod.db) was partially lost.

(I don't know how this happened. Perhaps Bob moved an old version of
lib/softoken/sftkmod.c and lib/softoken/sftkpars.c and recreated some of my changes.
Perhaps Bob deliberately removed some of my changes.)

I adapted my patch to utilmod.c and utilpars.c. Some of the code I modified in
utilmod.c has been removed, so I am not sure if I adapted my patch correctly.

Notes on the patch:

1. The most important change is the change in nssutil_ReadSecmodDB to allow
the 'dbname' argument to be NULL. Please check that change carefully.

2. I changed the comment "old one doesn't exist" to "old one exists" because
the original code was:

        /* old one doesn't exist */
        status = PR_Access(olddbname, PR_ACCESS_EXISTS);
        if (status != PR_SUCCESS) {
            goto bail;

But the new code is:

	/* old one doesn't exist */
 	status = PR_Access(olddbname, PR_ACCESS_EXISTS);
 	if (status == PR_SUCCESS) {
 	    return NULL;

So the old comment doesn't match the new code.
Comment 1 User image Wan-Teh Chang 2012-11-15 19:27:12 PST
I confirmed the regression. With the current NSS trunk, the strace output
of a program calling NSS_NoDB_Init() on Linux contains:

open("/pkcs11.txt", O_RDONLY)           = -1 ENOENT (No such file or directory)
access("/secmod.db", F_OK)              = -1 ENOENT (No such file or directory)

Those lines are gone if I apply the patch.

NOTE: I am not convinced this bug could cause NSS_NoDB_Init to fail though.
I created /pkcs11.txt and /secmod.db on my Linux computer with garbage contents,
but NSS_NoDB_Init still returns SECSuccess. The strace output shows NSS opens
/pkcs11.txt, reads its contents, and closes the it.
Comment 2 User image Robert Relyea 2012-11-16 16:03:51 PST
Comment on attachment 682298 [details] [diff] [review]

r+ rrelyea
Comment 3 User image Wan-Teh Chang 2012-11-16 17:38:39 PST
I figured out a way for this bug to cause NSS_NoDB_Init to fail.

1. /pkcs11.txt does not exist.

2. /secmod.db exists.

3. does not exist.

I missed the third condition when I tried to cause NSS_NoDB_Init to fail
last night. Now I think this bug is very likely the cause of bug 811928.

Patch checked in on the NSS trunk (NSS 3.14.1).

Checking in utilmod.c;
/cvsroot/mozilla/security/nss/lib/util/utilmod.c,v  <--  utilmod.c
new revision: 1.8; previous revision: 1.7
Checking in utilpars.c;
/cvsroot/mozilla/security/nss/lib/util/utilpars.c,v  <--  utilpars.c
new revision: 1.6; previous revision: 1.5
Comment 4 User image Wan-Teh Chang 2012-11-19 12:46:11 PST
*** Bug 811928 has been marked as a duplicate of this bug. ***

Note You need to log in before you can comment on or make changes to this bug.