Quick update on the systray crash fix.
I reached out to @korbel.jak, maintainer of tint2-patched-git on AUR, to ask whether his package includes the iterator UAF fix for on_change_systray().
His reply: yes, it does — but implemented differently. His version restarts iteration from the beginning when a deletion occurs. He noted our fix (capturing l->next before any potential free) is more efficient, since it just continues forward without restarting.
So the good news: tint2-patched-git users are protected from the Steam/Electron SIGSEGV. The approach in the Mabox patched branch is technically the cleaner solution.
Not sure what the next step is from here — submitting the patch upstream to o9000/tint2 would be the logical move.
Snap from AUR…
@musdus
Yes, it does, but differently.
The issue here is that the original code deleted from the list it iterated on at the same time.
Now the iteration stops on deletion and goes from the beginning when that happens. Your fix is more efficient.
Hi @korbel.jak
Does this package include the systray iterator UAF fix? It addresses a SIGSEGV in on_change_systray() triggered by Steam, Electron apps (Vesktop/Discord) and similar tray clients that destroy their window before the XEMBED handshake completes.
From: zackattackz
Subject: [PATCH] systray: fix iterator UAF in on_change_systray
Patch adapted for original tint2 (o9000/tint2) by danieln@maboxlinux.org
on_change_systray() iterates systray.list_icons and calls reparent_icon()
for each icon not yet reparented. If the XEMBED handshake fails,
reparent_icon() calls remove_icon(), which g_slist_remove()s and g_free()s
the current node. The loop then advances via l = l->next on the freed node,
and the next iteration dereferences l->data on garbage memory (SIGSEGV).
Trigger: any tray client that requests dock then destroys its window before
the embed completes — e.g. Steam's transient 200x200 loader icon, Electron
apps (Vesktop/Discord) that tear down their GdkWindow mid-embed.
Fix: capture l->next at the top of the loop body, before any call that
might free the current node. remove_icon() only frees the current node, so
the pre-captured next pointer stays valid.
This is the same iterator-invalidation pattern fixed in uevent_handler() by
the glib2.76 patch (issue #4). That fix addressed one occurrence; this is
the missed sibling in systraybar.c.
Tested with a reproducer that queues N_SYSTEM_TRAY_REQUEST_DOCK events then
destroys the windows before on_change_systray can reparent them:
unpatched → SIGSEGV within a few rounds; patched → all rounds survive.
---
src/systray/systraybar.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/src/systray/systraybar.c b/src/systray/systraybar.c
index 2a9fafe..5eb5c6f 100644
--- a/src/systray/systraybar.c
+++ b/src/systray/systraybar.c
@@ -292,9 +292,10 @@ void on_change_systray(void *obj)
}
TrayWindow *traywin;
- GSList *l;
+ GSList *l, *next;
int i;
- for (i = 1, l = systray.list_icons; l; i++, l = l->next) {
+ for (i = 1, l = systray.list_icons; l; i++, l = next) {
+ next = l->next;
traywin = (TrayWindow *)l->data;
traywin->y = posy;
