Merge pull request #1950 from bugsnag/PLAT-11264/fix_setStaticJsonData

Avoid possible double-free in `setStaticJsonData`
bugsnag · Dec 4, 2023 · dea908f · dea908f
2 parents 76802e8 + 75234b5
commit dea908f
Show file tree

Hide file tree

Showing 4 changed files with 17 additions and 8 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -11,6 +11,8 @@
 
 * Updating existing feature flags no longer causes them to change location.
   [#1940](https://github.com/bugsnag/bugsnag-android/pull/1940)
+* Fixed possible NDK crash when constructing several concurrent `Client` instances
+  []()
 
 
 ## 6.0.0 (2023-11-20)

diff --git a/bugsnag-plugin-android-ndk/src/main/jni/bugsnag_ndk.c b/bugsnag-plugin-android-ndk/src/main/jni/bugsnag_ndk.c
@@ -217,6 +217,8 @@ JNIEXPORT void JNICALL Java_com_bugsnag_android_ndk_NativeBridge_install(
   bugsnag_env->next_event.feature_flag_count = 0;
   bugsnag_env->next_event.feature_flags = NULL;
 
+  atomic_init(&bugsnag_env->static_json_data, NULL);
+
   bsg_global_env = bugsnag_env;
   bsg_update_next_run_info(bsg_global_env);
   BUGSNAG_LOG("Initialization complete!");
@@ -912,13 +914,18 @@ Java_com_bugsnag_android_ndk_NativeBridge_setStaticJsonData(JNIEnv *env,
     return;
   }
 
-  size_t length = strlen(data);
-  if (length == 0) {
+  // strlen(data) == 0
+  if (*data == 0) {
+    goto done;
+  }
+
+  const char *new_data = strdup(data);
+  if (!new_data) {
     goto done;
   }
 
-  const char *data_old = bsg_global_env->static_json_data;
-  bsg_global_env->static_json_data = strdup(data);
+  const char *data_old =
+      atomic_exchange(&bsg_global_env->static_json_data, new_data);
   free((void *)data_old);
 
 done:

diff --git a/bugsnag-plugin-android-ndk/src/main/jni/bugsnag_ndk.h b/bugsnag-plugin-android-ndk/src/main/jni/bugsnag_ndk.h
@@ -81,7 +81,7 @@ typedef struct {
    * On delivery, this data gets sent to the JVM alongside of the JSONified
    * event object.
    */
-  const char *static_json_data;
+  const char *_Atomic static_json_data;
 
 } bsg_environment;
 

diff --git a/bugsnag-plugin-android-ndk/src/main/jni/utils/serializer/event_writer.c b/bugsnag-plugin-android-ndk/src/main/jni/utils/serializer/event_writer.c
@@ -37,12 +37,12 @@ bool bsg_event_write(bsg_environment *env) {
 
   writer.dispose(&writer);
 
-  if (result && env->static_json_data != NULL) {
+  const char *static_json_data = atomic_load(&env->static_json_data);
+  if (result && static_json_data != NULL) {
     // Attempt to write the static data, but don't worry if it fails.
     // We'll check for truncated/missing static data on load.
     if (bsg_buffered_writer_open(&writer, env->next_event_static_data_path)) {
-      writer.write(&writer, env->static_json_data,
-                   strlen(env->static_json_data));
+      writer.write(&writer, static_json_data, strlen(static_json_data));
       writer.dispose(&writer);
     }
   }