Coverity reports a data race condition where call_hooks() accesses
h->hook_cb without holding the mutex (as is done elsewhere 1 out of 1
times when writing to hook_cb).
The function fatal_signal_add_hook() writes to hooks[] with the mutex
held, but fatal_signal_atexit_handler() calls call_hooks() without
acquiring the mutex. While this is unlikely to cause issues in practice
(hooks are registered during initialization and the atexit handler runs
single-threaded at exit), there's no memory barrier guaranteeing
visibility of hook data.
Add mutex locking to fatal_signal_atexit_handler() and add OVS_REQUIRES
annotations to call_hooks() to enforce proper locking. Since the mutex
is recursive, this is safe even if called from signal context.
Fixes: b847adc62006 ("fatal-signal: Make thread-safe.")
Signed-off-by: Eelco Chaudron <[email protected]>
---
lib/fatal-signal.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/lib/fatal-signal.c b/lib/fatal-signal.c
index ff9c021db..83a7bbe93 100644
--- a/lib/fatal-signal.c
+++ b/lib/fatal-signal.c
@@ -77,7 +77,8 @@ static HANDLE wevent;
static struct ovs_mutex mutex;
-static void call_hooks(int sig_nr);
+static void call_hooks(int sig_nr)
+ OVS_REQUIRES(mutex);
#ifdef _WIN32
static BOOL WINAPI ConsoleHandlerRoutine(DWORD dwCtrlType);
#endif
@@ -404,11 +405,14 @@ fatal_ignore_sigpipe(void)
void
fatal_signal_atexit_handler(void)
{
+ ovs_mutex_lock(&mutex);
call_hooks(0);
+ ovs_mutex_unlock(&mutex);
}
static void
call_hooks(int sig_nr)
+ OVS_REQUIRES(mutex)
{
static volatile sig_atomic_t recurse = 0;
if (!recurse) {
--
2.52.0
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
