Closed Bug 239827 Opened 20 years ago Closed 20 years ago

NSS has race condition in unwrap key code.

Categories

(NSS :: Libraries, defect)

Product:
NSS
Component:
Libraries
Version:
3.7.8
Platform:
x86
Windows 2000
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(Not tracked)

Status:
VERIFIED FIXED

People

(Reporter: rrelyea, Assigned: rrelyea)

Details

(Whiteboard: [3.7.11])

Attachments

(2 files)

Add the appropriate locks. - Version NSS 3.7
1.04 KB, patch
wtc
: review+
nelson
: superreview+
Details | Diff | Splinter Review
Add locks, patch for nss 3.9
1.04 KB, patch
wtc
: review+
nelson
: superreview+
Details | Diff | Splinter Review 
Bug reported byt thomasknscp@aol.com:

This bug is triggered when the application is under stress by multiple threads. 
I think the stack trace that we captured supports the fact that the problem is 
originated from PK11_UnwrapPrivKey where we forgot to enter a monitor that 
protects the C_UnwrapKey function.
The problem is in multiple creations of non-permanant keys. For permanent keys, 
the appropriate locks or threadsafe session handle is acquired.

bob

    
    

    
  
This patch has been verified on NSS 3.7 by thomask.

    
    

    
  
Comment on attachment 145559 [details] [diff] [review]
Add the appropriate locks. - Version NSS 3.7

r=wtc.

        Attachment #145559 -
        Flags: superreview?(MisterSSL)

        Attachment #145559 -
        Flags: review+

    
    

    
  
This fix should go into 3.9.1.  Fortunately the code
path is not executed by the NISCC tests so the fix
does not invalidate the NISCC testing Bishakha has
done.
Status: NEW → ASSIGNED
Target Milestone: --- → 3.9.1

    
  

        Attachment #145560 -
        Flags: superreview?(MisterSSL)

        Attachment #145560 -
        Flags: review+

    
    

    
  
Comment on attachment 145559 [details] [diff] [review]
Add the appropriate locks. - Version NSS 3.7

This patch appears to be correct.  However, it seems odd that all such
operations will now be single threaded for session objects, but not for token
objects.  
But this operation is thought to be very rare, so maybe this detail doesn't
matter.  r=MisterSSL

        Attachment #145559 -
        Flags: superreview?(MisterSSL) → superreview+

    
    

    
  
Comment on attachment 145560 [details] [diff] [review]
Add locks, patch for nss 3.9

Both of these patches are effectively identical.  So my comments apply to both.

        Attachment #145560 -
        Flags: superreview?(MisterSSL) → superreview+

    
  
Whiteboard: [3.7.11]

    
    

    
  
Bob,  This bug has several patches all r+ and sr+
Are these patchces checked in now?  If not, why not?
Can this bug be closed?

    
  
Status: ASSIGNED → RESOLVED
Closed: 20 years ago
Resolution: --- → FIXED

    
    

    
  
Marked the bug fixed and verified.  The fix is
in the tip (3.10), NSS_3_9_BRANCH (3.9.1), and
NSS_3_7_BRANCH (3.7.11).  The fix was verified
by the original bug reporter (Thomas Kwan).
Status: RESOLVED → VERIFIED

    
    

    
  
For future reference:

3.7 patch was pk11skey.c rev 1.57.2.9
3.9 patch was pk11skey.c rev 1.84.2.1
Trunk patch was pk11skey.c rev 1.86

bob

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






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: