Skip to content

geyes: fix crash/hang when theme-path is empty and "config" is a directory - #703

Open
u9g wants to merge 2 commits into
mate-desktop:masterfrom
u9g:geyes-empty-theme-path-crash
Open

u9g wants to merge 2 commits into
mate-desktop:masterfrom
u9g:geyes-empty-theme-path-crash

Conversation

@u9g

@u9g u9g commented Sep 5, 2026 •

Copy link
Copy Markdown

The Eyes applet crashes (or hangs at 100% CPU) at panel start-up when its theme-path key is unset and the process's working directory contains an entry named config. Seen on Ubuntu 22.04 with mate-applets 1.26.0; the affected code is unchanged on master.

Cause

Two things combine:

  1. properties_load() checks the theme path for NULL, but g_settings_get_string() returns "" for the unset key. g_build_filename("", "config") then yields the relative path config, resolved against the working directory. Under D-Bus activation that is the user's home directory.
  2. fopen() happily opens a directory. Reads then fail with EISDIR, which never sets the EOF flag, so the feof()-driven loop in parse_theme_file() runs on an uninitialised buffer. strtok() returns NULL and strncmp() dereferences it.

Backtrace from the crash report (frame 1-3 are in the applet):

#0  __strcmp_avx2 ()                     rdi=0x0, rsi="wall-thickness"
#1  parse_theme_file / load_theme
#4  mate_panel_applet_marshal_BOOLEAN__STRING ()

Fix

  • Treat an empty theme-path like NULL and fall back to Default-tiny.
  • Require the theme's config to be a regular file before opening it.
  • Drive the parser loop with fgets(), skip lines where strtok() returns NULL, and read values with a second strtok() call instead of walking past the NUL that the first call wrote over =.

Testing

  • All seven shipped themes parse to the same wall-thickness, num-eyes and pixmap values as before the change.
  • With a directory named config in the working directory and an empty theme-path, the old build hangs or segfaults; the patched build loads Default-tiny and draws the eyes on the panel.

@u9g
u9g force-pushed the geyes-empty-theme-path-crash branch from e98ca78 to 1fd592a Compare September 5, 2026 14:56
u9g added 2 commits September 5, 2026 10:58
g_settings_get_string() returns "" for the unset key, not NULL, so the
existing NULL check never fired and load_theme() was handed an empty
directory. g_build_filename("", "config") yields the relative path
"config", resolved against the applet's working directory, which under
D-Bus activation is the user's home directory.

Treat an empty string like NULL and use Default-tiny.
If the theme's "config" path names a directory (for example a stray
~/config when the theme path resolves to the working directory),
fopen() succeeds but every read fails with EISDIR. A read error never
sets the EOF flag, so the feof()-controlled parser loop ran on an
uninitialised line buffer: strtok() returned NULL and strncmp()
dereferenced it, crashing the applet at panel start-up, or the loop
spun forever at 100% CPU depending on stack contents.

- Require "config" to be a regular file before opening it, so a bogus
  theme path falls through to the Default-tiny fallback.
- Drive the parser loop with fgets() instead of feof() and skip lines
  where strtok() returns NULL.
- Read wall-thickness and num-eyes with a second strtok() call. The old
  code advanced past the NUL that the first strtok() wrote in place of
  '=' and only worked by reading beyond the terminator.
- Guard the pixmap branches against a missing quoted value.

Verified that all seven shipped themes parse to the same values as
before, and that the crash no longer reproduces when the working
directory contains a directory named "config".
@u9g
u9g force-pushed the geyes-empty-theme-path-crash branch from 1fd592a to 1a13215 Compare September 5, 2026 14:58

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant