Skip to content

Commit

Permalink
Fix memory handling in slot refresh
Browse files Browse the repository at this point in the history
On refreshing slots, there were two issues:
- When reusing a PKCS11_SLOT_PRIVATE structure instance, the instance to
  be reused was accidentally freed
- Looking for an instance in the list of slots had bugs in pointer
  usage.
  • Loading branch information
Pavol Marko authored and mtrojnar committed Aug 8, 2023
1 parent 34c1b1c commit 6c96847
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 9 deletions.
5 changes: 3 additions & 2 deletions src/libp11-int.h
Original file line number Diff line number Diff line change
Expand Up @@ -220,8 +220,9 @@ extern unsigned long pkcs11_get_slotid_from_slot(PKCS11_SLOT_private *);
/* Increment slot reference count */
extern PKCS11_SLOT_private *pkcs11_slot_ref(PKCS11_SLOT_private *slot);

/* Decrement slot reference count, free if it becomes zero */
extern void pkcs11_slot_unref(PKCS11_SLOT_private *slot);
/* Decrement slot reference count, free if it becomes zero.
* Returns 1 if it was freed. */
extern int pkcs11_slot_unref(PKCS11_SLOT_private *slot);

/* Free the list of slots allocated by PKCS11_enumerate_slots() */
extern void pkcs11_release_all_slots(PKCS11_SLOT *slots, unsigned int nslots);
Expand Down
23 changes: 16 additions & 7 deletions src/p11_slot.c
Original file line number Diff line number Diff line change
Expand Up @@ -76,9 +76,14 @@ int pkcs11_enumerate_slots(PKCS11_CTX_private *ctx, PKCS11_SLOT **slotp,
for (n = 0; n < nslots; n++) {
PKCS11_SLOT_private *slot = NULL;
for (i = 0; i < *countp; i++) {
if (PRIVSLOT(slotp[i])->id != slotid[n])
PKCS11_SLOT_private *slot_old_private =
PRIVSLOT(&((*slotp)[i]));
if (slot_old_private->id != slotid[n])
continue;
slot = pkcs11_slot_ref(PRIVSLOT(slotp[i]));
/* Increase ref count so it doesn't get freed when ref
* count is decremented in pkcs11_release_all_slots
* at the end of this function. */
slot = pkcs11_slot_ref(slot_old_private);
break;
}
if (!slot)
Expand Down Expand Up @@ -447,10 +452,10 @@ PKCS11_SLOT_private *pkcs11_slot_ref(PKCS11_SLOT_private *slot)
return slot;
}

void pkcs11_slot_unref(PKCS11_SLOT_private *slot)
int pkcs11_slot_unref(PKCS11_SLOT_private *slot)
{
if (pkcs11_atomic_add(&slot->refcnt, -1, &slot->lock) != 0)
return;
return 0;

pkcs11_wipe_cache(slot);
if (slot->prev_pin) {
Expand All @@ -461,6 +466,8 @@ void pkcs11_slot_unref(PKCS11_SLOT_private *slot)
OPENSSL_free(slot->session_pool);
pthread_mutex_destroy(&slot->lock);
pthread_cond_destroy(&slot->cond);

return 1;
}

static int pkcs11_init_slot(PKCS11_CTX_private *ctx, PKCS11_SLOT *slot, PKCS11_SLOT_private *spriv)
Expand Down Expand Up @@ -500,11 +507,13 @@ static void pkcs11_release_slot(PKCS11_SLOT *slot)
pkcs11_destroy_token(slot->token);
OPENSSL_free(slot->token);
}
if (spriv)
pkcs11_slot_unref(spriv);
if (spriv) {
if (pkcs11_slot_unref(spriv) != 0) {
OPENSSL_free(slot->_private);
}
}
OPENSSL_free(slot->description);
OPENSSL_free(slot->manufacturer);
OPENSSL_free(slot->_private);

memset(slot, 0, sizeof(*slot));
}
Expand Down

0 comments on commit 6c96847

Please sign in to comment.