Skip to content

Fix resource leaks and session-pool transition races #665

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
mtrojnar wants to merge 7 commits into
base: master
Choose a base branch
from
Open

Fix resource leaks and session-pool transition races #665

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
018f056
Fix EC key generation error-path leak
mtrojnar Jul 24, 2026
c9912a2
Fix RSA public-key lookup template leak
mtrojnar Jul 24, 2026
b087e68
Make session-pool mode transitions concurrency-safe
mtrojnar Jul 24, 2026
4e96896
Make internal session-pool operations explicit
mtrojnar Jul 24, 2026
df92a24
Make session-pool transitions fork-safe
mtrojnar Jul 24, 2026
6436fbf
Simplify session-pool transition state
mtrojnar Jul 24, 2026
93885f8
Add session-pool concurrency regression tests
mtrojnar Jul 24, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 14 additions & 6 deletions src/libp11-int.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -279,14 +281,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,
Expand Down
8 changes: 4 additions & 4 deletions src/p11_cert.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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 */
Expand Down Expand Up @@ -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;
Expand Down
4 changes: 2 additions & 2 deletions src/p11_ckr.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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}
};
Expand Down
4 changes: 2 additions & 2 deletions src/p11_ec.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand All @@ -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();
Expand Down
16 changes: 8 additions & 8 deletions src/p11_eddsa.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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);
Expand All @@ -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);
Expand Down
4 changes: 2 additions & 2 deletions src/p11_falcon.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion src/p11_front.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Loading
Loading