WolfSSL
/
wolfssl
mirror of https://github.com/wolfSSL/wolfssl.git
1
0
Fork 0
Code Issues Releases Wiki Activity

Only release the sniffer's shared state on the last free 

The sniffer's session, server and secret tables are qualified THREAD_LS_T, so
each thread that calls ssl_InitSniffer*() owns its own. Its trace file, its
mutexes and its crypto device are not: they are shared by the whole process.
ssl_FreeSniffer() released all of it on every call, so in a build such as
snifftest with THREADED_SNIFFTEST, where every worker thread initializes and
frees the sniffer for itself, a thread that finished early closed the trace
file and freed the mutexes under the threads still running. A worker that was
still registering its key then crashed inside ssl_SetNamedEphemeralKey(),
writing to a file another thread had closed:

  flockfile -> vfprintf -> fprintf -> ssl_SetNamedEphemeralKey -> load_key

Count the callers of ssl_InitSniffer_ex(), which ssl_InitSniffer() and
ssl_InitSniffer_ex2() both funnel through, and only release the shared state
once the count reaches zero. The count is atomic where the platform offers
atomics and a plain int otherwise, which covers the single threaded build. The
per-thread tables are still freed on every call, and wolfSSL_Cleanup() is
still called every time, since it keeps a count of its own.

Verified with two ssl_InitSniffer() callers and one ssl_FreeSniffer(): before
this the trace file was closed by the early free and everything traced after
it was lost, now both entries are present. The full sniffer test suite passes
with and without THREADED_SNIFFTEST.
pull/11319/head
Tobias Frauenschläger 2026-08-28 15:13:09 +02:00
parent 1a303870c5
commit 3c503f5933
3 changed files with 141 additions and 18 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
ChangeLog.md
View File

@ -201,6 +201,16 @@
decrypt output buffer; that overread is confirmed by AddressSanitizer and is
fixed here.
* **Fix (`ssl_FreeSniffer()` tore down state shared by every thread)**: the
sniffer's session, server and secret tables are per thread, but its trace
file, mutexes and crypto device belong to the whole process, and every call
released all of it. A thread that finished early closed the trace file and
freed the mutexes under the threads still running. The init entry points now
count their callers and only the last free releases the shared state; a free
with no matching init leaves the count at zero rather than driving it
negative. `StatsMutex` was initialized but never freed, and is now released
with the rest. Single threaded use is unaffected.
* **Fix (`snifftest` could not report a failed capture, and decrypted nothing
when threaded)**: the read loop assigned `hadBadPacket` on every packet
instead of accumulating it, so an early error was erased by any later packet

139
src/sniffer.c
View File

@ -232,6 +232,80 @@ BOOL APIENTRY DllMain( HMODULE hModule,
static WC_THREADSHARED int TraceOn = 0; /* Trace is off by default */
static WC_THREADSHARED XFILE TraceFile = 0;
/* ssl_InitSniffer*() may be called from several threads, and each of them
* calls ssl_FreeSniffer() when it is done. The session, server and secret
* tables are per thread, but the trace file, the mutexes and the crypto
* device are shared, so only the last caller out may tear those down. */
#if defined(WOLFSSL_ATOMIC_OPS) && defined(WOLFSSL_ATOMIC_INITIALIZER) && \
!defined(SINGLE_THREADED)
static WC_THREADSHARED wolfSSL_Atomic_Int InitRefCount =
WOLFSSL_ATOMIC_INITIALIZER(0);
#define SNIFFER_INIT_REF_INC() \
wolfSSL_Atomic_Int_AddFetch(&InitRefCount, 1)
#define SNIFFER_INIT_REF_DEC() \
wolfSSL_Atomic_Int_SubFetch(&InitRefCount, 1)
#else
/* No atomics available. The count is then only reliable when the sniffer
* is initialized and freed from one thread at a time, so a threaded user
* on such a platform has to do its first ssl_InitSniffer*() before it
* creates the threads. */
static WC_THREADSHARED int InitRefCount = 0;
#define SNIFFER_INIT_REF_INC() (++InitRefCount)
#define SNIFFER_INIT_REF_DEC() (--InitRefCount)
#endif
#ifndef WOLFSSL_MUTEX_INITIALIZER
/* Set once the shared mutexes are usable, so that the callers which lost the
* race to initialize them do not return before they exist, and cleared again
* when the last caller frees them. Only ever read and written through
* WOLFSSL_ATOMIC_LOAD()/WOLFSSL_ATOMIC_STORE(): a plain int would let the
* compiler hoist the load out of the wait loop below, and the release/acquire
* pair is what publishes the setup to the callers that waited. Not needed when
* the platform can initialize the mutexes statically. */
#if defined(WOLFSSL_ATOMIC_OPS) && !defined(SINGLE_THREADED)
static WC_THREADSHARED wolfSSL_Atomic_Int SharedInitDone =
WOLFSSL_ATOMIC_INITIALIZER(0);
#else
static WC_THREADSHARED volatile int SharedInitDone = 0;
#endif
#endif
/* Take a reference to the shared state.
returns 1 when this caller is the one that has to set it up */
static int SnifferInitAcquire(void)
{
if (SNIFFER_INIT_REF_INC() == 1)
return 1;
#ifndef WOLFSSL_MUTEX_INITIALIZER
/* Another caller got there first; wait for it to finish. Startup only, and
* only on platforms without a static mutex initializer. */
while (!WOLFSSL_ATOMIC_LOAD(SharedInitDone)) {
WC_RELAX_LONG_LOOP();
}
#endif
return 0;
}
/* Drop a reference to the shared state.
returns 1 when this caller is the one that has to release it, 0 when others
still hold a reference, and -1 when there was no reference left to drop */
static int SnifferInitRelease(void)
{
int count = SNIFFER_INIT_REF_DEC();
if (count < 0) {
/* More frees than inits. Put the count back rather than letting it
run away: the shared state is already gone, and so are the locks
the caller would otherwise take on the way to it. */
(void)SNIFFER_INIT_REF_INC();
return -1;
}
return (count == 0);
}
/* windows uses .rc table for this */
#ifndef _WIN32
@ -699,23 +773,31 @@ static int addKeyLogSnifferServerHelper(const char* address,
void ssl_InitSniffer_ex(int devId)
{
wolfSSL_Init();
/* Only the first caller sets the shared state up, matching the release in
* ssl_FreeSniffer(): re-initializing a mutex another thread already holds
* is undefined, and the statistics belong to every thread at once. */
if (SnifferInitAcquire()) {
#ifndef WOLFSSL_MUTEX_INITIALIZER
#ifndef SNIFFER_LOCKLESS_TABLES
wc_InitMutex(&ServerListMutex);
wc_InitMutex(&SessionMutex);
wc_InitMutex(&ServerListMutex);
wc_InitMutex(&SessionMutex);
#endif
#ifndef WOLFSSL_SNIFFER_NO_RECOVERY
wc_InitMutex(&RecoveryMutex);
wc_InitMutex(&RecoveryMutex);
#endif
#ifdef WOLFSSL_SNIFFER_STATS
XMEMSET(&SnifferStats, 0, sizeof(SSLStats));
wc_InitMutex(&StatsMutex);
wc_InitMutex(&StatsMutex);
#endif
#endif /* !WOLFSSL_MUTEX_INITIALIZER */
#ifdef WOLFSSL_SNIFFER_STATS
XMEMSET(&SnifferStats, 0, sizeof(SSLStats));
XMEMSET(&SnifferStats, 0, sizeof(SSLStats));
#endif
#ifndef WOLFSSL_MUTEX_INITIALIZER
WOLFSSL_ATOMIC_STORE(SharedInitDone, 1);
#endif
}
#if defined(WOLF_CRYPTO_CB) || defined(WOLFSSL_ASYNC_CRYPT)
CryptoDeviceId = devId;
#endif
@ -887,6 +969,14 @@ void ssl_FreeSniffer(void)
SnifferSession* session;
SnifferSession* removeSession;
int i;
int releaseShared;
releaseShared = SnifferInitRelease();
if (releaseShared < 0) {
/* Nothing was left to release, and the locks below may not exist any
* more, so there is nothing safe to do here. */
return;
}
LOCK_SERVER_LIST();
LOCK_SESSION();
@ -921,35 +1011,50 @@ void ssl_FreeSniffer(void)
freeSecretList();
#endif /* WOLFSSL_SNIFFER_KEYLOGFILE */
/* What is left is shared by every thread that initialized the sniffer, so
* it may only be undone by the last one to get here. Tearing it down from
* a thread that finishes early closes the trace file and frees the mutexes
* under the threads still running. */
if (releaseShared) {
#ifndef WOLFSSL_MUTEX_INITIALIZER
#ifndef WOLFSSL_SNIFFER_NO_RECOVERY
wc_FreeMutex(&RecoveryMutex);
wc_FreeMutex(&RecoveryMutex);
#endif
#ifndef SNIFFER_LOCKLESS_TABLES
wc_FreeMutex(&SessionMutex);
wc_FreeMutex(&ServerListMutex);
wc_FreeMutex(&SessionMutex);
wc_FreeMutex(&ServerListMutex);
#endif
#ifdef WOLFSSL_SNIFFER_STATS
wc_FreeMutex(&StatsMutex);
#endif
/* The mutexes are gone, so a later ssl_InitSniffer*() has to build
them again. Leaving the flag set would let a caller that lost the
race to that re-initialization run on mutexes that do not exist
yet. */
WOLFSSL_ATOMIC_STORE(SharedInitDone, 0);
#endif /* !WOLFSSL_MUTEX_INITIALIZER */
#ifdef WOLF_CRYPTO_CB
#ifdef HAVE_INTEL_QA_SYNC
wc_CryptoCb_CleanupIntelQa(&CryptoDeviceId);
wc_CryptoCb_CleanupIntelQa(&CryptoDeviceId);
#endif
#ifdef HAVE_CAVIUM_OCTEON_SYNC
wc_CryptoCb_CleanupOcteon(&CryptoDeviceId);
wc_CryptoCb_CleanupOcteon(&CryptoDeviceId);
#endif
#endif
#ifdef WOLFSSL_ASYNC_CRYPT
wolfAsync_DevClose(&CryptoDeviceId);
wolfAsync_DevClose(&CryptoDeviceId);
#endif
if (TraceFile) {
TraceOn = 0;
XFCLOSE(TraceFile);
TraceFile = NULL;
if (TraceFile) {
TraceOn = 0;
XFCLOSE(TraceFile);
TraceFile = NULL;
}
}
/* Matches the wolfSSL_Init() in ssl_InitSniffer_ex(); it keeps its own
* count, so every caller has to come through here. */
wolfSSL_Cleanup();
}

10
sslSniffer/sslSnifferTest/snifftest.c
View File

@ -232,10 +232,11 @@ static void DumpStats(void)
static void sig_handler(const int sig)
{
printf("SIGINT handled = %d.\n", sig);
FreeAll();
#ifdef WOLFSSL_SNIFFER_STATS
/* Read the statistics while the sniffer still holds them. */
DumpStats();
#endif
FreeAll();
if (sig)
exit(EXIT_SUCCESS);
}
@ -1138,6 +1139,13 @@ int main(int argc, char** argv)
ssl_SetStoreDataCallback(myStoreDataCb);
#endif
#else
/* main() dispatches packets and calls FreeAll(), so it holds a reference
* of its own; each worker takes another one for itself. Matched with the
* release in FreeAll(), which Windows does from the DLL instead. */
#ifndef _WIN32
ssl_InitSniffer();
ssl_Trace(traceFile, err);
#endif
#ifdef HAVE_SESSION_TICKET
/* Multiple threads on resume not yet supported */
workerThreadCount = 1;