Fix resource leaks and session-pool transition races

.gitignore

@@ -101,6 +101,8 @@ tests/ed448-keygen-prov
 tests/mldsa87-keygen-prov
 tests/mlkem768-keygen-prov
 tests/check-all-prov
+tests/session-pool-stress
+tests/session-pool-test
 
 tests/*.log
 tests/*.trs

src/libp11-int.h

@@ -94,6 +94,8 @@ struct pkcs11_slot_private {
 	pthread_mutex_t lock;
 	pthread_cond_t cond;
 	int8_t rw_mode, logged_in;
+	int transition_active; /* session-pool transition active */
+	unsigned int sessions_in_use; /* sessions currently checked out */
 	CK_SLOT_ID id;
 	CK_SESSION_HANDLE *session_pool;
 	unsigned int session_head, session_tail, session_poolsize;
@@ -124,7 +126,6 @@ struct pkcs11_object_private {
 	unsigned int forkid;
 	int refcnt;
 	pthread_mutex_t lock;
-	PKCS11_KEY *public; /* our current public object */
 };
 
 struct pkcs11_object_ops {
@@ -279,14 +280,20 @@ extern void pkcs11_CTX_unload(PKCS11_CTX *ctx);
 /* Free a libp11 context */
 extern void pkcs11_CTX_free(PKCS11_CTX *ctx);
 
-/* Open a session in RO or RW mode */
-extern int pkcs11_open_session(PKCS11_SLOT_private *, int rw);
+/* Set the R/O or R/W mode of the session pool */
+extern int pkcs11_session_pool_set_mode(PKCS11_SLOT_private *, int rw);
 
-/* Acquire a session from the slot specific session pool */
-extern int pkcs11_get_session(PKCS11_SLOT_private *, int rw, CK_SESSION_HANDLE *sessionp);
+/* Acquire a session from the slot-specific session pool */
+extern int pkcs11_session_pool_acquire(PKCS11_SLOT_private *, int rw,
+	CK_SESSION_HANDLE *sessionp);
 
-/* Return a session the the slot specific session pool */
-extern void pkcs11_put_session(PKCS11_SLOT_private *, CK_SESSION_HANDLE session);
+/* Switch to R/W mode, log in again if needed, and acquire a session */
+extern int pkcs11_session_pool_acquire_keygen(PKCS11_SLOT_private *,
+	CK_SESSION_HANDLE *sessionp);
+
+/* Release a session back to the slot-specific session pool */
+extern void pkcs11_session_pool_release(PKCS11_SLOT_private *,
+	CK_SESSION_HANDLE session);
 
 /* Get a list of all slots */
 extern int pkcs11_enumerate_slots(PKCS11_CTX_private *ctx,

src/p11_cert.c

@@ -50,11 +50,11 @@ int pkcs11_enumerate_certs(PKCS11_SLOT_private *slot, const PKCS11_CERT *cert_te
 			pkcs11_addattr_s(&tmpl, CKA_LABEL, cert_template->label);
 	}
 
-	if (pkcs11_get_session(slot, 0, &session))
+	if (pkcs11_session_pool_acquire(slot, 0, &session))
 		return -1;
 
 	rv = pkcs11_find_certs(slot, &tmpl, session);
-	pkcs11_put_session(slot, session);
+	pkcs11_session_pool_release(slot, session);
 	if (rv < 0) {
 		pkcs11_destroy_certs(slot);
 		return -1;
@@ -211,7 +211,7 @@ int pkcs11_store_certificate(PKCS11_SLOT_private *slot, X509 *x509, char *label,
 	CK_MECHANISM_TYPE ckm_md;
 
 	/* First, make sure we have a session */
-	if (pkcs11_get_session(slot, 1, &session))
+	if (pkcs11_session_pool_acquire(slot, 1, &session))
 		return -1;
 
 	/* Now build the template */
@@ -295,7 +295,7 @@ int pkcs11_store_certificate(PKCS11_SLOT_private *slot, X509 *x509, char *label,
 	if (rv == CKR_OK) {
 		r = pkcs11_init_cert(slot, session, object, ret_cert);
 	}
-	pkcs11_put_session(slot, session);
+	pkcs11_session_pool_release(slot, session);
 
 	CRYPTOKI_checkerr(CKR_F_PKCS11_STORE_CERTIFICATE, rv);
 	return r;

src/p11_ckr.c

@@ -48,15 +48,15 @@ static ERR_STRING_DATA CKR_str_functs[] = {
 	{ERR_FUNC(CKR_F_PKCS11_LOGOUT), "pkcs11_logout"},
 	{ERR_FUNC(CKR_F_PKCS11_NEXT_CERT), "pkcs11_next_cert"},
 	{ERR_FUNC(CKR_F_PKCS11_NEXT_KEY), "pkcs11_next_key"},
-	{ERR_FUNC(CKR_F_PKCS11_OPEN_SESSION), "pkcs11_open_session"},
+	{ERR_FUNC(CKR_F_PKCS11_OPEN_SESSION), "pkcs11_session_pool_set_mode"},
 	{ERR_FUNC(CKR_F_PKCS11_PRIVATE_DECRYPT), "pkcs11_private_decrypt"},
 	{ERR_FUNC(CKR_F_PKCS11_PRIVATE_ENCRYPT), "pkcs11_private_encrypt"},
 	{ERR_FUNC(CKR_F_PKCS11_RELOAD_KEY), "pkcs11_reload_key"},
 	{ERR_FUNC(CKR_F_PKCS11_SEED_RANDOM), "pkcs11_seed_random"},
 	{ERR_FUNC(CKR_F_PKCS11_STORE_CERTIFICATE), "pkcs11_store_certificate"},
 	{ERR_FUNC(CKR_F_PKCS11_STORE_KEY), "pkcs11_store_key"},
 	{ERR_FUNC(CKR_F_PKCS11_RELOAD_CERTIFICATE), "pkcs11_reload_certificate"},
-	{ERR_FUNC(CKR_F_PKCS11_GET_SESSION), "pkcs11_get_session"},
+	{ERR_FUNC(CKR_F_PKCS11_GET_SESSION), "pkcs11_session_pool_acquire"},
 	{ERR_FUNC(CKR_F_PKCS11_EDDSA_SIGN), "pkcs11_eddsa_sign"},
 	{0, NULL}
 };

src/p11_ec.c

@@ -325,7 +325,7 @@ static EC_KEY *pkcs11_get_ec(PKCS11_OBJECT_private *key)
 	 * Continue even if it fails, as the sign operation does not need
 	 * it if the PKCS#11 module or the hardware can figure this out
 	 */
-	if (pkcs11_get_session(slot, 0, &session)) {
+	if (pkcs11_session_pool_acquire(slot, 0, &session)) {
 		EC_KEY_free(ec);
 		return NULL;
 	}
@@ -335,7 +335,7 @@ static EC_KEY *pkcs11_get_ec(PKCS11_OBJECT_private *key)
 		no_point = pkcs11_get_point_associated(ec, key, CKO_PUBLIC_KEY, session);
 	if (no_point && key->object_class == CKO_PRIVATE_KEY) /* Retry with the certificate */
 		no_point = pkcs11_get_point_associated(ec, key, CKO_CERTIFICATE, session);
-	pkcs11_put_session(slot, session);
+	pkcs11_session_pool_release(slot, session);
 
 	if (key->object_class == CKO_PRIVATE_KEY && EC_KEY_get0_private_key(ec) == NULL) {
 		BIGNUM *bn = BN_new();

src/p11_eddsa.c

@@ -92,10 +92,10 @@ static int pkcs11_eddsa_pmeth_sign(EVP_PKEY_CTX *ctx, unsigned char *sig,
 	if (!slot)
 		return 0;
 
-	if (pkcs11_get_session(slot, 0, &session))
+	if (pkcs11_session_pool_acquire(slot, 0, &session))
 		return 0;
 
-	pkcs11_put_session(slot, session);
+	pkcs11_session_pool_release(slot, session);
 
 	if (!pkcs11_evp_pkey_eddsa_sign(key, sig, siglen, tbs, tbslen))
 		return 0;
@@ -136,10 +136,10 @@ static int pkcs11_eddsa_pmeth_digestsign(EVP_MD_CTX *ctx, unsigned char *sig,
 	if (!slot)
 		return -1;
 
-	if (pkcs11_get_session(slot, 0, &session))
+	if (pkcs11_session_pool_acquire(slot, 0, &session))
 		return -1;
 
-	pkcs11_put_session(slot, session);
+	pkcs11_session_pool_release(slot, session);
 
 	/* Step 1: caller asks for signature length only */
 	if (sig == NULL) {
@@ -351,10 +351,10 @@ static int pkcs11_xdh_pmeth_derive(EVP_PKEY_CTX *ctx, unsigned char *secret,
 	if (!slot)
 		return -1;
 
-	if (pkcs11_get_session(slot, 0, &session))
+	if (pkcs11_session_pool_acquire(slot, 0, &session))
 		return -1;
 
-	pkcs11_put_session(slot, session);
+	pkcs11_session_pool_release(slot, session);
 
 	type = EVP_PKEY_id(pkey);
 	switch (type) {
@@ -755,7 +755,7 @@ static int pkcs11_get_raw_public_key(PKCS11_OBJECT_private *key,
 	slot = key->slot;
 	ctx = slot->ctx;
 
-	if (pkcs11_get_session(slot, 0, &session))
+	if (pkcs11_session_pool_acquire(slot, 0, &session))
 		return -1;
 
 	obj = pkcs11_choose_public_source(key, session, &obj_needs_free);
@@ -775,7 +775,7 @@ static int pkcs11_get_raw_public_key(PKCS11_OBJECT_private *key,
 	}
 
 end:
-	pkcs11_put_session(slot, session);
+	pkcs11_session_pool_release(slot, session);
 
 	if (!ok) {
 		OPENSSL_free(*raw);
@@ -813,8 +813,8 @@ static EVP_PKEY *pkcs11_get_evp_key_ed25519(PKCS11_OBJECT_private *key)
 				EVP_PKEY_free(pkey);
 				return NULL;
 			}
-			/* creates a new EVP_PKEY object which requires its own key object reference */
-			key = pkcs11_object_ref(key);
+			/* The cached EVP_PKEY borrows the object. Returned keys get
+			 * an owning reference in pkcs11_get_key(). */
 			alloc_pkey_ex_index();
 			pkcs11_set_ex_data_pkey(pkey, key);
 			atexit(pkcs11_ed25519_method_free);
@@ -849,11 +849,11 @@ static EVP_PKEY *pkcs11_get_evp_key_ed448(PKCS11_OBJECT_private *key)
 				EVP_PKEY_free(pkey);
 				return NULL;
 			}
-			/* create a new EVP_PKEY object which requires its own key object reference */
-			key = pkcs11_object_ref(key);
+			/* The cached EVP_PKEY borrows the object. Returned keys get
+			 * an owning reference in pkcs11_get_key(). */
 			alloc_pkey_ex_index();
 			pkcs11_set_ex_data_pkey(pkey, key);
-			atexit(pkcs11_ed25519_method_free);
+			atexit(pkcs11_ed448_method_free);
 		}
 	}
 #endif /* OPENSSL_VERSION_NUMBER < 0x40000000L */
@@ -885,8 +885,8 @@ static EVP_PKEY *pkcs11_get_evp_key_x25519(PKCS11_OBJECT_private *key)
 				EVP_PKEY_free(pkey);
 				return NULL;
 			}
-			/* creates a new EVP_PKEY object which requires its own key object reference */
-			key = pkcs11_object_ref(key);
+			/* The cached EVP_PKEY borrows the object. Returned keys get
+			 * an owning reference in pkcs11_get_key(). */
 			alloc_pkey_ex_index();
 			pkcs11_set_ex_data_pkey(pkey, key);
 			atexit(pkcs11_x25519_method_free);
@@ -921,8 +921,8 @@ static EVP_PKEY *pkcs11_get_evp_key_x448(PKCS11_OBJECT_private *key)
 				EVP_PKEY_free(pkey);
 				return NULL;
 			}
-			/* create a new EVP_PKEY object which requires its own key object reference */
-			key = pkcs11_object_ref(key);
+			/* The cached EVP_PKEY borrows the object. Returned keys get
+			 * an owning reference in pkcs11_get_key(). */
 			alloc_pkey_ex_index();
 			pkcs11_set_ex_data_pkey(pkey, key);
 			atexit(pkcs11_x448_method_free);

src/p11_falcon.c

@@ -217,7 +217,7 @@ static int pkcs11_get_raw_public_key(PKCS11_OBJECT_private *key,
 	slot = key->slot;
 	ctx = slot->ctx;
 
-	if (pkcs11_get_session(slot, 0, &session))
+	if (pkcs11_session_pool_acquire(slot, 0, &session))
 		return -1;
 
 	obj = pkcs11_choose_public_source(key, session, &obj_needs_free);
@@ -239,7 +239,7 @@ static int pkcs11_get_raw_public_key(PKCS11_OBJECT_private *key,
 	}
 
 end:
-	pkcs11_put_session(slot, session);
+	pkcs11_session_pool_release(slot, session);
 
 	if (!ok) {
 		OPENSSL_free(*raw);

src/p11_front.c

@@ -75,7 +75,7 @@ int PKCS11_open_session(PKCS11_SLOT *pslot, int rw)
 	PKCS11_SLOT_private *slot = pslot->_private;
 	if (check_slot_fork(slot) < 0)
 		return -1;
-	return pkcs11_open_session(slot, rw);
+	return pkcs11_session_pool_set_mode(slot, rw);
 }
 
 int PKCS11_enumerate_slots(PKCS11_CTX *pctx,