Last Comment Bug 353739 - Klocwork Null ptr dereferences in instance.c
: Klocwork Null ptr dereferences in instance.c
Status: RESOLVED FIXED
: klocwork
Product: NSS
Classification: Components
Component: Libraries (show other bugs)
: trunk
: All All
: P2 normal (vote)
: 3.12
Assigned To: Alexei Volkov
:
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2006-09-21 21:20 PDT by Nelson Bolyard (seldom reads bugmail)
Modified: 2006-10-09 15:17 PDT (History)
0 users
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
check pointer value returned by nssCKFWHash_Lookup (2.52 KB, patch)
2006-10-04 13:12 PDT, Alexei Volkov
nelson: review-
Details | Diff | Splinter Review
fix as suggested (2.77 KB, patch)
2006-10-04 13:47 PDT, Alexei Volkov
nelson: review+
Details | Diff | Splinter Review

Description Nelson Bolyard (seldom reads bugmail) 2006-09-21 21:20:21 PDT
Klocwork ID 92337
File     nss/lib/ckfw/instance.c
Function nssCKFWInstance_DestroySessionHandle

Pointer 'fwSession' returned from call to function 'nssCKFWHash_Lookup' at 
line 671 may be NULL and will be dereferenced by passing argument 1 to 
function 'nssCKFWSession_SetHandle' at line 675.

Klocwork ID 92338
File     nss/lib/ckfw/instance.c
Function nssCKFWInstance_ReassignObjectHandle

Pointer 'oldObject' returned from call to function 'nssCKFWHash_Lookup' at 
line 816 may be NULL and will be dereferenced by passing argument 1 to 
function 'nssCKFWObject_SetHandle' at line 819.

Klocwork ID 92339
File     nss/lib/ckfw/instance.c
Function nssCKFWInstance_DestroyObjectHandle

Pointer 'fwObject' returned from call to function 'nssCKFWHash_Lookup' at 
line 857 may be NULL and will be dereferenced by passing argument 1 to 
function 'nssCKFWObject_SetHandle' at line 861.
Comment 1 Alexei Volkov 2006-10-04 13:12:27 PDT
Created attachment 241206 [details] [diff] [review]
check pointer value returned by nssCKFWHash_Lookup
Comment 2 Nelson Bolyard (seldom reads bugmail) 2006-10-04 13:39:16 PDT
Comment on attachment 241206 [details] [diff] [review]
check pointer value returned by nssCKFWHash_Lookup

This change appears in two places:

>   fwSession = (NSSCKFWSession *)nssCKFWHash_Lookup(
>                 fwInstance->sessionHandleHash, (const void *)hSession);
>-
>+  if (!fwSession) {
>+      (void)nssCKFWMutex_Unlock(fwInstance->mutex);
>+      return;
>+  }
>+      
>   nssCKFWHash_Remove(fwInstance->sessionHandleHash, (const void *)hSession);
>   nssCKFWSession_SetHandle(fwSession, (CK_SESSION_HANDLE)0);
> 
>   (void)nssCKFWMutex_Unlock(fwInstance->mutex);
> 
>   return;

In both places, I'd rather see

>   fwSession = (NSSCKFWSession *)nssCKFWHash_Lookup(
>                 fwInstance->sessionHandleHash, (const void *)hSession);
>-
>-  nssCKFWHash_Remove(fwInstance->sessionHandleHash, (const void *)hSession);
>-  nssCKFWSession_SetHandle(fwSession, (CK_SESSION_HANDLE)0);
>+  if (fwSession) {
>+      nssCKFWHash_Remove(fwInstance->sessionHandleHash, (const void *)hSession);
>+      nssCKFWSession_SetHandle(fwSession, (CK_SESSION_HANDLE)0);
>+  }
> 
>   (void)nssCKFWMutex_Unlock(fwInstance->mutex);
> 
>   return;
Comment 3 Alexei Volkov 2006-10-04 13:47:37 PDT
Created attachment 241214 [details] [diff] [review]
fix as suggested
Comment 4 Nelson Bolyard (seldom reads bugmail) 2006-10-04 13:54:09 PDT
Comment on attachment 241214 [details] [diff] [review]
fix as suggested

r=nelson for trunk
Comment 5 Alexei Volkov 2006-10-09 15:17:14 PDT
/cvsroot/mozilla/security/nss/lib/ckfw/instance.c,v  <--  instance.c
new revision: 1.11; previous revision: 1.10

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