On 03/08/2016 04:17 PM, Jakub Jelen wrote:
Hello there,
Coverity scan on latest version reveled some possible problems in the
code, namely:
* NULL_RETURNS - variable is not checked against NULL
* MISSING_BREAK - this is not a bug, but would be nice to explicitly
mention that the fall-through is intentional
* OVERRUN - there is buffer overflow in trace code, which assigns
unknown errors ERR_MAX, which is over ock_err_msg boundaries
* DEADCODE - there is bad identification of readonly icsf tokens
And few more commits that were modified since yesterday. Namely:
* Explicit type cast to silent compiler and Coverity
* Dereferencing (possibly) NULL arguments in Dynamic traces
* (possibly) uninitialized variables
* Few more issues with NULL dereference
If some reasoning is not clear, please let me know.
See attached patches, which should cleanly apply on the master from
sourceforge.
Regards,
--
Jakub Jelen
Associate Software Engineer
Security Technologies
Red Hat
>From dbc58326be57d173c72cb6486246969879d0b5b7 Mon Sep 17 00:00:00 2001
From: Jakub Jelen <jje...@redhat.com>
Date: Tue, 8 Mar 2016 15:39:32 +0100
Subject: [PATCH 1/8] Coverity: NULL_RETURNS sanity check
---
usr/lib/pkcs11/icsf_stdll/icsf_specific.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/usr/lib/pkcs11/icsf_stdll/icsf_specific.c b/usr/lib/pkcs11/icsf_stdll/icsf_specific.c
index 2c12e91..e5c2750 100644
--- a/usr/lib/pkcs11/icsf_stdll/icsf_specific.c
+++ b/usr/lib/pkcs11/icsf_stdll/icsf_specific.c
@@ -934,6 +934,12 @@ CK_RV icsftok_open_session(SESSION *sess)
LDAP *ld;
struct session_state *session_state;
+ /* Sanity */
+ if (sess == NULL) {
+ TRACE_ERROR("%s\n", ock_err(ERR_ARGUMENTS_BAD));
+ return CKR_FUNCTION_FAILED;
+ }
+
/* Add session to list */
session_state = malloc(sizeof(struct session_state));
if (!session_state) {
@@ -1065,7 +1071,7 @@ CK_RV icsftok_close_session(SESSION *session)
struct session_state *session_state;
/* Get the related session_state */
- if (!(session_state = get_session_state(session->handle))) {
+ if (session == NULL || !(session_state = get_session_state(session->handle))) {
TRACE_ERROR("%s\n", ock_err(ERR_SESSION_HANDLE_INVALID));
return CKR_SESSION_HANDLE_INVALID;
}
--
2.5.0
>From a50fc9db83550a47dc6a88fdff0a467c6d3632b2 Mon Sep 17 00:00:00 2001
From: Jakub Jelen <jje...@redhat.com>
Date: Tue, 8 Mar 2016 15:51:21 +0100
Subject: [PATCH 2/8] Coverity: MISSING_BREAK: explicitly note the fallback is
intentional
---
usr/lib/pkcs11/soft_stdll/soft_specific.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/usr/lib/pkcs11/soft_stdll/soft_specific.c b/usr/lib/pkcs11/soft_stdll/soft_specific.c
index 0b9b891..ce53439 100644
--- a/usr/lib/pkcs11/soft_stdll/soft_specific.c
+++ b/usr/lib/pkcs11/soft_stdll/soft_specific.c
@@ -2470,21 +2470,25 @@ static CK_RV softtok_hmac(SIGN_VERIFY_CONTEXT *ctx, CK_BYTE *in_data,
switch(ctx->mech.mechanism) {
case CKM_SHA_1_HMAC_GENERAL:
general = TRUE;
+ /* fallthrough */
case CKM_SHA_1_HMAC:
mac_len = SHA1_HASH_SIZE;
break;
case CKM_SHA256_HMAC_GENERAL:
general = TRUE;
+ /* fallthrough */
case CKM_SHA256_HMAC:
mac_len = SHA2_HASH_SIZE;
break;
case CKM_SHA384_HMAC_GENERAL:
general = TRUE;
+ /* fallthrough */
case CKM_SHA384_HMAC:
mac_len = SHA3_HASH_SIZE;
break;
case CKM_SHA512_HMAC_GENERAL:
general = TRUE;
+ /* fallthrough */
case CKM_SHA512_HMAC:
mac_len = SHA5_HASH_SIZE;
break;
@@ -2610,11 +2614,13 @@ static CK_RV softtok_hmac_final(SIGN_VERIFY_CONTEXT *ctx, CK_BYTE *signature,
switch(ctx->mech.mechanism) {
case CKM_SHA_1_HMAC_GENERAL:
general = TRUE;
+ /* fallthrough */
case CKM_SHA_1_HMAC:
mac_len = SHA1_HASH_SIZE;
break;
case CKM_SHA256_HMAC_GENERAL:
general = TRUE;
+ /* fallthrough */
case CKM_SHA256_HMAC:
mac_len = SHA2_HASH_SIZE;
break;
@@ -2625,6 +2631,7 @@ static CK_RV softtok_hmac_final(SIGN_VERIFY_CONTEXT *ctx, CK_BYTE *signature,
break;
case CKM_SHA512_HMAC_GENERAL:
general = TRUE;
+ /* fallthrough */
case CKM_SHA512_HMAC:
mac_len = SHA5_HASH_SIZE;
break;
--
2.5.0
>From ab50caba0d1c98b00aed7420aa0bc0e298c16c8b Mon Sep 17 00:00:00 2001
From: Jakub Jelen <jje...@redhat.com>
Date: Tue, 8 Mar 2016 15:59:24 +0100
Subject: [PATCH 3/8] Coverity: OVERRUN: ERR_MAX fallback for unknown errors
---
usr/lib/pkcs11/common/trace.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/usr/lib/pkcs11/common/trace.c b/usr/lib/pkcs11/common/trace.c
index 4ba2c2d..da294f8 100644
--- a/usr/lib/pkcs11/common/trace.c
+++ b/usr/lib/pkcs11/common/trace.c
@@ -391,6 +391,7 @@ static const char *ock_err_msg[] = {
"API already Initialized", /*ERR_CRYPTOKI_ALREADY_INITIALIZED*/
"Mutex Invalid", /*ERR_MUTEX_BAD*/
"Mutex was not locked", /*ERR_MUTEX_NOT_LOCKED*/
+"Unknown error", /*ERR_MAX*/
};
void set_trace(struct trace_handle_t t_handle)
--
2.5.0
>From 33dcee8df8fcab16716e48abab6d7d11242eb6a4 Mon Sep 17 00:00:00 2001
From: Jakub Jelen <jje...@redhat.com>
Date: Tue, 8 Mar 2016 16:06:03 +0100
Subject: [PATCH 4/8] Coverity: Readonly token identification
---
usr/sbin/pkcsicsf/pkcsicsf.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/usr/sbin/pkcsicsf/pkcsicsf.c b/usr/sbin/pkcsicsf/pkcsicsf.c
index 81662fd..0a3100f 100644
--- a/usr/sbin/pkcsicsf/pkcsicsf.c
+++ b/usr/sbin/pkcsicsf/pkcsicsf.c
@@ -334,7 +334,7 @@ list_tokens(void)
num_seen, tokens[i].name,
tokens[i].manufacturer,
tokens[i].model, tokens[i].serial,
- tokens[i].flags ? "yes" : "no");
+ ICSF_IS_TOKEN_READ_ONLY(tokens[i].flags) ? "yes" : "no");
num_seen++;
}
--
2.5.0
>From 6180c189b2dca598d7721dc8d7c07070becc51e9 Mon Sep 17 00:00:00 2001
From: Jakub Jelen <jje...@redhat.com>
Date: Tue, 8 Mar 2016 16:58:40 +0100
Subject: [PATCH 5/8] Coverity: Explicit type cast to silent compiler warnings
---
usr/lib/pkcs11/common/new_host.c | 4 ++--
usr/lib/pkcs11/common/obj_mgr.c | 16 ++++++++--------
usr/lib/pkcs11/icsf_stdll/new_host.c | 4 ++--
3 files changed, 12 insertions(+), 12 deletions(-)
diff --git a/usr/lib/pkcs11/common/new_host.c b/usr/lib/pkcs11/common/new_host.c
index 1bc0403..ded2a29 100755
--- a/usr/lib/pkcs11/common/new_host.c
+++ b/usr/lib/pkcs11/common/new_host.c
@@ -358,8 +358,8 @@ void Fork_Initializer(void)
* When implemented... Although logout_all should clear this up.
*/
- bt_destroy(&priv_token_obj_btree, object_free);
- bt_destroy(&publ_token_obj_btree, object_free);
+ bt_destroy(&priv_token_obj_btree, (void (*)(void *)) object_free);
+ bt_destroy(&publ_token_obj_btree, (void (*)(void *)) object_free);
/* Need to do something to prevent the shared memory from
* having the objects loaded again.... The most likely place
diff --git a/usr/lib/pkcs11/common/obj_mgr.c b/usr/lib/pkcs11/common/obj_mgr.c
index 80f5998..8d288fd 100755
--- a/usr/lib/pkcs11/common/obj_mgr.c
+++ b/usr/lib/pkcs11/common/obj_mgr.c
@@ -1080,7 +1080,7 @@ destroy_object_cb(void *node)
OBJECT *o;
if (map->is_session_obj)
- bt_node_free(&sess_obj_btree, map->obj_handle, object_free);
+ bt_node_free(&sess_obj_btree, map->obj_handle, (void (*)(void *)) object_free);
else {
if (map->is_private)
o = bt_get_node_value(&priv_token_obj_btree, map->obj_handle);
@@ -1105,9 +1105,9 @@ destroy_object_cb(void *node)
XProcUnLock();
if (map->is_private)
- bt_node_free(&priv_token_obj_btree, map->obj_handle, object_free);
+ bt_node_free(&priv_token_obj_btree, map->obj_handle, (void (*)(void *)) object_free);
else
- bt_node_free(&publ_token_obj_btree, map->obj_handle, object_free);
+ bt_node_free(&publ_token_obj_btree, map->obj_handle, (void (*)(void *)) object_free);
}
done:
free(map);
@@ -1187,9 +1187,9 @@ delete_token_obj_cb(void *node, unsigned long map_handle, void *p3)
XProcUnLock();
if (map->is_private)
- bt_node_free(&priv_token_obj_btree, map->obj_handle, object_free);
+ bt_node_free(&priv_token_obj_btree, map->obj_handle, (void (*)(void *)) object_free);
else
- bt_node_free(&publ_token_obj_btree, map->obj_handle, object_free);
+ bt_node_free(&publ_token_obj_btree, map->obj_handle, (void (*)(void *)) object_free);
}
done:
/* delete @node from this btree */
@@ -1741,7 +1741,7 @@ purge_session_obj_cb(void *node, unsigned long obj_handle, void *p3)
if (obj->map_handle)
bt_node_free(&object_map_btree, obj->map_handle, free);
- bt_node_free(&sess_obj_btree, obj_handle, object_free);
+ bt_node_free(&sess_obj_btree, obj_handle, (void (*)(void *)) object_free);
}
}
}
@@ -1790,7 +1790,7 @@ purge_token_obj_cb(void *node, unsigned long obj_handle, void *p3)
if (obj->map_handle)
bt_node_free(&object_map_btree, obj->map_handle, free);
- bt_node_free(t, obj_handle, object_free);
+ bt_node_free(t, obj_handle, (void (*)(void *)) object_free);
}
// this routine cleans up the list of token objects. in general, we don't
@@ -2343,7 +2343,7 @@ delete_objs_from_btree_cb(void *node, unsigned long obj_handle, void *p3)
}
/* didn't find it in SHM, delete it from its btree */
- bt_node_free(ua->t, obj_handle, object_free);
+ bt_node_free(ua->t, obj_handle, (void (*)(void *)) object_free);
}
void
diff --git a/usr/lib/pkcs11/icsf_stdll/new_host.c b/usr/lib/pkcs11/icsf_stdll/new_host.c
index 941fe90..8c6e6e4 100644
--- a/usr/lib/pkcs11/icsf_stdll/new_host.c
+++ b/usr/lib/pkcs11/icsf_stdll/new_host.c
@@ -344,8 +344,8 @@ void Fork_Initializer(void)
* When implemented... Although logout_all should clear this up.
*/
- bt_destroy(&priv_token_obj_btree, object_free);
- bt_destroy(&publ_token_obj_btree, object_free);
+ bt_destroy(&priv_token_obj_btree, (void (*)(void *)) object_free);
+ bt_destroy(&publ_token_obj_btree, (void (*)(void *)) object_free);
/* Need to do something to prevent the shared memory from
* having the objects loaded again.... The most likely place
--
2.5.0
>From 6566862f208b216301c3b630a4945bc00b2d8bc3 Mon Sep 17 00:00:00 2001
From: Jakub Jelen <jje...@redhat.com>
Date: Tue, 8 Mar 2016 19:16:11 +0100
Subject: [PATCH 6/8] Coverity: Dynamic traces dereferencing null pointers
---
usr/lib/pkcs11/common/new_host.c | 26 +++++++++++++-------------
usr/lib/pkcs11/icsf_stdll/new_host.c | 22 +++++++++++-----------
2 files changed, 24 insertions(+), 24 deletions(-)
diff --git a/usr/lib/pkcs11/common/new_host.c b/usr/lib/pkcs11/common/new_host.c
index ded2a29..af63baf 100755
--- a/usr/lib/pkcs11/common/new_host.c
+++ b/usr/lib/pkcs11/common/new_host.c
@@ -627,7 +627,7 @@ CK_RV SC_GetMechanismList(CK_SLOT_ID sid, CK_MECHANISM_TYPE_PTR pMechList,
}
out:
TRACE_INFO("C_GetMechanismList: rc = 0x%08lx, # mechanisms: %lu\n",
- rc, *count);
+ rc, (count ? *count : 0));
return rc;
}
@@ -1850,7 +1850,7 @@ CK_RV SC_EncryptInit(ST_SESSION_HANDLE *sSession, CK_MECHANISM_PTR pMechanism,
done:
TRACE_INFO("C_EncryptInit: rc = 0x%08lx, sess = %ld, mech = 0x%lx\n",
rc, (sess == NULL) ? -1 : (CK_LONG)sess->handle,
- pMechanism->mechanism);
+ (pMechanism ? pMechanism->mechanism : -1));
return rc;
}
@@ -2083,7 +2083,7 @@ CK_RV SC_DecryptInit(ST_SESSION_HANDLE *sSession, CK_MECHANISM_PTR pMechanism,
done:
TRACE_INFO("C_DecryptInit: rc = 0x%08lx, sess = %ld, mech = 0x%lx\n",
rc, (sess == NULL) ? -1 : (CK_LONG)sess->handle,
- pMechanism->mechanism);
+ (pMechanism ? pMechanism->mechanism : -1));
return rc;
}
@@ -2242,7 +2242,7 @@ done:
TRACE_INFO("C_DecryptFinal: rc = 0x%08lx, sess = %ld, amount = %lu\n",
rc, (sess == NULL) ? -1 : (CK_LONG)sess->handle,
- *pulLastPartLen);
+ (pulLastPartLen ? *pulLastPartLen : 0));
return rc;
}
@@ -2294,7 +2294,7 @@ CK_RV SC_DigestInit(ST_SESSION_HANDLE *sSession, CK_MECHANISM_PTR pMechanism)
done:
TRACE_INFO("C_DigestInit: rc = 0x%08lx, sess = %ld, mech = %lu\n",
rc, (sess == NULL)?-1:(CK_LONG)sess->handle,
- pMechanism->mechanism);
+ (pMechanism ? pMechanism->mechanism : -1));
return rc;
}
@@ -2530,7 +2530,7 @@ CK_RV SC_SignInit(ST_SESSION_HANDLE *sSession, CK_MECHANISM_PTR pMechanism,
done:
TRACE_INFO("C_SignInit: rc = %08lx, sess = %ld, mech = %lx\n",
rc, (sess == NULL)?-1:(CK_LONG)sess->handle,
- pMechanism->mechanism);
+ (pMechanism ? pMechanism->mechanism : -1));
return rc;
}
@@ -2732,7 +2732,7 @@ CK_RV SC_SignRecoverInit(ST_SESSION_HANDLE *sSession,
done:
TRACE_INFO("C_SignRecoverInit: rc = %08lx, sess = %ld, mech = %lx\n",
rc, (sess == NULL)?-1:(CK_LONG)sess->handle,
- pMechanism->mechanism);
+ (pMechanism ? pMechanism->mechanism : -1));
return rc;
}
@@ -2839,7 +2839,7 @@ CK_RV SC_VerifyInit(ST_SESSION_HANDLE *sSession, CK_MECHANISM_PTR pMechanism,
done:
TRACE_INFO("C_VerifyInit: rc = %08lx, sess = %ld, mech = %lx\n",
rc, (sess == NULL)?-1:(CK_LONG)sess->handle,
- pMechanism->mechanism);
+ (pMechanism ? pMechanism->mechanism : -1));
return rc;
}
@@ -3033,7 +3033,7 @@ CK_RV SC_VerifyRecoverInit(ST_SESSION_HANDLE *sSession,
done:
TRACE_INFO("C_VerifyRecoverInit: rc = %08lx, sess = %ld, mech = %lx\n",
rc, (sess == NULL)?-1:(CK_LONG)sess->handle,
- pMechanism->mechanism);
+ (pMechanism ? pMechanism->mechanism : -1));
return rc;
}
@@ -3088,7 +3088,7 @@ done:
TRACE_INFO("C_VerifyRecover: rc = %08lx, sess = %ld, recover len = %lu, "
"length_only = %d\n", rc,
- (sess == NULL)?-1:(CK_LONG)sess->handle, *pulDataLen,
+ (sess == NULL)?-1:(CK_LONG)sess->handle, (pulDataLen ? *pulDataLen : 0),
length_only);
return rc;
@@ -3194,7 +3194,7 @@ CK_RV SC_GenerateKey(ST_SESSION_HANDLE *sSession, CK_MECHANISM_PTR pMechanism,
done:
TRACE_INFO("C_GenerateKey: rc = %08lx, sess = %ld, mech = %lx\n", rc,
(sess == NULL) ? -1 : (CK_LONG) sess->handle,
- pMechanism->mechanism);
+ (pMechanism ? pMechanism->mechanism : -1));
#ifdef DEBUG
CK_ATTRIBUTE *attr = NULL;
@@ -3271,7 +3271,7 @@ CK_RV SC_GenerateKeyPair(ST_SESSION_HANDLE *sSession,
done:
TRACE_INFO("C_GenerateKeyPair: rc = %08lx, sess = %ld, mech = %lu\n",
rc, (sess == NULL) ? -1 : ((CK_LONG) sess->handle),
- pMechanism->mechanism);
+ (pMechanism ? pMechanism->mechanism : -1));
#ifdef DEBUG
CK_ATTRIBUTE *attr = NULL;
@@ -3413,7 +3413,7 @@ done:
TRACE_INFO("C_UnwrapKey: rc = %08lx, sess = %ld, decrypting key = %lu,"
"unwrapped key = %lu\n", rc,
(sess == NULL) ? -1 : (CK_LONG) sess->handle,
- hUnwrappingKey, *phKey);
+ hUnwrappingKey, (phKey ? *phKey : 0));
#ifdef DEBUG
CK_ATTRIBUTE *attr = NULL;
diff --git a/usr/lib/pkcs11/icsf_stdll/new_host.c b/usr/lib/pkcs11/icsf_stdll/new_host.c
index 8c6e6e4..cf957ef 100644
--- a/usr/lib/pkcs11/icsf_stdll/new_host.c
+++ b/usr/lib/pkcs11/icsf_stdll/new_host.c
@@ -605,7 +605,7 @@ CK_RV SC_GetMechanismList(CK_SLOT_ID sid, CK_MECHANISM_TYPE_PTR pMechList,
}
out:
TRACE_INFO("C_GetMechanismList: rc = 0x%08lx, # mechanisms: %lu\n",
- rc, *count);
+ rc, (count ? *count : 0));
return rc;
}
@@ -1591,7 +1591,7 @@ CK_RV SC_EncryptInit(ST_SESSION_HANDLE *sSession, CK_MECHANISM_PTR pMechanism,
done:
TRACE_INFO("C_EncryptInit: rc = 0x%08lx, sess = %ld, mech = 0x%lx\n",
rc, (sess == NULL) ? -1 : (CK_LONG)sess->handle,
- pMechanism->mechanism);
+ (pMechanism ? pMechanism->mechanism : -1));
return rc;
}
@@ -1797,7 +1797,7 @@ CK_RV SC_DecryptInit(ST_SESSION_HANDLE *sSession, CK_MECHANISM_PTR pMechanism,
done:
TRACE_INFO("C_DecryptInit: rc = 0x%08lx, sess = %ld, mech = 0x%lx\n",
rc, (sess == NULL) ? -1 : (CK_LONG)sess->handle,
- pMechanism->mechanism);
+ (pMechanism ? pMechanism->mechanism : -1));
return rc;
}
@@ -1949,7 +1949,7 @@ done:
TRACE_INFO("C_DecryptFinal: rc = 0x%08lx, sess = %ld, amount = %lu\n",
rc, (sess == NULL) ? -1 : (CK_LONG)sess->handle,
- *pulLastPartLen);
+ (pulLastPartLen ? *pulLastPartLen : -1));
return rc;
}
@@ -2001,7 +2001,7 @@ CK_RV SC_DigestInit(ST_SESSION_HANDLE *sSession, CK_MECHANISM_PTR pMechanism)
done:
TRACE_INFO("C_DigestInit: rc = 0x%08lx, sess = %ld, mech = %lx\n",
rc, (sess == NULL)?-1:(CK_LONG)sess->handle,
- pMechanism->mechanism);
+ (pMechanism ? pMechanism->mechanism : -1));
return rc;
}
@@ -2237,7 +2237,7 @@ CK_RV SC_SignInit(ST_SESSION_HANDLE *sSession, CK_MECHANISM_PTR pMechanism,
done:
TRACE_INFO("C_SignInit: rc = %08lx, sess = %ld, mech = %lx\n",
rc, (sess == NULL)?-1:(CK_LONG)sess->handle,
- pMechanism->mechanism);
+ (pMechanism ? pMechanism->mechanism : -1));
return rc;
}
@@ -2458,7 +2458,7 @@ CK_RV SC_VerifyInit(ST_SESSION_HANDLE *sSession, CK_MECHANISM_PTR pMechanism,
done:
TRACE_INFO("C_VerifyInit: rc = %08lx, sess = %ld, mech = %lx\n",
rc, (sess == NULL)?-1:(CK_LONG)sess->handle,
- pMechanism->mechanism);
+ (pMechanism ? pMechanism->mechanism : -1));
return rc;
}
@@ -2730,7 +2730,7 @@ CK_RV SC_GenerateKey(ST_SESSION_HANDLE *sSession, CK_MECHANISM_PTR pMechanism,
done:
TRACE_INFO("C_GenerateKey: rc = %08lx, sess = %ld, mech = %lu\n", rc,
(sess == NULL) ? -1 : (CK_LONG) sess->handle,
- pMechanism->mechanism);
+ (pMechanism ? pMechanism->mechanism : -1));
#ifdef DEBUG
int i;
@@ -2806,7 +2806,7 @@ CK_RV SC_GenerateKeyPair(ST_SESSION_HANDLE *sSession,
done:
TRACE_INFO("C_GenerateKeyPair: rc = %08lx, sess = %ld, mech = %lx\n",
rc, (sess == NULL) ? -1 : ((CK_LONG) sess->handle),
- pMechanism->mechanism);
+ (pMechanism ? pMechanism->mechanism : -1));
#ifdef DEBUG
int i;
@@ -2944,7 +2944,7 @@ done:
TRACE_INFO("C_UnwrapKey: rc = %08lx, sess = %ld, decrypting key = %lu,"
"unwrapped key = %lu\n", rc,
(sess == NULL) ? -1 : (CK_LONG) sess->handle,
- hUnwrappingKey, *phKey);
+ hUnwrappingKey, (phKey ? *phKey : -1));
#ifdef DEBUG
int i;
@@ -3011,7 +3011,7 @@ CK_RV SC_DeriveKey(ST_SESSION_HANDLE *sSession, CK_MECHANISM_PTR pMechanism,
done:
TRACE_INFO("C_DeriveKey: rc = %08lx, sess = %ld, mech = %lu\n",
rc, (sess == NULL)?-1:(CK_LONG)sess->handle,
- pMechanism->mechanism);
+ (pMechanism ? pMechanism->mechanism : -1));
#ifdef DEBUG
int i;
CK_ATTRIBUTE *attr = NULL;
--
2.5.0
>From 44ac49bbdc49307c5f17bbfcbb5d8a2d5b8849d1 Mon Sep 17 00:00:00 2001
From: Jakub Jelen <jje...@redhat.com>
Date: Tue, 8 Mar 2016 19:17:29 +0100
Subject: [PATCH 7/8] Coverity: (possibly) unitialized variables
---
usr/lib/pkcs11/common/object.c | 2 +-
usr/lib/pkcs11/icsf_stdll/icsf_specific.c | 8 ++++++++
usr/lib/pkcs11/icsf_stdll/new_host.c | 2 +-
3 files changed, 10 insertions(+), 2 deletions(-)
diff --git a/usr/lib/pkcs11/common/object.c b/usr/lib/pkcs11/common/object.c
index fe001c1..6b88593 100755
--- a/usr/lib/pkcs11/common/object.c
+++ b/usr/lib/pkcs11/common/object.c
@@ -816,7 +816,7 @@ object_set_attribute_values( OBJECT * obj,
CK_ATTRIBUTE * pTemplate,
CK_ULONG ulCount )
{
- TEMPLATE * new_tmpl;
+ TEMPLATE * new_tmpl = NULL;
CK_BBOOL found;
CK_ULONG class, subclass;
CK_RV rc;
diff --git a/usr/lib/pkcs11/icsf_stdll/icsf_specific.c b/usr/lib/pkcs11/icsf_stdll/icsf_specific.c
index e5c2750..996d62d 100644
--- a/usr/lib/pkcs11/icsf_stdll/icsf_specific.c
+++ b/usr/lib/pkcs11/icsf_stdll/icsf_specific.c
@@ -4203,6 +4203,10 @@ CK_RV icsftok_verify_update(SESSION *session, CK_BYTE *in_data,
if (multi_part_ctx->initiated)
memcpy(chain_data, multi_part_ctx->chain_data,
chain_data_len);
+ } else {
+ TRACE_ERROR("%s\n", ock_err(ERR_ARGUMENTS_BAD));
+ rc = ERR_ARGUMENTS_BAD;
+ goto done;
}
switch (ctx->mech.mechanism) {
@@ -4359,6 +4363,10 @@ CK_RV icsftok_verify_final(SESSION *session, CK_BYTE *signature,
if (ctx->context) {
multi_part_ctx = (struct icsf_multi_part_context *)ctx->context;
memcpy(chain_data, multi_part_ctx->chain_data, chain_data_len);
+ } else {
+ TRACE_ERROR("%s\n", ock_err(ERR_ARGUMENTS_BAD));
+ rc = ERR_ARGUMENTS_BAD;
+ goto done;
}
switch (ctx->mech.mechanism) {
diff --git a/usr/lib/pkcs11/icsf_stdll/new_host.c b/usr/lib/pkcs11/icsf_stdll/new_host.c
index cf957ef..9945b6c 100644
--- a/usr/lib/pkcs11/icsf_stdll/new_host.c
+++ b/usr/lib/pkcs11/icsf_stdll/new_host.c
@@ -998,7 +998,7 @@ CK_RV SC_Login(ST_SESSION_HANDLE *sSession, CK_USER_TYPE userType,
if (!sess) {
TRACE_ERROR("%s\n", ock_err(ERR_SESSION_HANDLE_INVALID));
rc = CKR_SESSION_HANDLE_INVALID;
- goto done;
+ return rc; /* can't goto done; We need sess to log */
}
flags = &nv_token_data->token_info.flags;
--
2.5.0
>From 3c021099c8414e92acdcc6b710d27f621c5f5c3a Mon Sep 17 00:00:00 2001
From: Jakub Jelen <jje...@redhat.com>
Date: Wed, 9 Mar 2016 13:26:28 +0100
Subject: [PATCH 8/8] Coverity: NULL dereference
---
usr/lib/pkcs11/common/mech_aes.c | 6 +++---
usr/lib/pkcs11/common/new_host.c | 2 +-
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/usr/lib/pkcs11/common/mech_aes.c b/usr/lib/pkcs11/common/mech_aes.c
index 296c8e7..afe19e0 100644
--- a/usr/lib/pkcs11/common/mech_aes.c
+++ b/usr/lib/pkcs11/common/mech_aes.c
@@ -2933,14 +2933,14 @@ CK_RV aes_gcm_decrypt(SESSION *sess, CK_BBOOL length_only,
CK_ULONG tag_data_len;
CK_RV rc;
- aesgcm = (CK_GCM_PARAMS *)ctx->mech.pParameter;
- tag_data_len = (aesgcm->ulTagBits + 7) / 8; /* round to full byte */
-
if (!sess || !ctx || !in_data || !out_data_len) {
TRACE_ERROR("%s received bad argument(s)\n", __FUNCTION__);
return CKR_FUNCTION_FAILED;
}
+ aesgcm = (CK_GCM_PARAMS *)ctx->mech.pParameter;
+ tag_data_len = (aesgcm->ulTagBits + 7) / 8; /* round to full byte */
+
if (length_only == TRUE) {
*out_data_len = in_data_len - tag_data_len;
return CKR_OK;
diff --git a/usr/lib/pkcs11/common/new_host.c b/usr/lib/pkcs11/common/new_host.c
index af63baf..397319e 100755
--- a/usr/lib/pkcs11/common/new_host.c
+++ b/usr/lib/pkcs11/common/new_host.c
@@ -3480,7 +3480,7 @@ CK_RV SC_DeriveKey(ST_SESSION_HANDLE *sSession, CK_MECHANISM_PTR pMechanism,
done:
TRACE_INFO("C_DeriveKey: rc = %08lx, sess = %ld, mech = %lu\n",
rc, (sess == NULL)?-1:(CK_LONG)sess->handle,
- pMechanism->mechanism);
+ (pMechanism ? pMechanism->mechanism : -1));
#ifdef DEBUG
CK_ATTRIBUTE *attr = NULL;
CK_BYTE *ptr = NULL;
--
2.5.0
------------------------------------------------------------------------------
Transform Data into Opportunity.
Accelerate data analysis in your applications with
Intel Data Analytics Acceleration Library.
Click to learn more.
http://pubads.g.doubleclick.net/gampad/clk?id=278785111&iu=/4140
_______________________________________________
Opencryptoki-tech mailing list
Opencryptoki-tech@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opencryptoki-tech
