Skip to content

Fix memory leaks in SocketMbedTLS when closing. - #527

Merged
bsergean merged 1 commit into
machinezone:masterfrom
plmzod:master
Sep 27, 2024
Merged

bsergean merged 1 commit into
machinezone:masterfrom
plmzod:master

Conversation

@plmzod

@plmzod plmzod commented Sep 3, 2024

Copy link
Copy Markdown
Contributor

Hello (and thanks for this library!), I've found two leaks when using sockets with SSL/TLS and MbedTLS.
One leak about the key, and one from the new PSA layer introduced with MbedTLS 3.6.0.
The leaks are detected on Windows with VLD and on Linux with ASAN.

@bsergean

Copy link
Copy Markdown
Collaborator

Thanks for the fixes !

@bsergean
bsergean merged commit d0fa8c7 into machinezone:master Sep 27, 2024
bsergean pushed a commit that referenced this pull request Oct 14, 2026
…612) (#613)

* Add regression test for process-global PSA state surviving a socket close

Closing one SocketMbedTLS must not invalidate PSA crypto state (RNG, key
slots) used by other live sockets in the same process.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

* Release PSA crypto state in uninitNetSystem(), not per socket (fix #612)

SocketMbedTLS::close() called mbedtls_psa_crypto_free() for every socket.
That function is process-global: it wipes all PSA key slots and the
shared entropy/RNG state, so closing one TLS socket broke every other
live TLS socket in the process. With several concurrent TLS WebSocket
clients reconnecting on their own threads this showed up as
PSA_ERROR_BAD_STATE failures and heap-corruption aborts.

- close() no longer calls mbedtls_psa_crypto_free().
- uninitNetSystem() now calls mbedtls_psa_crypto_free() on all
  platforms when built with mbedTLS >= 3.6, mirroring the existing
  WSAStartup/WSACleanup pairing. It must be called only after all
  sockets are closed.
- initMBedTLS() is guarded by a per-socket flag so the ctor + init()
  double call no longer re-initialises live mbedTLS contexts (which
  re-inits mutexes under MBEDTLS_THREADING_C). close() clears the flag
  so a socket can be re-initialised.
- init() checks psa_crypto_init() and fails the connect with an error
  message instead of running TLS on uninitialised PSA state.

Affected releases: v11.4.6, v12.0.0, v12.0.1 and master (introduced in
#527, widened to mbedTLS 4.x in #579), only when built against
mbedTLS >= 3.6. v11.4.5 and earlier are not affected.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

* Guard the PSA include for pre-3.6 mbedTLS and keep the PSA test binary non-empty

- IXNetSystem.cpp: include <psa/crypto.h> only for mbedTLS >= 3.6, so
  builds against mbedTLS 2.x (no psa/ headers) still compile.
- The PSA lifetime test compiles to a single passing placeholder case
  below mbedTLS 3.6 instead of an empty binary, which Catch2 reports as
  "no tests ran" (exit 2) and would fail ctest for a supported config.
- Document in usage.md that uninitNetSystem() now also releases
  process-global TLS (PSA) state and must follow the last socket close.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

---------

Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants