Skip to content

Commit 49416f5

Browse files
committed
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.
1 parent 5180dec commit 49416f5

2 files changed

Lines changed: 26 additions & 0 deletions

File tree

include/modules/wireplumber.hpp

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,9 @@ class Wireplumber : public ALabel {
3636
std::vector<std::string> getWPIcon();
3737

3838
static std::list<waybar::modules::Wireplumber*> modules;
39+
// Returns true while `self` is still a live module. Async load/activation callbacks use this to
40+
// avoid dereferencing a `self` that was destroyed before the callback fired (see #3974).
41+
static bool isModuleAlive(waybar::modules::Wireplumber* self);
3942

4043
uint32_t resolvePhysicalSink(uint32_t start_id);
4144
uint32_t findPlaybackNodeId(const gchar* description);

src/modules/wireplumber.cpp

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,17 @@ bool isValidNodeId(uint32_t id) { return id > 0 && id < G_MAXUINT32; }
1010

1111
std::list<waybar::modules::Wireplumber*> waybar::modules::Wireplumber::modules;
1212

13+
// Async load/activation callbacks (onDefaultNodesApiLoaded, onMixerApiLoaded, onPluginActivated)
14+
// are handed a raw `self` pointer with no GCancellable, and WirePlumber has no way to withdraw an
15+
// in-flight callback. If the module is destroyed before such a callback fires (e.g. an output/bar
16+
// is removed while a component load is still pending, or during an audio route transition), the
17+
// callback would dereference a freed `self`. The destructor removes `this` from this registry
18+
// before any teardown, so a missing entry means `self` is dangling and the callback must bail out
19+
// without touching it. See https://github.com/Alexays/Waybar/issues/3974.
20+
bool waybar::modules::Wireplumber::isModuleAlive(waybar::modules::Wireplumber* self) {
21+
return std::find(modules.begin(), modules.end(), self) != modules.end();
22+
}
23+
1324
waybar::modules::Wireplumber::Wireplumber(const std::string& id, const Json::Value& config)
1425
: ALabel(config, "wireplumber", id, "{volume}%"),
1526
wp_core_(nullptr),
@@ -387,6 +398,10 @@ void waybar::modules::Wireplumber::onObjectManagerInstalled(waybar::modules::Wir
387398

388399
void waybar::modules::Wireplumber::onPluginActivated(WpObject* p, GAsyncResult* res,
389400
waybar::modules::Wireplumber* self) {
401+
if (!isModuleAlive(self)) {
402+
return;
403+
}
404+
390405
const auto* pluginName = wp_plugin_get_name(WP_PLUGIN(p));
391406
spdlog::debug("[{}]: onPluginActivated: {}", self->name_, pluginName);
392407
g_autoptr(GError) error = nullptr;
@@ -432,6 +447,10 @@ void waybar::modules::Wireplumber::prepare(waybar::modules::Wireplumber* self) {
432447

433448
void waybar::modules::Wireplumber::onDefaultNodesApiLoaded(WpObject* p, GAsyncResult* res,
434449
waybar::modules::Wireplumber* self) {
450+
if (!isModuleAlive(self)) {
451+
return;
452+
}
453+
435454
gboolean success = FALSE;
436455
g_autoptr(GError) error = nullptr;
437456

@@ -453,6 +472,10 @@ void waybar::modules::Wireplumber::onDefaultNodesApiLoaded(WpObject* p, GAsyncRe
453472

454473
void waybar::modules::Wireplumber::onMixerApiLoaded(WpObject* p, GAsyncResult* res,
455474
waybar::modules::Wireplumber* self) {
475+
if (!isModuleAlive(self)) {
476+
return;
477+
}
478+
456479
gboolean success = FALSE;
457480
g_autoptr(GError) error = nullptr;
458481

0 commit comments

Comments
 (0)