From e84951a5bf7f65d20061b49b6ebd8d739dd76708 Mon Sep 17 00:00:00 2001 From: Frank Morgner Date: Thu, 30 Apr 2015 01:45:23 +0200 Subject: [PATCH] fix resource leaks in while registering PKCS#11 mechanisms introduces a free_mech_data for sc_pkcs11_mechanism_type_t to clear the mechanisms private memory --- src/pkcs11/framework-pkcs15.c | 22 +++++++++++----------- src/pkcs11/mechanism.c | 11 +++++++++-- src/pkcs11/openssl.c | 21 ++++++++++++++------- src/pkcs11/pkcs11-object.c | 3 ++- src/pkcs11/sc-pkcs11.h | 6 ++++-- src/pkcs11/slot.c | 12 +++--------- src/tools/pkcs11-tool.c | 3 +-- 7 files changed, 44 insertions(+), 34 deletions(-) diff --git a/src/pkcs11/framework-pkcs15.c b/src/pkcs11/framework-pkcs15.c index c5809753..d841fceb 100644 --- a/src/pkcs11/framework-pkcs15.c +++ b/src/pkcs11/framework-pkcs15.c @@ -4509,7 +4509,7 @@ register_gost_mechanisms(struct sc_pkcs11_card *p11card, int flags) if (flags & SC_ALGORITHM_GOSTR3410_HASH_NONE) { mt = sc_pkcs11_new_fw_mechanism(CKM_GOSTR3410, - &mech_info, CKK_GOSTR3410, NULL); + &mech_info, CKK_GOSTR3410, NULL, NULL); if (!mt) return CKR_HOST_MEMORY; rc = sc_pkcs11_register_mechanism(p11card, mt); @@ -4518,7 +4518,7 @@ register_gost_mechanisms(struct sc_pkcs11_card *p11card, int flags) } if (flags & SC_ALGORITHM_GOSTR3410_HASH_GOSTR3411) { mt = sc_pkcs11_new_fw_mechanism(CKM_GOSTR3410_WITH_GOSTR3411, - &mech_info, CKK_GOSTR3410, NULL); + &mech_info, CKK_GOSTR3410, NULL, NULL); if (!mt) return CKR_HOST_MEMORY; rc = sc_pkcs11_register_mechanism(p11card, mt); @@ -4528,7 +4528,7 @@ register_gost_mechanisms(struct sc_pkcs11_card *p11card, int flags) if (flags & SC_ALGORITHM_ONBOARD_KEY_GEN) { mech_info.flags = CKF_HW | CKF_GENERATE_KEY_PAIR; mt = sc_pkcs11_new_fw_mechanism(CKM_GOSTR3410_KEY_PAIR_GEN, - &mech_info, CKK_GOSTR3410, NULL); + &mech_info, CKK_GOSTR3410, NULL, NULL); if (!mt) return CKR_HOST_MEMORY; rc = sc_pkcs11_register_mechanism(p11card, mt); @@ -4566,7 +4566,7 @@ static int register_ec_mechanisms(struct sc_pkcs11_card *p11card, int flags, mech_info.ulMinKeySize = min_key_size; mech_info.ulMaxKeySize = max_key_size; - mt = sc_pkcs11_new_fw_mechanism(CKM_ECDSA, &mech_info, CKK_EC, NULL); + mt = sc_pkcs11_new_fw_mechanism(CKM_ECDSA, &mech_info, CKK_EC, NULL, NULL); if (!mt) return CKR_HOST_MEMORY; rc = sc_pkcs11_register_mechanism(p11card, mt); @@ -4575,7 +4575,7 @@ static int register_ec_mechanisms(struct sc_pkcs11_card *p11card, int flags, #if ENABLE_OPENSSL mt = sc_pkcs11_new_fw_mechanism(CKM_ECDSA_SHA1, - &mech_info, CKK_EC, NULL); + &mech_info, CKK_EC, NULL, NULL); if (!mt) return CKR_HOST_MEMORY; rc = sc_pkcs11_register_mechanism(p11card, mt); @@ -4588,14 +4588,14 @@ static int register_ec_mechanisms(struct sc_pkcs11_card *p11card, int flags, mech_info.flags &= ~CKF_SIGN; mech_info.flags |= CKF_DERIVE; - mt = sc_pkcs11_new_fw_mechanism(CKM_ECDH1_COFACTOR_DERIVE, &mech_info, CKK_EC, NULL); + mt = sc_pkcs11_new_fw_mechanism(CKM_ECDH1_COFACTOR_DERIVE, &mech_info, CKK_EC, NULL, NULL); if (!mt) return CKR_HOST_MEMORY; rc = sc_pkcs11_register_mechanism(p11card, mt); if (rc != CKR_OK) return rc; - mt = sc_pkcs11_new_fw_mechanism(CKM_ECDH1_DERIVE, &mech_info, CKK_EC, NULL); + mt = sc_pkcs11_new_fw_mechanism(CKM_ECDH1_DERIVE, &mech_info, CKK_EC, NULL, NULL); if (!mt) return CKR_HOST_MEMORY; rc = sc_pkcs11_register_mechanism(p11card, mt); @@ -4605,7 +4605,7 @@ static int register_ec_mechanisms(struct sc_pkcs11_card *p11card, int flags, if (flags & SC_ALGORITHM_ONBOARD_KEY_GEN) { mech_info.flags = CKF_HW | CKF_GENERATE_KEY_PAIR; mech_info.flags |= ec_flags; - mt = sc_pkcs11_new_fw_mechanism(CKM_EC_KEY_PAIR_GEN, &mech_info, CKK_EC, NULL); + mt = sc_pkcs11_new_fw_mechanism(CKM_EC_KEY_PAIR_GEN, &mech_info, CKK_EC, NULL, NULL); if (!mt) return CKR_HOST_MEMORY; rc = sc_pkcs11_register_mechanism(p11card, mt); @@ -4701,7 +4701,7 @@ register_mechanisms(struct sc_pkcs11_card *p11card) /* Check if we support raw RSA */ if (flags & SC_ALGORITHM_RSA_RAW) { - mt = sc_pkcs11_new_fw_mechanism(CKM_RSA_X_509, &mech_info, CKK_RSA, NULL); + mt = sc_pkcs11_new_fw_mechanism(CKM_RSA_X_509, &mech_info, CKK_RSA, NULL, NULL); rc = sc_pkcs11_register_mechanism(p11card, mt); if (rc != CKR_OK) return rc; @@ -4729,7 +4729,7 @@ register_mechanisms(struct sc_pkcs11_card *p11card) /* No need to Check for PKCS1 We support it in software and turned it on above so always added it */ if (flags & SC_ALGORITHM_RSA_PAD_PKCS1) { - mt = sc_pkcs11_new_fw_mechanism(CKM_RSA_PKCS, &mech_info, CKK_RSA, NULL); + mt = sc_pkcs11_new_fw_mechanism(CKM_RSA_PKCS, &mech_info, CKK_RSA, NULL, NULL); rc = sc_pkcs11_register_mechanism(p11card, mt); if (rc != CKR_OK) return rc; @@ -4776,7 +4776,7 @@ register_mechanisms(struct sc_pkcs11_card *p11card) if (flags & SC_ALGORITHM_ONBOARD_KEY_GEN) { mech_info.flags = CKF_GENERATE_KEY_PAIR; - mt = sc_pkcs11_new_fw_mechanism(CKM_RSA_PKCS_KEY_PAIR_GEN, &mech_info, CKK_RSA, NULL); + mt = sc_pkcs11_new_fw_mechanism(CKM_RSA_PKCS_KEY_PAIR_GEN, &mech_info, CKK_RSA, NULL, NULL); if (!mt) return CKR_HOST_MEMORY; rc = sc_pkcs11_register_mechanism(p11card, mt); diff --git a/src/pkcs11/mechanism.c b/src/pkcs11/mechanism.c index a49acdb6..720bce6f 100644 --- a/src/pkcs11/mechanism.c +++ b/src/pkcs11/mechanism.c @@ -942,7 +942,8 @@ sc_pkcs11_mechanism_type_t * sc_pkcs11_new_fw_mechanism(CK_MECHANISM_TYPE mech, CK_MECHANISM_INFO_PTR pInfo, CK_KEY_TYPE key_type, - void *priv_data) + const void *priv_data, + void (*free_priv_data)(const void *priv_data)) { sc_pkcs11_mechanism_type_t *mt; @@ -953,6 +954,7 @@ sc_pkcs11_new_fw_mechanism(CK_MECHANISM_TYPE mech, mt->mech_info = *pInfo; mt->key_type = key_type; mt->mech_data = priv_data; + mt->free_mech_data = free_priv_data; mt->obj_size = sizeof(sc_pkcs11_operation_t); mt->release = sc_pkcs11_signature_release; @@ -994,6 +996,11 @@ sc_pkcs11_register_generic_mechanisms(struct sc_pkcs11_card *p11card) return CKR_OK; } +void free_info(const void *info) +{ + free((void *) info); +} + /* * Register a sign+hash algorithm derived from an algorithm supported * by the token + a software hash mechanism @@ -1024,7 +1031,7 @@ sc_pkcs11_register_sign_and_hash_mechanism(struct sc_pkcs11_card *p11card, info->sign_mech = sign_type->mech; info->hash_mech = hash_mech; - new_type = sc_pkcs11_new_fw_mechanism(mech, &mech_info, sign_type->key_type, info); + new_type = sc_pkcs11_new_fw_mechanism(mech, &mech_info, sign_type->key_type, info, free_info); if (!new_type) return CKR_HOST_MEMORY; diff --git a/src/pkcs11/openssl.c b/src/pkcs11/openssl.c index 241a9cf2..5120611e 100644 --- a/src/pkcs11/openssl.c +++ b/src/pkcs11/openssl.c @@ -48,7 +48,8 @@ static sc_pkcs11_mechanism_type_t openssl_sha1_mech = { NULL, NULL, NULL, /* verif_* */ NULL, NULL, /* decrypt_* */ NULL, /* derive */ - NULL /* mech_data */ + NULL, /* mech_data */ + NULL, /* free_mech_data */ }; #if OPENSSL_VERSION_NUMBER >= 0x00908000L @@ -65,7 +66,8 @@ static sc_pkcs11_mechanism_type_t openssl_sha256_mech = { NULL, NULL, NULL, /* verif_* */ NULL, NULL, /* decrypt_* */ NULL, /* derive */ - NULL /* mech_data */ + NULL, /* mech_data */ + NULL, /* free_mech_data */ }; static sc_pkcs11_mechanism_type_t openssl_sha384_mech = { @@ -81,7 +83,8 @@ static sc_pkcs11_mechanism_type_t openssl_sha384_mech = { NULL, NULL, NULL, /* verif_* */ NULL, NULL, /* decrypt_* */ NULL, /* derive */ - NULL /* mech_data */ + NULL, /* mech_data */ + NULL, /* free_mech_data */ }; static sc_pkcs11_mechanism_type_t openssl_sha512_mech = { @@ -97,7 +100,8 @@ static sc_pkcs11_mechanism_type_t openssl_sha512_mech = { NULL, NULL, NULL, /* verif_* */ NULL, NULL, /* decrypt_* */ NULL, /* derive */ - NULL /* mech_data */ + NULL, /* mech_data */ + NULL, /* free_mech_data */ }; #endif @@ -115,7 +119,8 @@ static sc_pkcs11_mechanism_type_t openssl_gostr3411_mech = { NULL, NULL, NULL, /* verif_* */ NULL, NULL, /* decrypt_* */ NULL, /* derive */ - NULL /* mech_data */ + NULL, /* mech_data */ + NULL, /* free_mech_data */ }; #endif @@ -132,7 +137,8 @@ static sc_pkcs11_mechanism_type_t openssl_md5_mech = { NULL, NULL, NULL, /* verif_* */ NULL, NULL, /* decrypt_* */ NULL, /* derive */ - NULL /* mech_data */ + NULL, /* mech_data */ + NULL, /* free_mech_data */ }; static sc_pkcs11_mechanism_type_t openssl_ripemd160_mech = { @@ -148,7 +154,8 @@ static sc_pkcs11_mechanism_type_t openssl_ripemd160_mech = { NULL, NULL, NULL, /* verif_* */ NULL, NULL, /* decrypt_* */ NULL, /* derive */ - NULL /* mech_data */ + NULL, /* mech_data */ + NULL, /* free_mech_data */ }; void diff --git a/src/pkcs11/pkcs11-object.c b/src/pkcs11/pkcs11-object.c index 48824cfc..607bbba8 100644 --- a/src/pkcs11/pkcs11-object.c +++ b/src/pkcs11/pkcs11-object.c @@ -47,7 +47,8 @@ static sc_pkcs11_mechanism_type_t find_mechanism = { NULL, /* decrypt_init */ NULL, /* decrypt */ NULL, /* derive */ - NULL /* mech_data */ + NULL, /* mech_data */ + NULL, /* free_mech_data */ }; static void diff --git a/src/pkcs11/sc-pkcs11.h b/src/pkcs11/sc-pkcs11.h index 9bac0630..af0557c8 100644 --- a/src/pkcs11/sc-pkcs11.h +++ b/src/pkcs11/sc-pkcs11.h @@ -275,7 +275,9 @@ struct sc_pkcs11_mechanism_type { CK_BYTE_PTR, CK_ULONG, CK_BYTE_PTR, CK_ULONG_PTR); /* mechanism specific data */ - const void * mech_data; + const void * mech_data; + /* free mechanism specific data */ + void (*free_mech_data)(const void *mech_data); }; typedef struct sc_pkcs11_mechanism_type sc_pkcs11_mechanism_type_t; @@ -403,7 +405,7 @@ sc_pkcs11_mechanism_type_t *sc_pkcs11_find_mechanism(struct sc_pkcs11_card *, CK_MECHANISM_TYPE, unsigned int); sc_pkcs11_mechanism_type_t *sc_pkcs11_new_fw_mechanism(CK_MECHANISM_TYPE, CK_MECHANISM_INFO_PTR, CK_KEY_TYPE, - void *); + const void *, void (*)(const void *)); sc_pkcs11_operation_t *sc_pkcs11_new_operation(sc_pkcs11_session_t *, sc_pkcs11_mechanism_type_t *); void sc_pkcs11_release_operation(sc_pkcs11_operation_t **); diff --git a/src/pkcs11/slot.c b/src/pkcs11/slot.c index 1d9c1a41..e41f7a93 100644 --- a/src/pkcs11/slot.c +++ b/src/pkcs11/slot.c @@ -170,18 +170,12 @@ CK_RV card_removed(sc_reader_t * reader) if (card) { card->framework->unbind(card); sc_disconnect_card(card->card); - /* FIXME: free mechanisms - * spaces allocated by the - * sc_pkcs11_register_sign_and_hash_mechanism - * and sc_pkcs11_new_fw_mechanism. - * but see sc_pkcs11_register_generic_mechanisms for (i=0; i < card->nmechanisms; ++i) { - // if 'mech_data' is a pointer earlier returned by the ?alloc - free(card->mechanisms[i]->mech_data); - // if 'mechanisms[i]' is a pointer earlier returned by the ?alloc + if (card->mechanisms[i]->free_mech_data) { + card->mechanisms[i]->free_mech_data(card->mechanisms[i]->mech_data); + } free(card->mechanisms[i]); } - */ free(card->mechanisms); free(card); } diff --git a/src/tools/pkcs11-tool.c b/src/tools/pkcs11-tool.c index a2f0ad53..0014a030 100644 --- a/src/tools/pkcs11-tool.c +++ b/src/tools/pkcs11-tool.c @@ -2425,9 +2425,8 @@ find_mechanism(CK_SLOT_ID slot, CK_FLAGS flags, else { *result = mechs[0]; } - - free(mechs); } + free(mechs); return count; }