Bug#1033385: openbox: Openbox crashes when switching out of a fullscreen window (requires GLib 2.76.0 to reproduce)

[Aaron Rainbolt]

Package: openbox
Severity: important
Tags: patch upstream
X-Debbugs-Cc: array...@ubuntu.com

This bug currently does not affect Debian with the current version of GLib in
the Debian archives. However, when Debian upgrades to GLib 2.75.0 or later,
this will almost certainly start happening.

In at least GLib 2.75.0 (possibly earlier but I'm not sure), the slice
allocator has been removed. This theoretically should not cause problems,
however it is revealing memory management problems in a number of apps, one of
which is Openbox. The commit removing the slice allocator is:
https://gitlab.gnome.org/GNOME/glib/-/commit/45b5a6c1e56d5b73cc5ed798ef59a5601e56c170

The offending function in Openbox:

-----

void client_calc_layer(ObClient *self)
{
GList *it;

/* skip over stuff above fullscreen layer */
for (it = stacking_list; it; it = g_list_next(it))
if (window_layer(it->data) <= OB_STACKING_LAYER_FULLSCREEN) break;

/* find the windows in the fullscreen layer, and mark them not-visited */
for (; it; it = g_list_next(it)) {
if (window_layer(it->data) < OB_STACKING_LAYER_FULLSCREEN) break;
else if (WINDOW_IS_CLIENT(it->data))
WINDOW_AS_CLIENT(it->data)->visited = FALSE;
}

client_calc_layer_internal(self);

/* skip over stuff above fullscreen layer */
for (it = stacking_list; it; it = g_list_next(it))
if (window_layer(it->data) <= OB_STACKING_LAYER_FULLSCREEN) break;

/* now recalc any windows in the fullscreen layer which have not
had their layer recalced already */
for (; it; it = g_list_next(it)) {
if (window_layer(it->data) < OB_STACKING_LAYER_FULLSCREEN) break;
else if (WINDOW_IS_CLIENT(it->data) &&
!WINDOW_AS_CLIENT(it->data)->visited)
client_calc_layer_internal(it->data);
}
}

-----

Notice in particular the "client_calc_layer_internal(it->data)" call. This
function calls code that proceeds to remove the list item that "it"
references. This renders "it" invalid. On the next iteration through the loop,
the now-invalid "it" pointer is used as if it were still valid (walking to the
next element in the list and then dereferencing it). When "it" is dereferenced
in the window_layer(it->data) call, Openbox crashes with a segmentation fault.

This bug has been reported upstream at
https://bugzilla.icculus.org/show_bug.cgi?id=6669. The following patch is
provided to fix the bug, and has been accepted into a developer's work branch
here:
http://git.openbox.org/?p=mikachu/openbox.git;a=commit;h=d41128e5a1002af41c976c8860f8299cfcd3cd72

-----

diff --git a/openbox/client.c b/openbox/client.c
index 3ff278ae..ac4ff827 100644
--- a/openbox/client.c
+++ b/openbox/client.c
@@ -2702,9 +2702,10 @@ static void client_calc_layer_internal(ObClient *self)
void client_calc_layer(ObClient *self)
{
GList *it;
+ GList *list = g_list_copy(stacking_list);

/* skip over stuff above fullscreen layer */
- for (it = stacking_list; it; it = g_list_next(it))
+ for (it = list; it; it = g_list_next(it))
if (window_layer(it->data) <= OB_STACKING_LAYER_FULLSCREEN) break;

/* find the windows in the fullscreen layer, and mark them not-visited */
@@ -2717,7 +2718,7 @@ void client_calc_layer(ObClient *self)
client_calc_layer_internal(self);

/* skip over stuff above fullscreen layer */
- for (it = stacking_list; it; it = g_list_next(it))
+ for (it = list; it; it = g_list_next(it))
if (window_layer(it->data) <= OB_STACKING_LAYER_FULLSCREEN) break;

/* now recalc any windows in the fullscreen layer which have not
@@ -2728,6 +2729,8 @@ void client_calc_layer(ObClient *self)
!WINDOW_AS_CLIENT(it->data)->visited)
client_calc_layer_internal(it->data);
}
+
+ g_list_free(it);
}

gboolean client_should_show(ObClient *self)

-----

I have verified that this does indeed fix the bug on Ubuntu (which uses GLib
2.75.0). It would likely be benefitial to Debian if this patch was applied
*before* Debian updates GLib to 2.75.0 or later, to avoid having these crashes
start happening.

I have not attempted to reproduce this bug on Debian, however since it is
known upstream and has a well-known cause and fix, I believe this is still
valid.


-- System Information:
Debian Release: bookworm/sid
APT prefers jammy-updates
APT policy: (500, 'jammy-updates'), (500, 'jammy-security'), (500, 'jammy'), (100, 'jammy-backports')
Architecture: amd64 (x86_64)
Foreign Architectures: i386

Kernel: Linux 5.19.0-32-generic (SMP w/8 CPU threads; PREEMPT)
Kernel taint flags: TAINT_OOT_MODULE, TAINT_UNSIGNED_MODULE
Locale: LANG=en_US.UTF-8, LC_CTYPE=en_US.UTF-8 (charmap=UTF-8), LANGUAGE not set
Shell: /bin/sh linked to /usr/bin/dash
Init: systemd (via /run/systemd/system)
LSM: AppArmor: enabled

Versions of packages openbox depends on:
ii libc6 2.35-0ubuntu3.1
ii libglib2.0-0 2.72.4-0ubuntu1
ii libice6 2:1.0.10-1build2
pn libobrender32v5 <none>
pn libobt2v5 <none>
ii libsm6 2:1.2.3-1build2
pn libstartup-notification0 <none>
ii libx11-6 2:1.7.5-1
ii libxau6 1:1.0.9-1build5
ii libxcursor1 1:1.2.0-2build4
ii libxext6 2:1.3.4-1build1
ii libxi6 2:1.8-1build1
ii libxinerama1 2:1.1.4-3
ii libxrandr2 2:1.5.2-1build1
ii python3 3.10.6-1~22.04

Versions of packages openbox recommends:
pn obconf | obconf-qt <none>
pn scrot <none>

Versions of packages openbox suggests:
ii fonts-dejavu 2.37-2build1
ii libxml2-dev 2.9.13+dfsg-1ubuntu0.2
pn openbox-gnome-session <none>
pn openbox-kde-session <none>
pn tint2 <none>

[Karine Crèvecœur]

Hi,

I have encoutered the same bug. When playing a video with mpv, mplayer
or vlc, switching to another windows while in fullscreen leads to crash
of openbox.

So, I have tested the proposed path. It works perfectly. Thanks to
Aaron.

I use debian/sid. apt policy libglib2.0-0 returns:

libglib2.0-0:
Installed: 2.76.4-3
Candidate: 2.76.4-3
Version table:
*** 2.76.4-3 900
900 https://deb.debian.org/debian sid/main amd64 Packages
100 /var/lib/dpkg/status
2.76.4-1 100
100 https://deb.debian.org/debian rc-buggy/main amd64 Packages


openbox version I use is 3.6.1-10.

--
Karine Crèvecœur

[justhate]

Same issue here as Karine explained.

The bug was reported 4 months ago and here we are, upgraded glib
yesterday [libglib2.0-0:amd64 (2.74.6-2, 2.76.4-3)] and Openbox
(3.6.1-10) is quite unusable.

Despite the icculus bug report seems to be a bit in an unknown state
isn't the patch good enough to be applied?

[Pierre Tomon]

Le Sun, 23 Jul 2023 15:09:31 +0200,
justhate <just...@gmx.com> a écrit :

There is an updated package of Openbox, but it requires a sponsor:
https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1041486