GNOME Bugzilla – Bug 773435
After g_get_home_dir() fails, a second use deadlocks
Last modified: 2018-04-26 14:25:46 UTC
Steps to reproduce: * Don't set $HOME, for example in a systemd system service * Have an LSM profile or container environment that denies access to /etc/passwd, for example a strict AppArmor profile for a system service (in my case the system service updates a cache of AppStream entries) * Do something that looks up g_get_home_dir() as a side-effect, twice (in my case it was constructing a GFileMonitor, which uses g_get_home_dir() to probe for a NFS-mounted $HOME) Expected result: * g_get_home_dir() issues a warning, and returns "/" or "/nonexistent" or NULL * Second invocation of g_get_home_dir() returns the same thing, without crashing or deadlocking Actual result: GLib-WARNING **: getpwuid_r(): failed due to: Permission denied GLib-CRITICAL **: g_once_init_leave: assertion 'result != 0' failed and the second use of g_get_home_dir deadlocks, with this at the top of the stack:
+ Trace 236769
Analysis: This is because on error, g_get_home_dir() behaves contrary to g_once_init_enter()'s documented requirements: it outputs a zero value. This means the second g_get_home_dir() thinks the first is still performing its "do this once" actions, and waits for it; but because the first already gave up, it will wait forever.
Created attachment 371421 [details] [review] gutils: Fix deadlock if g_get_home_dir() fails when called twice If g_get_home_dir() calculated a NULL home directory (due to $HOME being unset and /etc/passwd being inaccessible, for example due to an overly-zealous LSM), it would call g_once_init_leave (&home_dir, NULL), which would emit a critical and fail to leave the GOnce critical section. That meant that the following call to g_get_home_dir() would deadlock in g_once_init_enter(). Fix that by setting the home directory to a made-up value in such cases (which the documentation handily already explicitly allows). Thanks to Simon McVittie for the analysis leading to an easy patch. Signed-off-by: Philip Withnall <withnall@endlessm.com>
Review of attachment 371421 [details] [review]: ::: glib/gutils.c @@ +893,3 @@ + g_warning ("Could not find home directory: $HOME is not set, and " + "user database could not be read."); + tmp = "/"; This is going to result in compiler warnings if built with -Wwrite-strings, which changes the type of string constants from char[n] to const char[n]. Is that OK? (If not, copy it anyway: one very small waste of memory per process, in a situation that is already a bug, seems harmless.)
Other than that, looks good to me (not a maintainer, etc.). If -Wwrite-strings is not something GLib aims to support, the patch seems good as-is; or if -Wwrite-strings is of interest, replacing tmp = "/" with tmp = g_strdup ("/") and amending the comment accordingly would also be fine.
Are you building GLib with -Wwrite-strings? I’ve just tried, and it produces a lot of warnings (too many for me to fix at the moment). I don’t think this is something we’re realistically going to enable any time soon.
Pushed unmodified, plus a bonus patch to fix a few of the more trivial cases I spotted while looking at -Wwrite-strings output briefly. Attachment 371421 [details] pushed as 9cbfb56 - gutils: Fix deadlock if g_get_home_dir() fails when called twice
Backported to glib-2-56 for good measure: c96e45496 (HEAD -> glib-2-56, origin/glib-2-56) gutils: Fix deadlock if g_get_home_dir() fails when called twice