Skip to content

Commit ff53bac

Browse files
committed
Fix leak and use-after-free of session objects on session close
SessionObjects were kept alive in allObjects as tombstones with valid=false after their session closed, so every object ever created lingered until C_Finalize, holding its Mutex*. Long-running clients saw unbounded memory growth dominated by mutex objects. Add refcounting to OSObject and have SessionObjectStore, HandleManager, and C_FindObjectsInit acquire/release across the boundaries where session-object pointers escape the store mutex. Releases happen outside the relevant lock to avoid widening critical sections. Also erase from allObjects in the store cleanup paths to avoid a double-free against the destructor.
1 parent 679f33d commit ff53bac

11 files changed

Lines changed: 434 additions & 312 deletions

File tree

src/lib/SoftHSM.cpp

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2048,9 +2048,23 @@ CK_RV SoftHSM::C_FindObjectsInit(CK_SESSION_HANDLE hSession, CK_ATTRIBUTE_PTR pT
20482048
// Check if we are out of memory
20492049
if (findOp == NULL_PTR) return CKR_HOST_MEMORY;
20502050

2051+
std::set<OSObject*> tokenObjects;
2052+
std::set<OSObject*> sessionObjects;
2053+
token->getObjects(tokenObjects);
2054+
sessionObjectStore->getObjects(slot->getSlotID(), sessionObjects);
2055+
2056+
struct SessionObjReleaser {
2057+
std::set<OSObject*>& s;
2058+
~SessionObjReleaser()
2059+
{
2060+
for (auto* o : s)
2061+
o->release();
2062+
}
2063+
} sessionObjectGuard{sessionObjects};
2064+
20512065
std::set<OSObject*> allObjects;
2052-
token->getObjects(allObjects);
2053-
sessionObjectStore->getObjects(slot->getSlotID(),allObjects);
2066+
allObjects.insert(tokenObjects.begin(), tokenObjects.end());
2067+
allObjects.insert(sessionObjects.begin(), sessionObjects.end());
20542068

20552069
std::set<CK_OBJECT_HANDLE> handles;
20562070
std::set<OSObject*>::iterator it;

src/lib/handle_mgr/CMakeLists.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ set(INCLUDE_DIRS ${PROJECT_SOURCE_DIR}
66
${PROJECT_SOURCE_DIR}/../data_mgr
77
${PROJECT_SOURCE_DIR}/../object_store
88
${PROJECT_SOURCE_DIR}/../pkcs11
9+
${PROJECT_SOURCE_DIR}/../session_mgr
910
${PROJECT_SOURCE_DIR}/../slot_mgr
1011
)
1112

src/lib/handle_mgr/Handle.cpp

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -34,17 +34,20 @@
3434

3535
// Constructor
3636
Handle::Handle(CK_HANDLE_KIND _kind, CK_SLOT_ID _slotID, CK_SESSION_HANDLE _hSession)
37-
: kind(_kind), slotID(_slotID), hSession(_hSession), object(NULL_PTR), isPrivate(false)
37+
: kind(_kind), slotID(_slotID), hSession(_hSession), object(nullptr), session(nullptr),
38+
isPrivate(false)
3839
{
3940
}
4041

4142
Handle::Handle(CK_HANDLE_KIND _kind, CK_SLOT_ID _slotID)
42-
: kind(_kind), slotID(_slotID), hSession(CK_INVALID_HANDLE), object(NULL_PTR), isPrivate(false)
43+
: kind(_kind), slotID(_slotID), hSession(CK_INVALID_HANDLE), object(nullptr), session(nullptr),
44+
isPrivate(false)
4345
{
4446
}
4547

4648
Handle::Handle()
47-
: kind(CKH_INVALID), slotID(0), hSession(CK_INVALID_HANDLE), object(NULL_PTR), isPrivate(false)
49+
: kind(CKH_INVALID), slotID(0), hSession(CK_INVALID_HANDLE), object(nullptr), session(nullptr),
50+
isPrivate(false)
4851
{
4952

5053
}

src/lib/handle_mgr/Handle.h

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,8 @@
3434
#define _SOFTHSM_V2_HANDLE_H
3535

3636
#include "cryptoki.h"
37+
#include "OSObject.h"
38+
#include "Session.h"
3739

3840
enum {
3941
CKH_INVALID,
@@ -54,7 +56,8 @@ class Handle
5456
CK_SLOT_ID slotID;
5557
CK_SESSION_HANDLE hSession;
5658

57-
CK_VOID_PTR object;
59+
OSObject *object;
60+
Session *session;
5861
bool isPrivate;
5962
};
6063

0 commit comments

Comments
 (0)