jedisct1 / libsodium

A modern, portable, easy to use crypto library.
https://libsodium.org
Other
12.08k stars 1.72k forks source link

[Question] How to properly clean up the critical section created by sodium_init()? #1050

Closed TSchick90 closed 3 years ago

TSchick90 commented 3 years ago

Hey guys!

As the title says. I have some code checks in place which tell me about the critical section which is created during the sodium_init() but never really destroyed? Is there some sodium_cleanup() function or something I'm missing?

I would expect something like this in the core.c

void sodium_crit_destroy()
{
   // if initialized, check _sodium_lock_initialized
   DestroyCriticalSection(&_sodium_lock);
} 

Stay save and best, Thomas

jedisct1 commented 3 years ago

There's nothing to do. sodium_init() doesn't do any heap allocations.

jedisct1 commented 3 years ago

Ah, I see what you are referring to. On windows, an InitializeCriticalSection() section is called by sodium_init(). There's no sodium_deinit() function, so that value will not be deallocated. Not a big deal since it's only initialized once.

jedisct1 commented 3 years ago

If that worries you, the following diff should destroy the critical section on release.

--- a/src/libsodium/sodium/core.c
+++ b/src/libsodium/sodium/core.c
@@ -100,8 +100,15 @@ sodium_crit_leave(void)
 # endif
         return -1;
     }
+    while (InterlockedCompareExchange(&_sodium_lock_initialized,
+                                      1L, 2L) == 1L) {
+        Sleep(0);
+    }
     locked = 0;
     LeaveCriticalSection(&_sodium_lock);
+    DeleteCriticalSection(&_sodium_lock);
+    _sodium_lock = NULL;
+    InterlockedAnd(&_sodium_lock_initialized, 0L);

     return 0;
 }

Completely untested, and probably not something that's going to be merged.

TSchick90 commented 3 years ago

Ah yes, that I saw this on Windows would have been a useful information ... anyway, thanks for your answers :)