Skip to content

[Linux] Potential crash when the view is disposed mid present #188659

Description

@loic-sharma

Problem

When the FlView is disposed, the view's FlCompositor is freed.

However, the engine's raster thread may be actively presenting frames onto the screen via compositor_present_view_callback(). While the raster thread correctly obtains a GWeakRef strong reference to keep the FlView instance memory alive, it does not acquire a reference to the FlCompositor or lock the instance against disposal:

void fl_view_renderer_present_layers(FlViewRenderer* self,
const FlutterLayer** layers,
size_t layers_count) {
g_return_if_fail(FL_IS_VIEW_RENDERER(self));
// Frames may be presented before the widget is realized and the compositor
// is set up; ignore them.
if (self->compositor == nullptr) {
return;
}
fl_compositor_present_layers(self->compositor, layers, layers_count);
// Perform the redraw in the GTK thread.
g_idle_add(redraw_cb, g_object_ref(self));
}

As a result, while the raster thread runs fl_compositor_opengl_present_layers(), the GTK thread can free the FlCompositorOpenGL or FlCompositorSoftware instance.

Suggested fix

  1. Modify fl_compositor_opengl_dispose and fl_compositor_software_dispose to acquire self->frame_mutex before clearing resources, synchronizing teardown with the raster thread.
  2. modify fl_view_present_layers to take a strong reference to the self->compositor before passing it to fl_compositor_present_layers, and unref it after present_layers completes, preventing the object from reaching a 0 refcount mid-execution.
Potential bandaid fix...

diff --git a/engine/src/flutter/shell/platform/linux/fl_view.cc b/engine/src/flutter/shell/platform/linux/fl_view.cc
--- a/engine/src/flutter/shell/platform/linux/fl_view.cc
+++ b/engine/src/flutter/shell/platform/linux/fl_view.cc
@@ -291,9 +291,18 @@ static void fl_view_present_layers(FlRenderable* renderable,
                                    const FlutterLayer** layers,
                                    size_t layers_count) {
   FlView* self = FL_VIEW(renderable);

-  fl_compositor_present_layers(self->compositor, layers, layers_count);
+  // self->compositor is owned solely by this FlView and is dropped in
+  // fl_view_dispose() on the GTK thread, which can run concurrently with this
+  // raster-thread callback (gtk_widget_destroy -> g_object_run_dispose runs
+  // regardless of the strong ref the raster thread holds on |self|). Take a
+  // local strong reference so the compositor cannot be finalized mid-present.
+  g_autoptr(FlCompositor) compositor = self->compositor != nullptr
+      ? FL_COMPOSITOR(g_object_ref(self->compositor))
+      : nullptr;
+  if (compositor != nullptr) {
+    fl_compositor_present_layers(compositor, layers, layers_count);
+  }

   // Perform the redraw in the GTK thead.
   g_idle_add(redraw_cb, g_object_ref(self));
 }
diff --git a/engine/src/flutter/shell/platform/linux/fl_compositor_software.cc b/engine/src/flutter/shell/platform/linux/fl_compositor_software.cc
--- a/engine/src/flutter/shell/platform/linux/fl_compositor_software.cc
+++ b/engine/src/flutter/shell/platform/linux/fl_compositor_software.cc
@@ -131,12 +131,16 @@ static gboolean fl_compositor_software_render(FlCompositor* compositor,
 static void fl_compositor_software_dispose(GObject* object) {
   FlCompositorSoftware* self = FL_COMPOSITOR_SOFTWARE(object);

+  // Wait for any in-flight present_layers() (raster thread) to complete before
+  // tearing down state it is using.
+  g_mutex_lock(&self->frame_mutex);
   g_clear_object(&self->task_runner);
   if (self->surface != nullptr) {
     g_free(cairo_image_surface_get_data(self->surface));
   }
   g_clear_pointer(&self->surface, cairo_surface_destroy);
-  g_mutex_clear(&self->frame_mutex);
+  g_mutex_unlock(&self->frame_mutex);
+  // Intentionally do not g_mutex_clear() — not required on the Linux futex
+  // impl and unsafe if the raster thread re-enters.

   G_OBJECT_CLASS(fl_compositor_software_parent_class)->dispose(object);
 }
diff --git a/engine/src/flutter/shell/platform/linux/fl_compositor_opengl.cc b/engine/src/flutter/shell/platform/linux/fl_compositor_opengl.cc
--- a/engine/src/flutter/shell/platform/linux/fl_compositor_opengl.cc
+++ b/engine/src/flutter/shell/platform/linux/fl_compositor_opengl.cc
@@ -456,13 +456,17 @@ static gboolean fl_compositor_opengl_render(FlCompositor* compositor,
 static void fl_compositor_opengl_dispose(GObject* object) {
   FlCompositorOpenGL* self = FL_COMPOSITOR_OPENGL(object);

+  // Wait for any in-flight present_layers() (raster thread) to complete before
+  // tearing down state it is using.
+  g_mutex_lock(&self->frame_mutex);
   cleanup_shader(self);

   g_clear_object(&self->task_runner);
   g_clear_object(&self->opengl_manager);
   g_clear_object(&self->framebuffer);
   g_clear_pointer(&self->pixels, g_free);
-  g_mutex_clear(&self->frame_mutex);
+  g_mutex_unlock(&self->frame_mutex);
+  // Intentionally do not g_mutex_clear() — see fl_compositor_software_dispose.

   G_OBJECT_CLASS(fl_compositor_opengl_parent_class)->dispose(object);
 }

Repro steps

Repro steps...

cd "$FLUTTER_ROOT"
git apply repro.patch

cd engine/src
./flutter/tools/gn --runtime-mode=debug --unoptimized --asan --lsan --no-goma --no-rbe
ninja -C out/host_debug_unopt flutter_linux_unittests

# G_SLICE=always-malloc is required so GObject instances go through
# malloc/free; otherwise GLib 2.74's slab allocator hides the freed instance
# from ASAN's shadow.

# Interleaving 1: vtable dispatch on freed FlCompositor instance
G_SLICE=always-malloc \
ASAN_OPTIONS=detect_leaks=0 \
ASAN_SYMBOLIZER_PATH=$PWD/flutter/buildtools/linux-x64/clang/bin/llvm-symbolizer \
dbus-run-session -- xvfb-run -a \
  ./out/host_debug_unopt/exe.unstripped/flutter_linux_unittests \
  --gtest_filter='FlCompositorSoftwareTest.DisposeRacePresentUAF'

# Interleaving 2: dispose clears frame_mutex while raster thread holds it
G_SLICE=always-malloc \
ASAN_OPTIONS=detect_leaks=0:handle_abort=1 \
dbus-run-session -- xvfb-run -a \
  ./out/host_debug_unopt/exe.unstripped/flutter_linux_unittests \
  --gtest_filter='FlCompositorSoftwareTest.DisposeDuringPresentLockedMutex'

Repro patch...
// POC: heap-use-after-free when FlCompositor is disposed on the GTK thread
// while the engine raster thread is presenting to it.
//
// Models the production interleaving at fl_view.cc:291-300 / fl_view.cc:631:
//
//   raster thread: compositor_present_view_callback (fl_engine.cc:394)
//                    g_weak_ref_get(view)            -- strong ref on FlView
//                    fl_view_present_layers (fl_view.cc:294)
//                      FlCompositor* c = self->compositor;   -- raw read, NO ref
//                      ----------------------- preemption -----------------------
//   GTK thread:    gtk_widget_destroy(secondary_window)
//                    g_object_run_dispose(view)       -- runs despite raster ref
//                      fl_view_dispose (fl_view.cc:598)
//                        g_clear_object(&self->compositor)  -- sole ref -> 0
//                          fl_compositor_*_dispose + finalize + g_free(instance)
//                      ----------------------- resume ---------------------------
//   raster thread:     fl_compositor_present_layers(c, ...)  -- c is FREED
//                        FL_IS_COMPOSITOR(c)                 -- UAF read
//                        FL_COMPOSITOR_GET_CLASS(c)->present_layers(c, ...)
//                                                            -- vtable call via
//                                                               freed g_class
//
// fl_engine_remove_view() (called earlier in fl_view_dispose) only removes the
// weak-ref hash entry and posts an async RemoveView; it does NOT fence against
// a present that has already passed get_renderable().
TEST(FlCompositorSoftwareTest, DisposeRacePresentUAF) {
  g_autoptr(FlDartProject) project = fl_dart_project_new();
  g_autoptr(FlEngine) engine = fl_engine_new(project);
  g_autoptr(FlTaskRunner) task_runner = fl_task_runner_new(engine);

  // FlView is the *sole* owner of its compositor (fl_view.cc:499-506 / :631).
  // Model that here by holding exactly one reference.
  FlCompositor* compositor =
      FL_COMPOSITOR(fl_compositor_software_new(task_runner));

  constexpr size_t width = 64;
  constexpr size_t height = 64;
  size_t row_bytes = width * 4;
  unsigned char* layer_data =
      static_cast<unsigned char*>(g_malloc0(height * row_bytes));
  FlutterBackingStore backing_store = {
      .type = kFlutterBackingStoreTypeSoftware,
      .software = {
          .allocation = layer_data, .row_bytes = row_bytes, .height = height}};
  FlutterLayer layer = {.type = kFlutterLayerContentTypeBackingStore,
                        .backing_store = &backing_store,
                        .offset = {0, 0},
                        .size = {width, height}};
  const FlutterLayer* layers[1] = {&layer};

  fml::ManualResetWaitableEvent raster_loaded_compositor;
  fml::ManualResetWaitableEvent gtk_disposed_view;

  // Simulated engine raster thread: it has obtained a strong ref on the FlView
  // via g_weak_ref_get() and is now in fl_view_present_layers(), having just
  // read the raw `self->compositor` field with no g_object_ref().
  std::thread raster([&]() {
    FlCompositor* c = compositor;  // raw read, as at fl_view.cc:296
    raster_loaded_compositor.Signal();
    gtk_disposed_view.Wait();
    // c now points to a freed FlCompositorSoftware instance.
    fl_compositor_present_layers(c, layers, 1);
  });

  // Simulated GTK thread: gtk_widget_destroy() on the secondary window runs
  // g_object_run_dispose() on the FlView even though the raster thread holds a
  // strong ref. fl_view_dispose() then drops the only compositor reference.
  raster_loaded_compositor.Wait();
  g_object_unref(compositor);  // == g_clear_object(&view->compositor)
  gtk_disposed_view.Signal();

  raster.join();
  g_free(layer_data);
}

// Second interleaving: raster thread is *inside* present_layers() holding
// frame_mutex when the GTK thread disposes the compositor.
// fl_compositor_software_dispose() calls g_mutex_clear() on the locked mutex
// (UB per GLib docs) and then frees the instance underneath the raster thread.
// On current GLib this manifests as a hard abort from g_mutex_clear(); without
// that check the raster thread would proceed to write self->surface and read
// self->task_runner on freed heap.
TEST(FlCompositorSoftwareTest, DisposeDuringPresentLockedMutex) {
  g_autoptr(FlDartProject) project = fl_dart_project_new();
  g_autoptr(FlEngine) engine = fl_engine_new(project);
  g_autoptr(FlTaskRunner) task_runner = fl_task_runner_new(engine);

  // FlView is the *sole* owner of its compositor (fl_view.cc:499/631). Model
  // that here by holding exactly one reference.
  FlCompositorSoftware* compositor = fl_compositor_software_new(task_runner);

  // Large layer so the memcpy() inside fl_compositor_software_present_layers()
  // (fl_compositor_software.cc:59) takes measurable wall-clock time, widening
  // the race window — the analogue of a large rendered frame on the real
  // raster thread (and of the glReadPixels host-memory copy on X11 in the
  // OpenGL compositor).
  constexpr size_t width = 4096;
  constexpr size_t height = 4096;  // 64 MiB
  size_t row_bytes = width * 4;
  size_t allocation_length = height * row_bytes;
  unsigned char* layer_data =
      static_cast<unsigned char*>(g_malloc0(allocation_length));
  FlutterBackingStore backing_store = {
      .type = kFlutterBackingStoreTypeSoftware,
      .software = {
          .allocation = layer_data, .row_bytes = row_bytes, .height = height}};
  FlutterLayer layer = {.type = kFlutterLayerContentTypeBackingStore,
                        .backing_store = &backing_store,
                        .offset = {0, 0},
                        .size = {width, height}};
  const FlutterLayer* layers[1] = {&layer};

  std::atomic<bool> entered{false};

  // Simulated engine raster thread.
  std::thread raster([&]() {
    entered.store(true);
    fl_compositor_present_layers(FL_COMPOSITOR(compositor), layers, 1);
  });

  // Wait until the raster thread is about to enter present_layers, then give
  // it a moment to acquire frame_mutex and begin the large memcpy.
  while (!entered.load()) {
    std::this_thread::yield();
  }
  g_usleep(2000);  // 2 ms — well inside the 64 MiB memcpy window under ASAN.

  // Simulated GTK thread: fl_view_dispose() drops the only compositor ref
  // while the raster thread is still inside present_layers(). This runs
  // fl_compositor_software_dispose() — which clears task_runner, frees the
  // surface buffer, and g_mutex_clear()s the *locked* frame_mutex — and then
  // finalizes/frees the instance.
  g_object_unref(compositor);

  // Raster thread now resumes on freed memory:
  //   - cairo_surface_destroy(self->surface)       UAF read of self->surface
  //   - self->surface = ...                        UAF write
  //   - fl_task_runner_stop_wait(self->task_runner) UAF read
  //   - g_mutex_unlock(&self->frame_mutex)         UAF (mutex in freed object)
  raster.join();

  g_free(layer_data);
}

Internal bug

b/525525829

Metadata

Metadata

Assignees

No one assigned

    Labels

    c: fatal crashCrashes that terminate the processplatform-linuxBuilding on or for Linux specificallyteam-linuxOwned by the Linux platform team

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions