6720255 soft_digest_init() memory leaks caused by digest operations on a system with HW crypto provider

6745244 inconsistent locking of soft_session_t mutex in soft_{get,set}_operationstate()
6748371 memcpy() wastes cycles in soft_get_operationstate()/soft_set_operationstate()

4 files changed, 158 insertions, 138 deletions

diff --git a/usr/src/lib/pkcs11/pkcs11_softtoken/common/softDigest.c b/usr/src/lib/pkcs11/pkcs11_softtoken/common/softDigest.c
index a18e6f25d6..56d43ba5c9 100644
--- a/usr/src/lib/pkcs11/pkcs11_softtoken/common/softDigest.c
+++ b/usr/src/lib/pkcs11/pkcs11_softtoken/common/softDigest.c
@@ -2,9 +2,8 @@
  * CDDL HEADER START
  *
  * The contents of this file are subject to the terms of the
- * Common Development and Distribution License, Version 1.0 only
- * (the "License").  You may not use this file except in compliance
- * with the License.
+ * Common Development and Distribution License (the "License").
+ * You may not use this file except in compliance with the License.
  *
  * You can obtain a copy of the license at usr/src/OPENSOLARIS.LICENSE
  * or http://www.opensolaris.org/os/licensing.
@@ -20,12 +19,10 @@
  * CDDL HEADER END
  */
 /*
- * Copyright 2005 Sun Microsystems, Inc.  All rights reserved.
+ * Copyright 2008 Sun Microsystems, Inc.  All rights reserved.
  * Use is subject to license terms.
  */
 
-#pragma ident	"%Z%%M%	%I%	%E% SMI"
-
 #include <pthread.h>
 #include <security/cryptoki.h>
 #include "softGlobal.h"
@@ -66,9 +63,7 @@ C_DigestInit(CK_SESSION_HANDLE hSession, CK_MECHANISM_PTR pMechanism)
 		 * Free the memory to avoid memory leak.
 		 * digest.context is only a flat structure.
 		 */
-
-		if (session_p->digest.context)
-			free(session_p->digest.context);
+		soft_digest_cleanup(session_p, lock_held);
 	}
 
 	/*
@@ -190,7 +185,8 @@ clean_exit:
 	 * digest operation.
 	 */
 	(void) pthread_mutex_lock(&session_p->session_mutex);
-	session_p->digest.flags = 0;
+
+	soft_digest_cleanup(session_p, lock_held);
 
 	/*
 	 * Decrement the session reference count.
@@ -268,7 +264,8 @@ clean_exit:
 	 * operation by resetting the active and update flags.
 	 */
 	(void) pthread_mutex_lock(&session_p->session_mutex);
-	session_p->digest.flags = 0;
+
+	soft_digest_cleanup(session_p, lock_held);
 
 	/*
 	 * Decrement the session reference count.
@@ -352,7 +349,8 @@ clean_exit:
 	 * operation by resetting the active and update flags.
 	 */
 	(void) pthread_mutex_lock(&session_p->session_mutex);
-	session_p->digest.flags = 0;
+
+	soft_digest_cleanup(session_p, lock_held);
 
 	/*
 	 * Decrement the session reference count.
@@ -429,7 +427,8 @@ C_DigestFinal(CK_SESSION_HANDLE hSession, CK_BYTE_PTR pDigest,
 clean_exit:
 	/* Terminates the active digest operation */
 	(void) pthread_mutex_lock(&session_p->session_mutex);
-	session_p->digest.flags = 0;
+
+	soft_digest_cleanup(session_p, lock_held);
 
 	/*
 	 * Decrement the session reference count.
diff --git a/usr/src/lib/pkcs11/pkcs11_softtoken/common/softDigestUtil.c b/usr/src/lib/pkcs11/pkcs11_softtoken/common/softDigestUtil.c
index 2af69c98a5..f1f685ed40 100644
--- a/usr/src/lib/pkcs11/pkcs11_softtoken/common/softDigestUtil.c
+++ b/usr/src/lib/pkcs11/pkcs11_softtoken/common/softDigestUtil.c
@@ -2,9 +2,8 @@
  * CDDL HEADER START
  *
  * The contents of this file are subject to the terms of the
- * Common Development and Distribution License, Version 1.0 only
- * (the "License").  You may not use this file except in compliance
- * with the License.
+ * Common Development and Distribution License (the "License").
+ * You may not use this file except in compliance with the License.
  *
  * You can obtain a copy of the license at usr/src/OPENSOLARIS.LICENSE
  * or http://www.opensolaris.org/os/licensing.
@@ -20,12 +19,10 @@
  * CDDL HEADER END
  */
 /*
- * Copyright 2005 Sun Microsystems, Inc.  All rights reserved.
+ * Copyright 2008 Sun Microsystems, Inc.  All rights reserved.
  * Use is subject to license terms.
  */
 
-#pragma ident	"%Z%%M%	%I%	%E% SMI"
-
 #include <strings.h>
 #include <md5.h>
 #include <pthread.h>
@@ -488,3 +485,28 @@ soft_digest_key(soft_session_t *session_p, soft_object_t *key_p)
 	return (rv);
 
 }
+
+/*
+ * This function releases allocated digest context. The caller
+ * may (lock_held == B_TRUE) or may not (lock_held == B_FALSE)
+ * hold a session mutex.
+ */
+void
+soft_digest_cleanup(soft_session_t *session_p, boolean_t lock_held)
+{
+	boolean_t lock_true = B_TRUE;
+
+	if (!lock_held)
+		(void) pthread_mutex_lock(&session_p->session_mutex);
+
+	if (session_p->digest.context != NULL) {
+		free(session_p->digest.context);
+		session_p->digest.context = NULL;
+	}
+
+	session_p->digest.flags = 0;
+
+	if (!lock_held)
+		SES_REFRELE(session_p, lock_true);
+
+}
diff --git a/usr/src/lib/pkcs11/pkcs11_softtoken/common/softOps.h b/usr/src/lib/pkcs11/pkcs11_softtoken/common/softOps.h
index a9e50440bc..9e51c980b3 100644
--- a/usr/src/lib/pkcs11/pkcs11_softtoken/common/softOps.h
+++ b/usr/src/lib/pkcs11/pkcs11_softtoken/common/softOps.h
@@ -2,9 +2,8 @@
  * CDDL HEADER START
  *
  * The contents of this file are subject to the terms of the
- * Common Development and Distribution License, Version 1.0 only
- * (the "License").  You may not use this file except in compliance
- * with the License.
+ * Common Development and Distribution License (the "License").
+ * You may not use this file except in compliance with the License.
  *
  * You can obtain a copy of the license at usr/src/OPENSOLARIS.LICENSE
  * or http://www.opensolaris.org/os/licensing.
@@ -20,15 +19,13 @@
  * CDDL HEADER END
  */
 /*
- * Copyright 2004 Sun Microsystems, Inc.  All rights reserved.
+ * Copyright 2008 Sun Microsystems, Inc.  All rights reserved.
  * Use is subject to license terms.
  */
 
 #ifndef _SOFTOPS_H
 #define	_SOFTOPS_H
 
-#pragma ident	"%Z%%M%	%I%	%E% SMI"
-
 #ifdef __cplusplus
 extern "C" {
 #endif
@@ -110,6 +107,8 @@ void soft_crypt_cleanup(soft_session_t *, boolean_t, boolean_t);
 
 void soft_sign_verify_cleanup(soft_session_t *, boolean_t, boolean_t);
 
+void soft_digest_cleanup(soft_session_t *, boolean_t);
+
 #ifdef	__cplusplus
 }
 #endif
diff --git a/usr/src/lib/pkcs11/pkcs11_softtoken/common/softSessionUtil.c b/usr/src/lib/pkcs11/pkcs11_softtoken/common/softSessionUtil.c
index 6c65da34e2..4c0859f41e 100644
--- a/usr/src/lib/pkcs11/pkcs11_softtoken/common/softSessionUtil.c
+++ b/usr/src/lib/pkcs11/pkcs11_softtoken/common/softSessionUtil.c
@@ -23,8 +23,6 @@
  * Use is subject to license terms.
  */
 
-#pragma ident	"%Z%%M%	%I%	%E% SMI"
-
 #include <md5.h>
 #include <pthread.h>
 #include <syslog.h>
@@ -44,6 +42,8 @@
 CK_ULONG soft_session_cnt = 0;		/* the number of opened sessions */
 CK_ULONG soft_session_rw_cnt = 0;	/* the number of opened R/W sessions */
 
+#define	DIGEST_MECH_OK(_m_)	((_m_) == CKM_MD5 || (_m_) == CKM_SHA_1)
+
 /*
  * Delete all the sessions. First, obtain the global session
  * list lock. Then start to delete one session at a time.
@@ -448,27 +448,37 @@ soft_get_operationstate(soft_session_t *session_p, CK_BYTE_PTR pOperationState,
     CK_ULONG_PTR pulOperationStateLen)
 {
 
-	internal_op_state_t op_state;
+	internal_op_state_t *p_op_state;
 	CK_ULONG op_data_len = 0;
+	CK_RV rv = CKR_OK;
+
+	if (pulOperationStateLen == NULL)
+		return (CKR_ARGUMENTS_BAD);
+
+	(void) pthread_mutex_lock(&session_p->session_mutex);
 
 	/* Check to see if encrypt operation is active. */
 	if (session_p->encrypt.flags & CRYPTO_OPERATION_ACTIVE) {
-		return (CKR_STATE_UNSAVEABLE);
+		rv = CKR_STATE_UNSAVEABLE;
+		goto unlock_session;
 	}
 
 	/* Check to see if decrypt operation is active. */
 	if (session_p->decrypt.flags & CRYPTO_OPERATION_ACTIVE) {
-		return (CKR_STATE_UNSAVEABLE);
+		rv = CKR_STATE_UNSAVEABLE;
+		goto unlock_session;
 	}
 
 	/* Check to see if sign operation is active. */
 	if (session_p->sign.flags & CRYPTO_OPERATION_ACTIVE) {
-		return (CKR_STATE_UNSAVEABLE);
+		rv = CKR_STATE_UNSAVEABLE;
+		goto unlock_session;
 	}
 
 	/* Check to see if verify operation is active. */
 	if (session_p->verify.flags & CRYPTO_OPERATION_ACTIVE) {
-		return (CKR_STATE_UNSAVEABLE);
+		rv = CKR_STATE_UNSAVEABLE;
+		goto unlock_session;
 	}
 
 	/* Check to see if digest operation is active. */
@@ -484,26 +494,27 @@ soft_get_operationstate(soft_session_t *session_p, CK_BYTE_PTR pOperationState,
 			op_data_len += sizeof (SHA1_CTX);
 			break;
 		default:
-			return (CKR_STATE_UNSAVEABLE);
+			rv = CKR_STATE_UNSAVEABLE;
+			goto unlock_session;
 		}
 
 		if (pOperationState == NULL_PTR) {
 			*pulOperationStateLen = op_data_len;
-			return (CKR_OK);
+			goto unlock_session;
 		} else {
 			if (*pulOperationStateLen < op_data_len) {
 				*pulOperationStateLen = op_data_len;
-				return (CKR_BUFFER_TOO_SMALL);
+				rv = CKR_BUFFER_TOO_SMALL;
+				goto unlock_session;
 			}
 		}
 
-		op_state.op_len = op_data_len;
-		op_state.op_active = DIGEST_OP;
-		op_state.op_session_state = session_p->state;
-
 		/* Save internal_op_state_t */
-		(void) memcpy(pOperationState, (CK_BYTE_PTR)&op_state,
-		    sizeof (internal_op_state_t));
+		/* LINTED E_BAD_PTR_CAST_ALIGN */
+		p_op_state = (internal_op_state_t *)pOperationState;
+		p_op_state->op_len = op_data_len;
+		p_op_state->op_active = DIGEST_OP;
+		p_op_state->op_session_state = session_p->state;
 
 		/* Save crypto_active_op_t */
 		(void) memcpy((CK_BYTE *)pOperationState +
@@ -531,15 +542,36 @@ soft_get_operationstate(soft_session_t *session_p, CK_BYTE_PTR pOperationState,
 			break;
 
 		default:
-			return (CKR_STATE_UNSAVEABLE);
+			rv = CKR_STATE_UNSAVEABLE;
 		}
 	}
 
 	*pulOperationStateLen = op_data_len;
-	return (CKR_OK);
+
+unlock_session:
+	(void) pthread_mutex_unlock(&session_p->session_mutex);
+
+	return (rv);
 
 }
 
+static CK_BYTE_PTR alloc_digest(CK_ULONG mech)
+{
+	CK_BYTE_PTR	ret_val;
+
+	switch (mech) {
+		case CKM_MD5:
+			ret_val = (CK_BYTE_PTR) malloc(sizeof (MD5_CTX));
+			break;
+		case CKM_SHA_1:
+			ret_val = (CK_BYTE_PTR) malloc(sizeof (SHA1_CTX));
+			break;
+		default: ret_val = NULL;
+	}
+
+	return (ret_val);
+}
+
 /*
  * The format to be restored from the pOperationState will be:
  * 1. internal_op_state_t
@@ -552,140 +584,108 @@ soft_set_operationstate(soft_session_t *session_p, CK_BYTE_PTR pOperationState,
     CK_OBJECT_HANDLE hAuthenticationKey)
 {
 
-	CK_RV		rv;
-	internal_op_state_t op_state;
-	crypto_active_op_t crypto_tmp;
+	CK_RV		rv = CKR_OK;
+	internal_op_state_t *p_op_state;
+	crypto_active_op_t *p_active_op;
 	CK_ULONG offset = 0;
+	CK_ULONG mech;
+	void *free_it = NULL;
 
-	/* Restore internal_op_state_t */
-	(void) memcpy((CK_BYTE_PTR)&op_state, pOperationState,
-	    sizeof (internal_op_state_t));
+	/* LINTED E_BAD_PTR_CAST_ALIGN */
+	p_op_state = (internal_op_state_t *)pOperationState;
 
-	if (session_p->state != op_state.op_session_state) {
+	if (p_op_state->op_len != ulOperationStateLen) {
 		/*
-		 * The supplied session state does not match with
-		 * the saved session state.
+		 * The supplied data length does not match with
+		 * the saved data length.
 		 */
 		return (CKR_SAVED_STATE_INVALID);
 	}
 
-	if (op_state.op_len != ulOperationStateLen) {
-		/*
-		 * The supplied data length does not match with
-		 * the saved data length.
-		 */
+	if (p_op_state->op_active != DIGEST_OP)
 		return (CKR_SAVED_STATE_INVALID);
+
+	if ((hAuthenticationKey != 0) || (hEncryptionKey != 0)) {
+		return (CKR_KEY_NOT_NEEDED);
 	}
 
 	offset = sizeof (internal_op_state_t);
+	/* LINTED E_BAD_PTR_CAST_ALIGN */
+	p_active_op = (crypto_active_op_t *)(pOperationState + offset);
+	offset += sizeof (crypto_active_op_t);
+	mech = p_active_op->mech.mechanism;
 
-	(void) memcpy((CK_BYTE *)&crypto_tmp,
-	    (CK_BYTE *)pOperationState + offset,
-	    sizeof (crypto_active_op_t));
+	if (!DIGEST_MECH_OK(mech)) {
+		return (CKR_SAVED_STATE_INVALID);
+	}
 
-	switch (op_state.op_active) {
-	case DIGEST_OP:
-		if ((hAuthenticationKey != 0) || (hEncryptionKey != 0)) {
-			return (CKR_KEY_NOT_NEEDED);
-		}
+	/*
+	 * We may reuse digest.context in case the digest mechanisms (the one,
+	 * which belongs to session and the operation, which we are restoring)
+	 * are the same. If digest mechanisms are different, we have to release
+	 * the digest context, which belongs to session and allocate a new one.
+	 */
+	(void) pthread_mutex_lock(&session_p->session_mutex);
 
+	if (session_p->state != p_op_state->op_session_state) {
 		/*
-		 * If the destination session has the same mechanism
-		 * as the source, we can reuse the memory allocated for
-		 * the crypto context. Otherwise, we free the crypto
-		 * context of the destination session now.
+		 * The supplied session state does not match with
+		 * the saved session state.
 		 */
-		if (session_p->digest.context) {
-			if (session_p->digest.mech.mechanism !=
-			    crypto_tmp.mech.mechanism) {
-				(void) pthread_mutex_lock(&session_p->
-				    session_mutex);
-				free(session_p->digest.context);
-				session_p->digest.context = NULL;
-				(void) pthread_mutex_unlock(&session_p->
-				    session_mutex);
-			}
-		}
-		break;
-
-	default:
-		return (CKR_SAVED_STATE_INVALID);
+		rv = CKR_SAVED_STATE_INVALID;
+		goto unlock_session;
 	}
 
-	/* Restore crypto_active_op_t */
-	(void) pthread_mutex_lock(&session_p->session_mutex);
-	session_p->digest.mech.mechanism = crypto_tmp.mech.mechanism;
-	session_p->digest.flags = crypto_tmp.flags;
-	(void) pthread_mutex_unlock(&session_p->session_mutex);
+	if (session_p->digest.context &&
+	    (session_p->digest.mech.mechanism != mech)) {
+		free_it = session_p->digest.context;
+		session_p->digest.context = NULL;
+	}
 
-	offset += sizeof (crypto_active_op_t);
+	if (session_p->digest.context == NULL) {
+		session_p->digest.context = alloc_digest(mech);
+
+		if (session_p->digest.context == NULL) {
+			/*
+			 * put back original context into session in case
+			 * allocation of new context has failed.
+			 */
+			session_p->digest.context = free_it;
+			free_it = NULL;
+			rv = CKR_HOST_MEMORY;
+			goto unlock_session;
+		}
+	}
 
-	/*
-	 * Make sure the supplied crypto operation state is valid
-	 */
-	switch (op_state.op_active) {
-	case DIGEST_OP:
+	/* Restore crypto_active_op_t */
+	session_p->digest.mech.mechanism = mech;
+	session_p->digest.flags = p_active_op->flags;
 
-		switch (session_p->digest.mech.mechanism) {
+	switch (mech) {
 		case CKM_MD5:
-			(void) pthread_mutex_lock(&session_p->session_mutex);
-			if (session_p->digest.context == NULL) {
-				session_p->digest.context =
-				    malloc(sizeof (MD5_CTX));
-
-				if (session_p->digest.context == NULL) {
-					(void) pthread_mutex_unlock(
-					    &session_p->session_mutex);
-					return (CKR_HOST_MEMORY);
-				}
-			}
-
 			/* Restore MD5_CTX from the saved digest operation */
 			(void) memcpy((CK_BYTE *)session_p->digest.context,
 			    (CK_BYTE *)pOperationState + offset,
 			    sizeof (MD5_CTX));
-
-			(void) pthread_mutex_unlock(&session_p->session_mutex);
-
-			rv = CKR_OK;
 			break;
-
 		case CKM_SHA_1:
-			(void) pthread_mutex_lock(&session_p->session_mutex);
-			if (session_p->digest.context == NULL) {
-				session_p->digest.context =
-				    malloc(sizeof (SHA1_CTX));
-
-				if (session_p->digest.context == NULL) {
-					(void) pthread_mutex_unlock(
-					    &session_p->session_mutex);
-					return (CKR_HOST_MEMORY);
-				}
-			}
-
 			/* Restore SHA1_CTX from the saved digest operation */
 			(void) memcpy((CK_BYTE *)session_p->digest.context,
 			    (CK_BYTE *)pOperationState + offset,
 			    sizeof (SHA1_CTX));
-
-			(void) pthread_mutex_unlock(&session_p->session_mutex);
-
-			rv = CKR_OK;
 			break;
-
 		default:
+			/* never reached */
 			rv = CKR_SAVED_STATE_INVALID;
-			break;
-		}
-		break;
-
-	default:
-		rv = CKR_SAVED_STATE_INVALID;
-		break;
 	}
 
-	return (rv);
+unlock_session:
+	(void) pthread_mutex_unlock(&session_p->session_mutex);
 
+	if (free_it != NULL)
+		free(free_it);
+
+	return (rv);
 }