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

Toggle useless messages

Message #5 received at submit@bugs.debian.org (full text, mbox, reply):

From: Aaron Rainbolt <arraybolt3@ubuntu.com>

To: Debian Bug Tracking System <submit@bugs.debian.org>

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

Date: Thu, 23 Mar 2023 20:40:17 -0500

Package: openbox
Severity: important
Tags: patch upstream
X-Debbugs-Cc: arraybolt3@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>

Message #10 received at 1033385@bugs.debian.org (full text, mbox, reply):

From: Karine Crèvecœur <enikar@gresille.org>

To: 1033385@bugs.debian.org

Subject: The proposed patch works

Date: Fri, 21 Jul 2023 01:37:52 +0200

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

Message #15 received at 1033385@bugs.debian.org (full text, mbox, reply):

From: justhate <justhate@gmx.com>

To: 1033385@bugs.debian.org

Subject: Any progress?

Date: Sun, 23 Jul 2023 15:09:31 +0200

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?

Message #20 received at 1033385@bugs.debian.org (full text, mbox, reply):

From: Pierre Tomon <pierretom+11@ik.me>

To: 1033385@bugs.debian.org

Subject: Re: Bug#1033385: Any progress?

Date: Sun, 23 Jul 2023 15:49:34 +0200

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

> 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?
> 

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

Message #25 received at 1033385-close@bugs.debian.org (full text, mbox, reply):

From: Debian FTP Masters <ftpmaster@ftp-master.debian.org>

To: 1033385-close@bugs.debian.org

Subject: Bug#1033385: fixed in openbox 3.6.1-11

Date: Mon, 24 Jul 2023 17:51:30 +0000

Source: openbox
Source-Version: 3.6.1-11
Done: Mateusz Łukasik <mati75@linuxmint.pl>

We believe that the bug you reported is fixed in the latest version of
openbox, which is due to be installed in the Debian FTP archive.

A summary of the changes between this version and the previous one is
attached.

Thank you for reporting the bug, which will now be closed.  If you
have further comments please address them to 1033385@bugs.debian.org,
and the maintainer will reopen the bug report if appropriate.

Debian distribution maintenance software
pp.
Mateusz Łukasik <mati75@linuxmint.pl> (supplier of updated openbox package)

(This message was generated automatically at their request; if you
believe that there is a problem with it please contact the archive
administrators by mailing ftpmaster@ftp-master.debian.org)


-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

Format: 1.8
Date: Wed, 19 Jul 2023 17:18:18 +0200
Source: openbox
Architecture: source
Version: 3.6.1-11
Distribution: unstable
Urgency: medium
Maintainer: Mateusz Łukasik <mati75@linuxmint.pl>
Changed-By: Mateusz Łukasik <mati75@linuxmint.pl>
Closes: 886716 1007147 1010019 1033385
Changes:
 openbox (3.6.1-11) unstable; urgency=medium
 .
   [ Mateusz Łukasik ]
   * debian/obamenu:
     + Use x-terminal-emulator as terminal emulator. (Closes: #1010019)
     + Fix ignoring broken links. (Closes: #886716)
   * d/patches:
     + Add patch for fix openbox invokes imlib_free_image when no image
     is loaded (Closes: #1007147)
     + Add patch from upstream for fix crashes when switching out of
     a fullscreen window with GLib 2.76.0. (LP: #2011751) (Closes: #1033385)
   * d/control: Bump Standards-Version to 4.6.2.
   * d/copyright: Welcome 2023.
   * d/watch: Update to version 4 and change sources repository.
 .
   [ Debian Janitor ]
   * Set upstream metadata fields: Repository-Browse.
   * Remove constraints unnecessary since buster:
     + Build-Depends: Drop versioned constraint on automake and libxml2-dev.
     + gnome-panel-control: Drop versioned constraint on openbox in Replaces.
     + gnome-panel-control: Drop versioned constraint on openbox in Breaks.
     + openbox-gnome-session: Drop versioned constraint on openbox in Replaces.
     + openbox-gnome-session: Drop versioned constraint on openbox in Breaks.
     + openbox-kde-session: Drop versioned constraint on openbox in Replaces.
     + openbox-kde-session: Drop versioned constraint on openbox in Breaks.
   * Remove 1 obsolete maintscript entry.
   * Avoid explicitly specifying -Wl,--as-needed linker flag.
 .
   [ Ingo Brückl ]
   * Update German translation.
Checksums-Sha1:
 4c99e966b8489fd9ab26578f523742db85374fe7 2301 openbox_3.6.1-11.dsc
 9ba527357e8b72af63cf9d83e1ddca4c3ce28ce4 61208 openbox_3.6.1-11.debian.tar.xz
 5a30ebfec8cf9667cced6adfc1756bba85d40d96 5730 openbox_3.6.1-11_source.buildinfo
Checksums-Sha256:
 8dd0f5db7374e8bb38982ddfc8e666270f01751fbb76dc7e56720d3b41608758 2301 openbox_3.6.1-11.dsc
 a241e07b1d838ee5a1204269a8b8ab1cda8a81ede8d078ddfce79130982d5ef6 61208 openbox_3.6.1-11.debian.tar.xz
 f6427ca310008986f66db0082d9939ba1a9868fef0f4dd22bf30fda184a96269 5730 openbox_3.6.1-11_source.buildinfo
Files:
 aa1e5da104745714fbcd707163bec938 2301 x11 optional openbox_3.6.1-11.dsc
 dad29b7dadc7e3c8b4f6a4d9ed0b7c05 61208 x11 optional openbox_3.6.1-11.debian.tar.xz
 1dc0cc651a6947895dca58c863946256 5730 x11 optional openbox_3.6.1-11_source.buildinfo

-----BEGIN PGP SIGNATURE-----

iQHEBAEBCgAuFiEEQGIgyLhVKAI3jM5BH1x6i0VWQxQFAmS+sV8QHGJhZ2VAZGVi
aWFuLm9yZwAKCRAfXHqLRVZDFBmXC/0XoDg6gIw0cUnspT5Y8olbhAHp5fgrH1uz
7opV9fWl2261W+TdsEe8ewqSmEzrrdqeO1q0brFfqhiN2RVjBqPMSlDzGWJhJF+0
CVVa//34an240FWuy7/+MFSGcaze3y9fefpiGpHhQHELQ7gtF/PWWoH/1uS//NGp
MyFCGZk6Ml/MEoLkZplImOdDmc5L1QOrQ+bsEU6fiiHAYiDZ4VX58bARBfS90Xbb
5Jv0wv96gNF+2R2tch4CmrV92aknmHICUx85PXFGMXNn1j4VxUGM34e19aP1zVxa
1BTZr6N67BGyyaiFyzFBb06i/ZPVoDfEK2++2fMhOBHmFrW0J+FR/5HOsb0/dvCL
kIiVZx14RMLcX7jv+xSpE8/Vz+14hZQ+eBujo83w89vH7/tVduAlJRP9Bg2Rl5oW
L6rvPYxc8j6GstT6jGjE/4szKp/RvIYXMGTCO/JVLi4ZFbKLsQU8Vyg+XXZRXBA8
fvw3mqd75Uw93CBV/xQAOrqbMoOrdSI=
=w70H
-----END PGP SIGNATURE-----

Send a report that this bug log contains spam.