From 8664d9a9635b0ffc61f7480fa0566cd950d8d59a Mon Sep 17 00:00:00 2001 From: Alex Date: Sat, 4 Jul 2026 12:56:57 +0200 Subject: [PATCH] fix(wireplumber): guard async load callbacks against use-after-free on teardown The WirePlumber module registers three async callbacks (onDefaultNodesApiLoaded, onMixerApiLoaded, onPluginActivated) that receive a raw self pointer with a NULL GCancellable. WirePlumber cannot withdraw an in-flight callback, so if the module is destroyed before a queued callback fires (e.g. a temporary output/bar is removed while a component load is still pending, or during an audio route transition), the callback dereferences the freed self, causing heap corruption / a crash. Guard each of these callbacks with isModuleAlive(), which checks the existing static modules registry. The destructor already removes this from the registry before any teardown, so a missing entry means self is dangling and the callback bails out without touching it. A GCancellable cannot fix this cleanly here: every callback dereferences self on its first line, and wp_core_load_component completes via a WpTransition (not a GTask), so the cancellable is not recoverable from the GAsyncResult either. The liveness check must not touch self at all. Fixes #3974. --- include/modules/wireplumber.hpp | 3 +++ src/modules/wireplumber.cpp | 23 +++++++++++++++++++++++ 2 files changed, 26 insertions(+) diff --git a/include/modules/wireplumber.hpp b/include/modules/wireplumber.hpp index 66900501..f1650e92 100644 --- a/include/modules/wireplumber.hpp +++ b/include/modules/wireplumber.hpp @@ -36,6 +36,9 @@ class Wireplumber : public ALabel { std::vector getWPIcon(); static std::list modules; + // Returns true while `self` is still a live module. Async load/activation callbacks use this to + // avoid dereferencing a `self` that was destroyed before the callback fired (see #3974). + static bool isModuleAlive(waybar::modules::Wireplumber* self); uint32_t resolvePhysicalSink(uint32_t start_id); uint32_t findPlaybackNodeId(const gchar* description); diff --git a/src/modules/wireplumber.cpp b/src/modules/wireplumber.cpp index b3167381..a2391071 100644 --- a/src/modules/wireplumber.cpp +++ b/src/modules/wireplumber.cpp @@ -10,6 +10,17 @@ bool isValidNodeId(uint32_t id) { return id > 0 && id < G_MAXUINT32; } std::list waybar::modules::Wireplumber::modules; +// Async load/activation callbacks (onDefaultNodesApiLoaded, onMixerApiLoaded, onPluginActivated) +// are handed a raw `self` pointer with no GCancellable, and WirePlumber has no way to withdraw an +// in-flight callback. If the module is destroyed before such a callback fires (e.g. an output/bar +// is removed while a component load is still pending, or during an audio route transition), the +// callback would dereference a freed `self`. The destructor removes `this` from this registry +// before any teardown, so a missing entry means `self` is dangling and the callback must bail out +// without touching it. See https://github.com/Alexays/Waybar/issues/3974. +bool waybar::modules::Wireplumber::isModuleAlive(waybar::modules::Wireplumber* self) { + return std::find(modules.begin(), modules.end(), self) != modules.end(); +} + waybar::modules::Wireplumber::Wireplumber(const std::string& id, const Json::Value& config) : ALabel(config, "wireplumber", id, "{volume}%"), wp_core_(nullptr), @@ -387,6 +398,10 @@ void waybar::modules::Wireplumber::onObjectManagerInstalled(waybar::modules::Wir void waybar::modules::Wireplumber::onPluginActivated(WpObject* p, GAsyncResult* res, waybar::modules::Wireplumber* self) { + if (!isModuleAlive(self)) { + return; + } + const auto* pluginName = wp_plugin_get_name(WP_PLUGIN(p)); spdlog::debug("[{}]: onPluginActivated: {}", self->name_, pluginName); g_autoptr(GError) error = nullptr; @@ -432,6 +447,10 @@ void waybar::modules::Wireplumber::prepare(waybar::modules::Wireplumber* self) { void waybar::modules::Wireplumber::onDefaultNodesApiLoaded(WpObject* p, GAsyncResult* res, waybar::modules::Wireplumber* self) { + if (!isModuleAlive(self)) { + return; + } + gboolean success = FALSE; g_autoptr(GError) error = nullptr; @@ -453,6 +472,10 @@ void waybar::modules::Wireplumber::onDefaultNodesApiLoaded(WpObject* p, GAsyncRe void waybar::modules::Wireplumber::onMixerApiLoaded(WpObject* p, GAsyncResult* res, waybar::modules::Wireplumber* self) { + if (!isModuleAlive(self)) { + return; + } + gboolean success = FALSE; g_autoptr(GError) error = nullptr;