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
- Modify
fl_compositor_opengl_dispose and fl_compositor_software_dispose to acquire self->frame_mutex before clearing resources, synchronizing teardown with the raster thread.
- 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
Problem
When the
FlViewis disposed, the view'sFlCompositoris 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 aGWeakRefstrong reference to keep theFlViewinstance memory alive, it does not acquire a reference to theFlCompositoror lock the instance against disposal:flutter/engine/src/flutter/shell/platform/linux/fl_view_renderer.cc
Lines 220 to 235 in 990e440
As a result, while the raster thread runs
fl_compositor_opengl_present_layers(), the GTK thread can free theFlCompositorOpenGLorFlCompositorSoftwareinstance.Suggested fix
fl_compositor_opengl_disposeandfl_compositor_software_disposeto acquireself->frame_mutexbefore clearing resources, synchronizing teardown with the raster thread.fl_view_present_layersto take a strong reference to theself->compositorbefore passing it tofl_compositor_present_layers, and unref it afterpresent_layerscompletes, preventing the object from reaching a 0 refcount mid-execution.Potential bandaid fix...
Repro steps
Repro steps...
Repro patch...
Internal bug
b/525525829