commit c79e899d77a5724635a9d4451a34a240e2c7e891 Author: Ingo Franzki Date: Fri Apr 16 13:41:41 2021 +0200 Fix potential deadlock situation with double read-locks Do not get and read-lock an object twice within the same thread via function object_mgr_find_in_map1(), as this would read-lock the object twice. This could cause a deadlock situation, when in-between the first and the second call to object_mgr_find_in_map1() the token object is modified by another process. The second object_mgr_find_in_map1() would detect that the object has been modified (object_mgr_check_shm()), and would try to re-load the object from the disk. For re-loading, the object is unlocked once, and a write-lock is acquired instead. However, if the current thread has read-locked the object twice, but releases only one read-lock, then it will never get the write lock, because it still owns the read lock itself. To avoid this situation, release the read-lock before calling another function that also acquires the read lock of the object. That way, only one read-lock is held by the current thread, and re-loading the object will not cause a deadlock. Signed-off-by: Ingo Franzki diff --git a/usr/lib/common/decr_mgr.c b/usr/lib/common/decr_mgr.c index 317ef995..9842302b 100644 --- a/usr/lib/common/decr_mgr.c +++ b/usr/lib/common/decr_mgr.c @@ -540,6 +540,10 @@ CK_RV decr_mgr_init(STDLL_TokData_t *tokdata, } memset(ctx->context, 0x0, sizeof(AES_GCM_CONTEXT)); + /* Release obj lock, token specific aes-gcm may re-acquire the lock */ + object_put(tokdata, key_obj, TRUE); + key_obj = NULL; + rc = aes_gcm_init(tokdata, sess, ctx, mech, key_handle, 0); if (rc) { TRACE_ERROR("Could not initialize AES_GCM parms.\n"); diff --git a/usr/lib/common/encr_mgr.c b/usr/lib/common/encr_mgr.c index d3ecdeee..3e85ceab 100644 --- a/usr/lib/common/encr_mgr.c +++ b/usr/lib/common/encr_mgr.c @@ -537,6 +537,10 @@ CK_RV encr_mgr_init(STDLL_TokData_t *tokdata, } memset(ctx->context, 0x0, sizeof(AES_GCM_CONTEXT)); + /* Release obj lock, token specific aes-gcm may re-acquire the lock */ + object_put(tokdata, key_obj, TRUE); + key_obj = NULL; + rc = aes_gcm_init(tokdata, sess, ctx, mech, key_handle, 1); if (rc != CKR_OK) { TRACE_ERROR("Could not initialize AES_GCM parms.\n"); diff --git a/usr/lib/common/mech_rsa.c b/usr/lib/common/mech_rsa.c index 1652f90a..e35b383c 100644 --- a/usr/lib/common/mech_rsa.c +++ b/usr/lib/common/mech_rsa.c @@ -602,6 +602,10 @@ CK_RV rsa_oaep_crypt(STDLL_TokData_t *tokdata, SESSION *sess, goto done; } + /* Release obj lock, token specific rsa-oaep may re-acquire the lock */ + object_put(tokdata, key_obj, TRUE); + key_obj = NULL; + rc = token_specific.t_rsa_oaep_encrypt(tokdata, ctx, in_data, in_data_len, out_data, out_data_len, hash, hlen); @@ -625,6 +629,10 @@ CK_RV rsa_oaep_crypt(STDLL_TokData_t *tokdata, SESSION *sess, goto done; } + /* Release obj lock, token specific rsa-oaep may re-acquire the lock */ + object_put(tokdata, key_obj, TRUE); + key_obj = NULL; + rc = token_specific.t_rsa_oaep_decrypt(tokdata, ctx, in_data, in_data_len, out_data, out_data_len, hash, hlen); @@ -1331,6 +1339,10 @@ CK_RV rsa_pss_sign(STDLL_TokData_t *tokdata, SESSION *sess, goto done; } + /* Release obj lock, token specific rsa_pss may re-acquire the lock */ + object_put(tokdata, key_obj, TRUE); + key_obj = NULL; + rc = token_specific.t_rsa_pss_sign(tokdata, sess, ctx, in_data, in_data_len, out_data, out_data_len); if (rc != CKR_OK) @@ -1389,6 +1401,10 @@ CK_RV rsa_pss_verify(STDLL_TokData_t *tokdata, SESSION *sess, goto done; } + /* Release obj lock, token specific rsa_pss may re-acquire the lock */ + object_put(tokdata, key_obj, TRUE); + key_obj = NULL; + rc = token_specific.t_rsa_pss_verify(tokdata, sess, ctx, in_data, in_data_len, signature, sig_len); if (rc != CKR_OK) diff --git a/usr/lib/common/sign_mgr.c b/usr/lib/common/sign_mgr.c index 937a371a..c7268e01 100644 --- a/usr/lib/common/sign_mgr.c +++ b/usr/lib/common/sign_mgr.c @@ -424,6 +424,10 @@ CK_RV sign_mgr_init(STDLL_TokData_t *tokdata, ctx->context_len = 0; ctx->context = NULL; + /* Release obj lock, token specific hmac-sign may re-acquire the lock */ + object_put(tokdata, key_obj, TRUE); + key_obj = NULL; + rc = hmac_sign_init(tokdata, sess, mech, key); if (rc != CKR_OK) { TRACE_ERROR("Failed to initialize hmac.\n"); diff --git a/usr/lib/ep11_stdll/ep11_specific.c b/usr/lib/ep11_stdll/ep11_specific.c index 3ac3768a..52f95d7a 100644 --- a/usr/lib/ep11_stdll/ep11_specific.c +++ b/usr/lib/ep11_stdll/ep11_specific.c @@ -6948,6 +6948,13 @@ CK_RV ep11tok_sign_init(STDLL_TokData_t * tokdata, SESSION * session, rc = ep11tok_pkey_check(tokdata, session, key_obj, mech); switch (rc) { case CKR_OK: + /* + * Release obj lock, sign_mgr_init or ep11tok_sign_verify_init_ibm_ed + * may re-acquire the lock + */ + object_put(tokdata, key_obj, TRUE); + key_obj = NULL; + /* Note that Edwards curves in general are not yet supported in * opencryptoki. These two special IBM specific ED mechs are only * supported by the ep11token, so let's keep them local here. */ @@ -7029,11 +7036,16 @@ CK_RV ep11tok_sign(STDLL_TokData_t * tokdata, SESSION * session, * opencryptoki. These two special IBM specific ED mechs are only * supported by the ep11token, so let's keep them local here. */ if (ctx->mech.mechanism == CKM_IBM_ED25519_SHA512 || - ctx->mech.mechanism == CKM_IBM_ED448_SHA3) + ctx->mech.mechanism == CKM_IBM_ED448_SHA3) { rc = pkey_ibm_ed_sign(key_obj, in_data, in_data_len, signature, sig_len); - else + } else { + /* Release obj lock, sign_mgr_sign may re-acquire the lock */ + object_put(tokdata, key_obj, TRUE); + key_obj = NULL; + rc = sign_mgr_sign(tokdata, session, length_only, ctx, in_data, in_data_len, signature, sig_len); + } goto done; /* no ep11 fallback possible */ } @@ -7071,6 +7083,11 @@ CK_RV ep11tok_sign_update(STDLL_TokData_t * tokdata, SESSION * session, if (!in_data || !in_data_len) return CKR_OK; + if (ctx->pkey_active) { + rc = sign_mgr_sign_update(tokdata, session, ctx, in_data, in_data_len); + goto done; /* no ep11 fallback possible */ + } + rc = h_opaque_2_blob(tokdata, ctx->key, &keyblob, &keyblobsize, &key_obj, READ_LOCK); if (rc != CKR_OK) { @@ -7078,11 +7095,6 @@ CK_RV ep11tok_sign_update(STDLL_TokData_t * tokdata, SESSION * session, return rc; } - if (ctx->pkey_active) { - rc = sign_mgr_sign_update(tokdata, session, ctx, in_data, in_data_len); - goto done; /* no ep11 fallback possible */ - } - RETRY_START rc = dll_m_SignUpdate(ctx->context, ctx->context_len, in_data, in_data_len, ep11_data->target); @@ -7115,6 +7127,11 @@ CK_RV ep11tok_sign_final(STDLL_TokData_t * tokdata, SESSION * session, CK_BYTE *keyblob; OBJECT *key_obj = NULL; + if (ctx->pkey_active) { + rc = sign_mgr_sign_final(tokdata, session, length_only, ctx, signature, sig_len); + goto done; /* no ep11 fallback possible */ + } + rc = h_opaque_2_blob(tokdata, ctx->key, &keyblob, &keyblobsize, &key_obj, READ_LOCK); if (rc != CKR_OK) { @@ -7122,11 +7139,6 @@ CK_RV ep11tok_sign_final(STDLL_TokData_t * tokdata, SESSION * session, return rc; } - if (ctx->pkey_active) { - rc = sign_mgr_sign_final(tokdata, session, length_only, ctx, signature, sig_len); - goto done; /* no ep11 fallback possible */ - } - RETRY_START rc = dll_m_SignFinal(ctx->context, ctx->context_len, signature, sig_len, ep11_data->target); @@ -7241,6 +7253,13 @@ CK_RV ep11tok_verify_init(STDLL_TokData_t * tokdata, SESSION * session, rc = ep11tok_pkey_check(tokdata, session, key_obj, mech); switch (rc) { case CKR_OK: + /* + * Release obj lock, verify_mgr_init or ep11tok_sign_verify_init_ibm_ed + * may re-acquire the lock + */ + object_put(tokdata, key_obj, TRUE); + key_obj = NULL; + /* Note that Edwards curves in general are not yet supported in * opencryptoki. These two special IBM specific ED mechs are only * supported by the ep11token, so let's keep them local here. */ @@ -7320,12 +7339,17 @@ CK_RV ep11tok_verify(STDLL_TokData_t * tokdata, SESSION * session, * opencryptoki. These two special IBM specific ED mechs are only * supported by the ep11token, so let's keep them local here. */ if (ctx->mech.mechanism == CKM_IBM_ED25519_SHA512 || - ctx->mech.mechanism == CKM_IBM_ED448_SHA3) + ctx->mech.mechanism == CKM_IBM_ED448_SHA3) { rc = pkey_ibm_ed_verify(key_obj, in_data, in_data_len, signature, sig_len); - else + } else { + /* Release obj lock, verify_mgr_verify may re-acquire the lock */ + object_put(tokdata, key_obj, TRUE); + key_obj = NULL; + rc = verify_mgr_verify(tokdata, session, ctx, in_data, in_data_len, signature, sig_len); + } goto done; /* no ep11 fallback possible */ } @@ -7363,6 +7387,11 @@ CK_RV ep11tok_verify_update(STDLL_TokData_t * tokdata, SESSION * session, if (!in_data || !in_data_len) return CKR_OK; + if (ctx->pkey_active) { + rc = verify_mgr_verify_update(tokdata, session, ctx, in_data, in_data_len); + goto done; /* no ep11 fallback possible */ + } + rc = h_opaque_2_blob(tokdata, ctx->key, &keyblob, &keyblobsize, &key_obj, READ_LOCK); if (rc != CKR_OK) { @@ -7370,11 +7399,6 @@ CK_RV ep11tok_verify_update(STDLL_TokData_t * tokdata, SESSION * session, return rc; } - if (ctx->pkey_active) { - rc = verify_mgr_verify_update(tokdata, session, ctx, in_data, in_data_len); - goto done; /* no ep11 fallback possible */ - } - RETRY_START rc = dll_m_VerifyUpdate(ctx->context, ctx->context_len, in_data, in_data_len, ep11_data->target); @@ -7406,6 +7430,11 @@ CK_RV ep11tok_verify_final(STDLL_TokData_t * tokdata, SESSION * session, CK_BYTE *keyblob; OBJECT *key_obj = NULL; + if (ctx->pkey_active) { + rc = verify_mgr_verify_final(tokdata, session, ctx, signature, sig_len); + goto done; /* no ep11 fallback possible */ + } + rc = h_opaque_2_blob(tokdata, ctx->key, &keyblob, &keyblobsize, &key_obj, READ_LOCK); if (rc != CKR_OK) { @@ -7413,11 +7442,6 @@ CK_RV ep11tok_verify_final(STDLL_TokData_t * tokdata, SESSION * session, return rc; } - if (ctx->pkey_active) { - rc = verify_mgr_verify_final(tokdata, session, ctx, signature, sig_len); - goto done; /* no ep11 fallback possible */ - } - RETRY_START rc = dll_m_VerifyFinal(ctx->context, ctx->context_len, signature, sig_len, ep11_data->target); @@ -7501,6 +7525,12 @@ CK_RV ep11tok_decrypt_final(STDLL_TokData_t * tokdata, SESSION * session, CK_BYTE *keyblob; OBJECT *key_obj = NULL; + if (ctx->pkey_active) { + rc = decr_mgr_decrypt_final(tokdata, session, length_only, + ctx, output_part, p_output_part_len); + goto done; /* no ep11 fallback possible */ + } + rc = h_opaque_2_blob(tokdata, ctx->key, &keyblob, &keyblobsize, &key_obj, READ_LOCK); if (rc != CKR_OK) { @@ -7508,12 +7538,6 @@ CK_RV ep11tok_decrypt_final(STDLL_TokData_t * tokdata, SESSION * session, return rc; } - if (ctx->pkey_active) { - rc = decr_mgr_decrypt_final(tokdata, session, length_only, - ctx, output_part, p_output_part_len); - goto done; /* no ep11 fallback possible */ - } - RETRY_START rc = dll_m_DecryptFinal(ctx->context, ctx->context_len, output_part, p_output_part_len, @@ -7548,13 +7572,6 @@ CK_RV ep11tok_decrypt(STDLL_TokData_t * tokdata, SESSION * session, CK_BYTE *keyblob; OBJECT *key_obj = NULL; - rc = h_opaque_2_blob(tokdata, ctx->key, &keyblob, &keyblobsize, &key_obj, - READ_LOCK); - if (rc != CKR_OK) { - TRACE_ERROR("%s h_opaque_2_blob, rc=0x%lx\n", __func__, rc); - return rc; - } - if (ctx->pkey_active) { rc = decr_mgr_decrypt(tokdata, session, length_only, ctx, input_data, input_data_len, output_data, @@ -7562,6 +7579,13 @@ CK_RV ep11tok_decrypt(STDLL_TokData_t * tokdata, SESSION * session, goto done; /* no ep11 fallback possible */ } + rc = h_opaque_2_blob(tokdata, ctx->key, &keyblob, &keyblobsize, &key_obj, + READ_LOCK); + if (rc != CKR_OK) { + TRACE_ERROR("%s h_opaque_2_blob, rc=0x%lx\n", __func__, rc); + return rc; + } + RETRY_START rc = dll_m_Decrypt(ctx->context, ctx->context_len, input_data, input_data_len, output_data, p_output_data_len, @@ -7602,13 +7626,6 @@ CK_RV ep11tok_decrypt_update(STDLL_TokData_t * tokdata, SESSION * session, return CKR_OK; /* nothing to update, keep context */ } - rc = h_opaque_2_blob(tokdata, ctx->key, &keyblob, &keyblobsize, &key_obj, - READ_LOCK); - if (rc != CKR_OK) { - TRACE_ERROR("%s h_opaque_2_blob, rc=0x%lx\n", __func__, rc); - return rc; - } - if (ctx->pkey_active) { rc = decr_mgr_decrypt_update(tokdata, session, length_only, ctx, input_part, input_part_len, @@ -7616,6 +7633,13 @@ CK_RV ep11tok_decrypt_update(STDLL_TokData_t * tokdata, SESSION * session, goto done; /* no ep11 fallback possible */ } + rc = h_opaque_2_blob(tokdata, ctx->key, &keyblob, &keyblobsize, &key_obj, + READ_LOCK); + if (rc != CKR_OK) { + TRACE_ERROR("%s h_opaque_2_blob, rc=0x%lx\n", __func__, rc); + return rc; + } + RETRY_START rc = dll_m_DecryptUpdate(ctx->context, ctx->context_len, input_part, input_part_len, output_part, @@ -7695,6 +7719,12 @@ CK_RV ep11tok_encrypt_final(STDLL_TokData_t * tokdata, SESSION * session, CK_BYTE *keyblob; OBJECT *key_obj = NULL; + if (ctx->pkey_active) { + rc = encr_mgr_encrypt_final(tokdata, session, length_only, + ctx, output_part, p_output_part_len); + goto done; /* no ep11 fallback possible */ + } + rc = h_opaque_2_blob(tokdata, ctx->key, &keyblob, &keyblobsize, &key_obj, READ_LOCK); if (rc != CKR_OK) { @@ -7702,12 +7732,6 @@ CK_RV ep11tok_encrypt_final(STDLL_TokData_t * tokdata, SESSION * session, return rc; } - if (ctx->pkey_active) { - rc = encr_mgr_encrypt_final(tokdata, session, length_only, - ctx, output_part, p_output_part_len); - goto done; /* no ep11 fallback possible */ - } - RETRY_START rc = dll_m_EncryptFinal(ctx->context, ctx->context_len, output_part, p_output_part_len, @@ -7742,13 +7766,6 @@ CK_RV ep11tok_encrypt(STDLL_TokData_t * tokdata, SESSION * session, CK_BYTE *keyblob; OBJECT *key_obj = NULL; - rc = h_opaque_2_blob(tokdata, ctx->key, &keyblob, &keyblobsize, &key_obj, - READ_LOCK); - if (rc != CKR_OK) { - TRACE_ERROR("%s h_opaque_2_blob, rc=0x%lx\n", __func__, rc); - return rc; - } - if (ctx->pkey_active) { rc = encr_mgr_encrypt(tokdata, session, length_only, ctx, input_data, input_data_len, output_data, @@ -7756,6 +7773,13 @@ CK_RV ep11tok_encrypt(STDLL_TokData_t * tokdata, SESSION * session, goto done; /* no ep11 fallback possible */ } + rc = h_opaque_2_blob(tokdata, ctx->key, &keyblob, &keyblobsize, &key_obj, + READ_LOCK); + if (rc != CKR_OK) { + TRACE_ERROR("%s h_opaque_2_blob, rc=0x%lx\n", __func__, rc); + return rc; + } + RETRY_START rc = dll_m_Encrypt(ctx->context, ctx->context_len, input_data, input_data_len, output_data, p_output_data_len, @@ -7796,13 +7820,6 @@ CK_RV ep11tok_encrypt_update(STDLL_TokData_t * tokdata, SESSION * session, return CKR_OK; /* nothing to update, keep context */ } - rc = h_opaque_2_blob(tokdata, ctx->key, &keyblob, &keyblobsize, &key_obj, - READ_LOCK); - if (rc != CKR_OK) { - TRACE_ERROR("%s h_opaque_2_blob, rc=0x%lx\n", __func__, rc); - return rc; - } - if (ctx->pkey_active) { rc = encr_mgr_encrypt_update(tokdata, session, length_only, ctx, input_part, input_part_len, output_part, @@ -7810,6 +7827,13 @@ CK_RV ep11tok_encrypt_update(STDLL_TokData_t * tokdata, SESSION * session, goto done; /* no ep11 fallback possible */ } + rc = h_opaque_2_blob(tokdata, ctx->key, &keyblob, &keyblobsize, &key_obj, + READ_LOCK); + if (rc != CKR_OK) { + TRACE_ERROR("%s h_opaque_2_blob, rc=0x%lx\n", __func__, rc); + return rc; + } + RETRY_START rc = dll_m_EncryptUpdate(ctx->context, ctx->context_len, input_part, input_part_len, output_part, @@ -7921,6 +7945,10 @@ static CK_RV ep11_ende_crypt_init(STDLL_TokData_t * tokdata, SESSION * session, rc = ep11tok_pkey_check(tokdata, session, key_obj, mech); switch (rc) { case CKR_OK: + /* Release obj lock, encr/decr_mgr_init may re-acquire the lock */ + object_put(tokdata, key_obj, TRUE); + key_obj = NULL; + if (op == DECRYPT) { rc = decr_mgr_init(tokdata, session, &session->decr_ctx, OP_DECRYPT_INIT, mech, key);