The default bug view has changed. See this FAQ.

NSC_Initialize leaves a slot list even if it fails

RESOLVED FIXED in 3.4.1

Status

NSS
Libraries
P1
major
RESOLVED FIXED
15 years ago
15 years ago

People

(Reporter: Julien Pierre, Assigned: Julien Pierre)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

15 years ago
Consider the following test program :

#include "nss.h"

int  main(int argc, char** argv)
{
    SECStatus rv = NSS_Initialize("1", "2", "3", "secmod.db", 
                        NSS_INIT_READONLY);
    if (SECSuccess != rv)
    {
        rv = NSS_NoDB_Init(".");
    }
    NSS_Shutdown();
}

In this case, the program attempts to initialize NSS with NSS_Initialize(). If 
it doesn't find its cert/key/secmod databases, it will revert to using 
NSS_NoDB_Init().

The problem occurs during shutdown. There is an assertion in NSC_Finalize in 
softoken. Debugging shows that there are 4 slots in the slot list instead of 2 
as there should be. Further investigation shows that


- NSC_Initialize got called twice during NSS_Initialize. SECMOD_LoadPKCS11Module 
does 2 attempts to load a module, one with thread safety and one without.

  213       if (PK11_GETTAB(mod)->C_Initialize(&secmodLockFunctions) != CKR_OK) 
{
  214           mod->isThreadSafe = PR_FALSE;
  215           if (PK11_GETTAB(mod)->C_Initialize(NULL) != CKR_OK) goto fail;
  216       }

Both times, the crypto slot was added to the slot list, and the internal db slot 
failed to be added

- NSC_Initialize got called a third time from NSS_NoDB_Init . At that point, 
both slots were added properly (the db slot is an empty slot), filling slots 3 
and 4.

The fix is to destroy the entire softoken slot list if any slot fails to be 
added within NSC_Initialize.
(Assignee)

Comment 1

15 years ago
Reassigning to self
Assignee: wtc → jpierre
Priority: -- → P1
Target Milestone: --- → 3.4.1
(Assignee)

Comment 2

15 years ago
Created attachment 79736 [details] [diff] [review]
Fix to clean up slot list on NSC_Initialize failure
(Assignee)

Comment 3

15 years ago
Bob,
I looked at the other possible error cases related to that list. I didn't see 
anything else that could cause the shutdown code to go bad.

I did find a potential memory leak in an error case though, due to a realloc().
This is a diff of my fix for it. I'd like to check it in at the same time as the 
patch I attached, which hasn't changed.

*** 2076,2085 ****
        }
      }
      if (nscSlotCount >= nscSlotListSize) {
        nscSlotListSize += NSC_SLOT_LIST_BLOCK_SIZE;
!       nscSlotList = (CK_SLOT_ID *) PORT_Realloc(nscSlotList,
                                        nscSlotListSize*sizeof(CK_SLOT_ID));
        if (nscSlotList == NULL) {
            return NULL;
        }
      }
--- 2076,2089 ----
        }
      }
      if (nscSlotCount >= nscSlotListSize) {
+         CK_SLOT_ID* oldNscSlotList = nscSlotList;
+         CK_ULONG oldNscSlotListSize = nscSlotListSize;
        nscSlotListSize += NSC_SLOT_LIST_BLOCK_SIZE;
!       nscSlotList = (CK_SLOT_ID *) PORT_Realloc(oldNscSlotList,
                                        nscSlotListSize*sizeof(CK_SLOT_ID));
        if (nscSlotList == NULL) {
+             nscSlotList = oldNscSlotList;
+             nscSlotListSize = oldNscSlotListSize;
            return NULL;
        }
      }
(Assignee)

Comment 4

15 years ago
I checked in the fix to both NSS_3_4_BRANCH and the tip.
Status: NEW → RESOLVED
Last Resolved: 15 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.