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 10 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
10 commits
Select commit Hold shift + click to select a range
8fbf767
Fix EC key generation error-path leak
mtrojnar Jul 24, 2026
4a19cb0
Fix RSA public-key lookup template leak
mtrojnar Jul 24, 2026
5ec85c6
Make session-pool mode transitions concurrency-safe
mtrojnar Jul 27, 2026
d33f9ec
Make internal session-pool operations explicit
mtrojnar Jul 27, 2026
c991e7f
Add session-pool concurrency regression tests
mtrojnar Jul 27, 2026
47a80f7
Fix key-cache use-after-free in EVP_PKEY ex-data
mtrojnar Jul 27, 2026
2cc8326
Fix cached EdDSA and XDH key object leaks
mtrojnar Jul 27, 2026
1d761b6
Register the correct Ed448 method cleanup
mtrojnar Jul 27, 2026
dfef24b
Keep ENGINE references alive until test keys are freed
mtrojnar Jul 27, 2026
09e064b
Keep provider EC group name alive while building params
mtrojnar Jul 27, 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
21 changes: 14 additions & 7 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 @@ -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 {
Expand Down Expand Up @@ -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,
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
34 changes: 17 additions & 17 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 Expand Up @@ -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);
Expand Down Expand Up @@ -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 */
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
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