I discovered this while trying to call sentry_init, sentry_shutdown and sentry_capture_event multiple times from multiple threads at the same time during unit tests, not a valid use-case scenario of this library.
But I can imagine a valid use case to be when sentry_shutdown is called while other threads are still busy using sentry_capture_event.
Basically what happens is that functions like sentry_capture_event do this:
|
const sentry_options_t *opts = sentry_get_options(); |
They check if it's valid here:
And then use it multiple times here:
|
if (opts->before_send_func) { |
|
event = opts->before_send_func(event, NULL, opts->before_send_data); |
|
} |
|
if (opts->transport && !sentry_value_is_null(event)) { |
The issue arises when on a different thread sentry_shutdown is called which invalidates g_options and makes access to it access freed memory.
So the missing part here to me seems a missing lock around g_options_mutex.
Another example is sentry_start_session:
|
void |
|
sentry_start_session(void) |
|
{ |
|
sentry_end_session(); |
|
SENTRY_WITH_SCOPE_MUT (scope) { |
|
scope->session = sentry__session_new(); |
|
sentry__scope_session_sync(scope); |
|
} |
|
} |
So
SENTRY_WITH_SCOPE_MUT locks
g_lock:
|
sentry_scope_t * |
|
sentry__scope_lock(void) |
|
{ |
|
sentry__mutex_lock(&g_lock); |
|
return get_scope(); |
|
} |
But
sentry_shutdown locks a different mutex,
g_options_mutex:
|
sentry__mutex_lock(&g_options_mutex); |
|
sentry_options_free(options); |
|
g_options = NULL; |
|
sentry__mutex_unlock(&g_options_mutex); |
Which doesn't block
sentry_start_session from using already freed memory:
|
char *release = sentry__string_clone(sentry_options_get_release(opts)); |
|
char *environment |
|
= sentry__string_clone(sentry_options_get_environment(opts)); |
There are more offenders similar to those.
Is there an intention to account for this scenario?
I discovered this while trying to call
sentry_init,sentry_shutdownandsentry_capture_eventmultiple times from multiple threads at the same time during unit tests, not a valid use-case scenario of this library.But I can imagine a valid use case to be when
sentry_shutdownis called while other threads are still busy usingsentry_capture_event.Basically what happens is that functions like
sentry_capture_eventdo this:sentry-native/src/sentry_core.c
Line 257 in 9651561
They check if it's valid here:
sentry-native/src/sentry_core.c
Line 258 in 9651561
And then use it multiple times here:
sentry-native/src/sentry_core.c
Lines 282 to 285 in 9651561
The issue arises when on a different thread
sentry_shutdownis called which invalidatesg_optionsand makes access to it access freed memory.So the missing part here to me seems a missing lock around
g_options_mutex.Another example is
sentry_start_session:sentry-native/src/sentry_session.c
Lines 206 to 214 in 9651561
So
SENTRY_WITH_SCOPE_MUTlocksg_lock:sentry-native/src/sentry_scope.c
Lines 85 to 90 in 9651561
But
sentry_shutdownlocks a different mutex,g_options_mutex:sentry-native/src/sentry_core.c
Lines 142 to 145 in 9651561
Which doesn't block
sentry_start_sessionfrom using already freed memory:sentry-native/src/sentry_session.c
Line 54 in 9651561
sentry-native/src/sentry_session.c
Lines 64 to 65 in 9651561
There are more offenders similar to those.
Is there an intention to account for this scenario?