On 2/17/25 5:02 AM, Martin Kletzander wrote:
On Mon, Feb 17, 2025 at 02:14:49AM -0500, Laine Stump wrote:
======
I'm sending this as an RFC just because what it's doing feels kind of
dirty - directly examining XDG_RUNTIME_DIR seems like an "end run"
around the Glib API. If anyone has a better idea of what to do, please
give details :-)
======
When running unprivileged (i.e. not as root, but as a regular user),
libvirt calls g_get_user_runtime_dir() (from Glib) to get the name of
a directory where status files can be saved. This is a directory that
is 1) writeable by the current user, and 2) will remain there until
the host reboots, but then 3) be erased after the reboot. This is used
for pidfiles, sockets created to communicate between processes, status
XML of active domains, etc.
Normally g_get_user_runtime_dir() returns the setting of
XDG_RUNTIME_DIR in the user's environment; usually this is set to
/run/user/${UID} (e.g. /run/user/1000) - that directory is created
when a user first logs in and is owned by the user, but is cleared out
when the system reboots (more specifically, this directory usually
resides in a tmpfs, and so disappears when that tmpfs is unmounted).
But sometimes XDG_RUNTIME_DIR isn't set in the user's environment. In
that case, g_get_user_runtime_dir() returns ${HOME}/.config
(e.g. /home/laine/.config). This directory fulfills the first 2
it's ~/.cache, not ~/.config
Yeah, sorry - that's what happens when I try to type up a commit log
message at 2AM :-)
criteria above, but fails the 3rd. This isn't just some pedantic
complaint - libvirt actually depends on the directory being cleared
out during a reboot - otherwise it might think that stale status files
are indicating active guests when in fact the guests were shutdown
during the reboot).
We should be able to cope with such files existing no matter what. It
might not be a reboot, but restart of the daemon during which the domain
dies etc. and we should handle that properly. If we do not, then there
is some other underlying problem we should still strive to solve.
Well, that part of my explanation for the reasoning behind the change
was just random supposition on my part. The main reason that made me
look into it is that when $HOME/.cache is used as the runtime dir, the
SELinux policies puke because there is no label saying that directory is
okay for us to write stuff. I only mention this other problem because
its a behavior we don't expect (although you're correct that if we don't
handle it properly anyway then that's a paddli.. er I mean bug.
In my opinion this behavior is a bug in Glib - see the requirements
for XDG_RUNTIME in the FreeDesktop documentation here:
https://specifications.freedesktop.org/basedir-spec/latest/#variables
and as this document points out, it's not the generally expected
behavior for XDG_RUNTIME_DIR - that doc explicitly says that the
directory MUST be deleted when the host reboots.
but they've documented the behavior as proper in the Glib docs for
g_get_user_runtime_dir(), and so likely will consider it not a bug.
...although the *same organization* documents that their function
returning a "runtime_dir" will return something that specifically breaks
that rule. (And due to it being documented, I'm guessing they won't want
to change it)
Beyond that, aside from failing the "must be cleared out during a reboot"
requirement, use of $HOME/.cache in this way also disturbs SELinux,
which gives an AVC denial when libvirt (or passt) tries to create a
file or socket in that directory (the SELinux policy permits use of
/run/user/$UID, but not of $HOME/.config). We *could* add that to the
SELinux policy, but since the glib behavior doesn't
All of the above is a very long leadup to the functionality in this
patch: rather than blindly accepting the path returned from
g_get_user_runtime_dir(), we first check if XDG_RUNTIME_DIR is set; if
it isn't set then we look to see if /run/user/$UID exists and is
writable by this user, if so we use *that* as the directory for our
status files. Otherwise (both when XDG_RUNTIME_DIR is set, and when
/run/user/$UID isn't usable) we fallback to just using the path
returned by g_get_user_runtime_dir() - that isn't perfect, but it's
what we were doing before, so at least it's not any worse.
Resolves: https://issues.redhat.com/browse/RHEL-70222
Signed-off-by: Laine Stump <laine@xxxxxxxxxx>
---
src/util/virutil.c | 22 ++++++++++++++++++++++
1 file changed, 22 insertions(+)
diff --git a/src/util/virutil.c b/src/util/virutil.c
index 2abcb282fe..4c7f4b62bc 100644
--- a/src/util/virutil.c
+++ b/src/util/virutil.c
@@ -538,6 +538,28 @@ char *virGetUserRuntimeDirectory(void)
#ifdef WIN32
return g_strdup(g_get_user_runtime_dir());
#else
+ /* tl;dr - if XDG_RUNTIME_DIR is set, use g_get_user_runtime_dir().
+ * if not set, then see if /run/user/$UID works
+ * if so, use that, else fallback to
g_get_user_runtime_dir()
+ *
+ * this is done because the directory returned by
+ * g_get_user_runtime_dir() when XDG_RUNTIME_DIR isn't set is
+ * "suboptimal" (it's a location that is owned by the user, but
+ * isn't erased when the user completely logs out)
+ */
+
+ if (!getenv("XDG_RUNTIME_DIR")) {
+ g_autofree char *runtime_dir = NULL;
+ struct stat sb;
+
+ runtime_dir = g_strdup_printf("/run/user/%d", getuid());
+ if (virFileIsDir(runtime_dir) &&
+ (stat(runtime_dir, &sb) == 0) && (sb.st_mode & S_IWUSR)) {
+ return g_build_filename(runtime_dir, "libvirt", NULL);
+ }
+ }
+
+ /* either XDG_RUNTIME_DIR was set, or /run/usr/$UID wasn't
writable */
return g_build_filename(g_get_user_runtime_dir(), "libvirt", NULL);
#endif
}
--
2.47.1
