[PATCH RFC] util: pick a better runtime directory when XDG_RUNTIME_DIR isn't set

Laine Stump posted 1 patch 10 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20250217071449.1443346-1-laine@redhat.com
src/util/virutil.c | 22 ++++++++++++++++++++++
1 file changed, 22 insertions(+)
[PATCH RFC] util: pick a better runtime directory when XDG_RUNTIME_DIR isn't set
Posted by Laine Stump 10 months ago
======

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
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).

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

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.

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@redhat.com>
---
 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
Re: [PATCH RFC] util: pick a better runtime directory when XDG_RUNTIME_DIR isn't set
Posted by Richard W.M. Jones 10 months ago
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
> 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).
> 
> 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
> 
> 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.
> 
> 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@redhat.com>
> ---
>  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());

I'm wondering if this should be geteuid.  (I'm not completely clear
myself, I guess it depends on whether this code could be a called in a
setuid / dropped privileges case, and if so what directory should be
used.)

> +        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
>  }

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
nbdkit - Flexible, fast NBD server with plugins
https://gitlab.com/nbdkit/nbdkit
Re: [PATCH RFC] util: pick a better runtime directory when XDG_RUNTIME_DIR isn't set
Posted by Daniel P. Berrangé 10 months ago
On Mon, Feb 17, 2025 at 11:31:32AM +0000, Richard W.M. Jones 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
> > 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).
> > 
> > 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
> > 
> > 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.
> > 
> > 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@redhat.com>
> > ---
> >  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());
> 
> I'm wondering if this should be geteuid.  (I'm not completely clear
> myself, I guess it depends on whether this code could be a called in a
> setuid / dropped privileges case, and if so what directory should be
> used.)

We explicitly block such usage

    if (getuid() != geteuid() ||
        getgid() != getegid()) {
        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
                       _("libvirt.so is not safe to use from setuid/setgid programs"));
        goto error;
    }

It is unsafe because too many things we link to have ELF constructors
that are untrustworthy.

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|
Re: [PATCH RFC] util: pick a better runtime directory when XDG_RUNTIME_DIR isn't set
Posted by Daniel P. Berrangé 10 months ago
On Mon, Feb 17, 2025 at 02:14:49AM -0500, Laine Stump wrote:
> But sometimes XDG_RUNTIME_DIR isn't set in the user's environment.

Do you have examples of scenarios in which this happens, and
yet the /run/user/NNNN directory is still being created, as
that rather sounds like something is broken outside of libvirt.

>                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 could make our status files robust against host reboots by
storing "/proc/sys/kernel/random/boot_id" in them. If boot_id
has changed, we know the pidfile is stale.


With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|
Re: [PATCH RFC] util: pick a better runtime directory when XDG_RUNTIME_DIR isn't set
Posted by Laine Stump 10 months ago
On 2/17/25 5:28 AM, Daniel P. Berrangé wrote:
> On Mon, Feb 17, 2025 at 02:14:49AM -0500, Laine Stump wrote:
>> But sometimes XDG_RUNTIME_DIR isn't set in the user's environment.
> 
> Do you have examples of scenarios in which this happens, and
> yet the /run/user/NNNN directory is still being created, as
> that rather sounds like something is broken outside of libvirt.

After seeing the bug report, I replicated the situation by ssh'ing in as 
a user that hadn't previously logged in, and then unsetting 
XDG_RUNTIME_DIR. I hadn't thought there might be some other case where 
the user could be logged in but XDG_RUNTIME_DIR had never been set.

But after seeing your question I tried running

   sudo $someuser virsh list

where $someuser was a user than hadn't logged in since the last host 
reboot (and so therefore /run/user/$someuser-id didn't exist) and then 
checking for /run/user/$someuser-id, and you're correct that it wasn't 
created :-/ (and also :-) I guess)

Of course if that user had previously logged in "normally" since the 
last host reboot, then that login session *would* create 
/run/user/$someuser-id. So what I'm doing in this patch would help in 
some situations, but not all (at least it doesn't make any situation 
*worse* though - if /root/user/$id doesn't exist then it falls back to 
the current behavior).

(As a matter of fact, it seems like we have a problem now that if 
someone does login as userX o a normal session, starts up some guests, 
and the virtqemud process is terminated for some reason and then later 
they use "su userX virsh list" (or whatever) then the newly started 
virtqemud will incorrectly look in $HOME/.cache to find the list of 
currently active guests.)

So other than sticking with the current behavior and requesting an 
SELinux change to allow us to use $HOME/.cache (which wouldn't solve the 
problem in the previous paragraph), I suppose we could just decide that 
/tmp/libvirt-${uid} shouldn't be used by anybody but us, and make *that* 
our backup directory (maybe all the time so that it's consistent).

> 
>>                 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 could make our status files robust against host reboots by
> storing "/proc/sys/kernel/random/boot_id" in them. If boot_id
> has changed, we know the pidfile is stale.

I haven't checked if there is an *actual* problem of us believing stale 
status files are valid (that was just an idle supposition on my part of 
something that *might* be problematic due to using a misbehaving runtime 
dir), but in general that sounds like a good idea (at least on Linux 
systems). Doesn't solve the SELinux (and presumably AppArmor) problem 
though...

(I guess I need to try something like

1) start a guest with "sudo $user virsh start blah"
2) restart the host
    [we now have a $HOME/.cache populated with status files]
3) again use sudo $user to try several virsh commands that
    would get info from status files to see if anything fails.

)

Fortunately this is only an issue for unprivileged libvirt, and most of 
the drivers don't work unprivileged anyway (right? the only other driver 
I've ever paid attention wrt to this was the network driver).
Re: [PATCH RFC] util: pick a better runtime directory when XDG_RUNTIME_DIR isn't set
Posted by Daniel P. Berrangé 10 months ago
On Mon, Feb 17, 2025 at 03:11:56PM -0500, Laine Stump wrote:
> On 2/17/25 5:28 AM, Daniel P. Berrangé wrote:
> > On Mon, Feb 17, 2025 at 02:14:49AM -0500, Laine Stump wrote:
> > > But sometimes XDG_RUNTIME_DIR isn't set in the user's environment.
> > 
> > Do you have examples of scenarios in which this happens, and
> > yet the /run/user/NNNN directory is still being created, as
> > that rather sounds like something is broken outside of libvirt.
> 
> After seeing the bug report, I replicated the situation by ssh'ing in as a
> user that hadn't previously logged in, and then unsetting XDG_RUNTIME_DIR. I
> hadn't thought there might be some other case where the user could be logged
> in but XDG_RUNTIME_DIR had never been set.
> 
> But after seeing your question I tried running
> 
>   sudo $someuser virsh list

NB, that is the classic sudo usage trapdoor, because they didn't
make "-i" (aka --login) the default, so your environment is not
populated correctly.

I'd hope that when passing sudo -i ... it will do the right
thing

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|
Re: [PATCH RFC] util: pick a better runtime directory when XDG_RUNTIME_DIR isn't set
Posted by Laine Stump 10 months ago
On 2/18/25 4:26 AM, Daniel P. Berrangé wrote:
> On Mon, Feb 17, 2025 at 03:11:56PM -0500, Laine Stump wrote:
>> On 2/17/25 5:28 AM, Daniel P. Berrangé wrote:
>>> On Mon, Feb 17, 2025 at 02:14:49AM -0500, Laine Stump wrote:
>>>> But sometimes XDG_RUNTIME_DIR isn't set in the user's environment.
>>>
>>> Do you have examples of scenarios in which this happens, and
>>> yet the /run/user/NNNN directory is still being created, as
>>> that rather sounds like something is broken outside of libvirt.
>>
>> After seeing the bug report, I replicated the situation by ssh'ing in as a
>> user that hadn't previously logged in, and then unsetting XDG_RUNTIME_DIR. I
>> hadn't thought there might be some other case where the user could be logged
>> in but XDG_RUNTIME_DIR had never been set.
>>
>> But after seeing your question I tried running
>>
>>    sudo $someuser virsh list
> 
> NB, that is the classic sudo usage trapdoor, because they didn't
> make "-i" (aka --login) the default, so your environment is not
> populated correctly.
> 
> I'd hope that when passing sudo -i ... it will do the right
> thing

It seems not. If I login as $someuser, start a guest, then in a separate 
terminal window from root run:

       sudo -u $someuser -i virsh list

It returns an empty list (the same as if I omit the -i). By running the 
same command without "virsh list", I'm given a shell instance, and 
within that shell I can see that $UID, $USER, and $EUID are all set, but 
$XDG_RUNTIME_DIR is not.
Re: [PATCH RFC] util: pick a better runtime directory when XDG_RUNTIME_DIR isn't set
Posted by Daniel P. Berrangé 10 months ago
On Tue, Feb 18, 2025 at 09:33:43AM -0500, Laine Stump wrote:
> On 2/18/25 4:26 AM, Daniel P. Berrangé wrote:
> > On Mon, Feb 17, 2025 at 03:11:56PM -0500, Laine Stump wrote:
> > > On 2/17/25 5:28 AM, Daniel P. Berrangé wrote:
> > > > On Mon, Feb 17, 2025 at 02:14:49AM -0500, Laine Stump wrote:
> > > > > But sometimes XDG_RUNTIME_DIR isn't set in the user's environment.
> > > > 
> > > > Do you have examples of scenarios in which this happens, and
> > > > yet the /run/user/NNNN directory is still being created, as
> > > > that rather sounds like something is broken outside of libvirt.
> > > 
> > > After seeing the bug report, I replicated the situation by ssh'ing in as a
> > > user that hadn't previously logged in, and then unsetting XDG_RUNTIME_DIR. I
> > > hadn't thought there might be some other case where the user could be logged
> > > in but XDG_RUNTIME_DIR had never been set.
> > > 
> > > But after seeing your question I tried running
> > > 
> > >    sudo $someuser virsh list
> > 
> > NB, that is the classic sudo usage trapdoor, because they didn't
> > make "-i" (aka --login) the default, so your environment is not
> > populated correctly.
> > 
> > I'd hope that when passing sudo -i ... it will do the right
> > thing
> 
> It seems not. If I login as $someuser, start a guest, then in a separate
> terminal window from root run:
> 
>       sudo -u $someuser -i virsh list
> 
> It returns an empty list (the same as if I omit the -i). By running the same
> command without "virsh list", I'm given a shell instance, and within that
> shell I can see that $UID, $USER, and $EUID are all set, but
> $XDG_RUNTIME_DIR is not.

Hmm, this appears to be caused by systemd_pam

When using "su -" (similar seen with sudo)

su[5870]: pam_systemd(su-l:session): pam-systemd initializing
su[5870]: pam_systemd(su-l:session): New sd-bus connection (system-bus-pam-systemd-5870) opened.
su[5870]: pam_systemd(su-l:session): Asking logind to create session: uid=1001 pid=5870 service=su-l type=tty class=user desktop= seat= vtnr=0 tty=pts/3 display= remote=no remote_user=root remote_host=
su[5870]: pam_systemd(su-l:session): Session limits: memory_max=n/a tasks_max=n/a cpu_weight=n/a io_weight=n/a runtime_max_sec=n/a
su[5870]: pam_systemd(su-l:session): Not creating session: Already running in a session or user slice
su[5870]: pam_systemd(su-l:session): pam-systemd shutting down

vs used with ssh:

sshd-session[5937]: pam_systemd(sshd:session): pam-systemd initializing
sshd-session[5937]: pam_systemd(sshd:session): New sd-bus connection (system-bus-pam-systemd-5937) opened.
sshd-session[5937]: pam_systemd(sshd:session): Asking logind to create session: uid=0 pid=5937 service=sshd type=tty class=user desktop= seat= vtnr=0 tty= display= remote=yes remote_user= remote_host=10.42.28.158
sshd-session[5937]: pam_systemd(sshd:session): Session limits: memory_max=n/a tasks_max=n/a cpu_weight=n/a io_weight=n/a runtime_max_sec=n/a
sshd-session[5937]: pam_systemd(sshd:session): Reply from logind: id=12 object_path=/org/freedesktop/login1/session/_312 runtime_path=/run/user/0 session_fd=9 seat= vtnr=0 original_uid=0


So if the current user is already in a login sesssion, it'll refuse to
start a new session.

I struggle to understand the rationale for this behaviour. It seems
guaranteed to break stuff...

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|
Re: [PATCH RFC] util: pick a better runtime directory when XDG_RUNTIME_DIR isn't set
Posted by Daniel P. Berrangé 10 months ago
On Tue, Feb 18, 2025 at 02:50:52PM +0000, Daniel P. Berrangé wrote:
> On Tue, Feb 18, 2025 at 09:33:43AM -0500, Laine Stump wrote:
> > On 2/18/25 4:26 AM, Daniel P. Berrangé wrote:
> > > On Mon, Feb 17, 2025 at 03:11:56PM -0500, Laine Stump wrote:
> > > > On 2/17/25 5:28 AM, Daniel P. Berrangé wrote:
> > > > > On Mon, Feb 17, 2025 at 02:14:49AM -0500, Laine Stump wrote:
> > > > > > But sometimes XDG_RUNTIME_DIR isn't set in the user's environment.
> > > > > 
> > > > > Do you have examples of scenarios in which this happens, and
> > > > > yet the /run/user/NNNN directory is still being created, as
> > > > > that rather sounds like something is broken outside of libvirt.
> > > > 
> > > > After seeing the bug report, I replicated the situation by ssh'ing in as a
> > > > user that hadn't previously logged in, and then unsetting XDG_RUNTIME_DIR. I
> > > > hadn't thought there might be some other case where the user could be logged
> > > > in but XDG_RUNTIME_DIR had never been set.
> > > > 
> > > > But after seeing your question I tried running
> > > > 
> > > >    sudo $someuser virsh list
> > > 
> > > NB, that is the classic sudo usage trapdoor, because they didn't
> > > make "-i" (aka --login) the default, so your environment is not
> > > populated correctly.
> > > 
> > > I'd hope that when passing sudo -i ... it will do the right
> > > thing
> > 
> > It seems not. If I login as $someuser, start a guest, then in a separate
> > terminal window from root run:
> > 
> >       sudo -u $someuser -i virsh list
> > 
> > It returns an empty list (the same as if I omit the -i). By running the same
> > command without "virsh list", I'm given a shell instance, and within that
> > shell I can see that $UID, $USER, and $EUID are all set, but
> > $XDG_RUNTIME_DIR is not.
> 
> Hmm, this appears to be caused by systemd_pam
> 
> When using "su -" (similar seen with sudo)
> 
> su[5870]: pam_systemd(su-l:session): pam-systemd initializing
> su[5870]: pam_systemd(su-l:session): New sd-bus connection (system-bus-pam-systemd-5870) opened.
> su[5870]: pam_systemd(su-l:session): Asking logind to create session: uid=1001 pid=5870 service=su-l type=tty class=user desktop= seat= vtnr=0 tty=pts/3 display= remote=no remote_user=root remote_host=
> su[5870]: pam_systemd(su-l:session): Session limits: memory_max=n/a tasks_max=n/a cpu_weight=n/a io_weight=n/a runtime_max_sec=n/a
> su[5870]: pam_systemd(su-l:session): Not creating session: Already running in a session or user slice
> su[5870]: pam_systemd(su-l:session): pam-systemd shutting down
> 
> vs used with ssh:
> 
> sshd-session[5937]: pam_systemd(sshd:session): pam-systemd initializing
> sshd-session[5937]: pam_systemd(sshd:session): New sd-bus connection (system-bus-pam-systemd-5937) opened.
> sshd-session[5937]: pam_systemd(sshd:session): Asking logind to create session: uid=0 pid=5937 service=sshd type=tty class=user desktop= seat= vtnr=0 tty= display= remote=yes remote_user= remote_host=10.42.28.158
> sshd-session[5937]: pam_systemd(sshd:session): Session limits: memory_max=n/a tasks_max=n/a cpu_weight=n/a io_weight=n/a runtime_max_sec=n/a
> sshd-session[5937]: pam_systemd(sshd:session): Reply from logind: id=12 object_path=/org/freedesktop/login1/session/_312 runtime_path=/run/user/0 session_fd=9 seat= vtnr=0 original_uid=0
> 
> 
> So if the current user is already in a login sesssion, it'll refuse to
> start a new session.
> 
> I struggle to understand the rationale for this behaviour. It seems
> guaranteed to break stuff...

Apparently, 'su -' and 'sudo' shouldn't be used anymore if you
want a shell running as a different user which can run arbitrary
apps. Instead you're expected to use

   machinectl shell username@

Or 

   sudo machinectl shell username@

which will get the full set of env info setup.

I find it somewhat dubious to simply re-declare that decades of usage
of 'su' and 'sudo' is now wrong but that's the documented answer:

  https://github.com/systemd/systemd/issues/7451#issuecomment-346787237

Likewise in context of RHEL:

  https://access.redhat.com/solutions/6634751

Anyway, given that this is deliberate behaviour, I'm not convinced that
it is libvirt's job to workaround, even if we think that behaviour is
sub-optimal.

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|
Re: [PATCH RFC] util: pick a better runtime directory when XDG_RUNTIME_DIR isn't set
Posted by Laine Stump 9 months, 2 weeks ago
On 2/18/25 12:21 PM, Daniel P. Berrangé wrote:
> On Tue, Feb 18, 2025 at 02:50:52PM +0000, Daniel P. Berrangé wrote:
>> On Tue, Feb 18, 2025 at 09:33:43AM -0500, Laine Stump wrote:
>>> On 2/18/25 4:26 AM, Daniel P. Berrangé wrote:
>>>> On Mon, Feb 17, 2025 at 03:11:56PM -0500, Laine Stump wrote:
>>>>> On 2/17/25 5:28 AM, Daniel P. Berrangé wrote:
>>>>>> On Mon, Feb 17, 2025 at 02:14:49AM -0500, Laine Stump wrote:
>>>>>>> But sometimes XDG_RUNTIME_DIR isn't set in the user's environment.
>>>>>>
>>>>>> Do you have examples of scenarios in which this happens, and
>>>>>> yet the /run/user/NNNN directory is still being created, as
>>>>>> that rather sounds like something is broken outside of libvirt.
>>>>>
>>>>> After seeing the bug report, I replicated the situation by ssh'ing in as a
>>>>> user that hadn't previously logged in, and then unsetting XDG_RUNTIME_DIR. I
>>>>> hadn't thought there might be some other case where the user could be logged
>>>>> in but XDG_RUNTIME_DIR had never been set.
>>>>>
>>>>> But after seeing your question I tried running
>>>>>
>>>>>     sudo $someuser virsh list
>>>>
>>>> NB, that is the classic sudo usage trapdoor, because they didn't
>>>> make "-i" (aka --login) the default, so your environment is not
>>>> populated correctly.
>>>>
>>>> I'd hope that when passing sudo -i ... it will do the right
>>>> thing
>>>
>>> It seems not. If I login as $someuser, start a guest, then in a separate
>>> terminal window from root run:
>>>
>>>        sudo -u $someuser -i virsh list
>>>
>>> It returns an empty list (the same as if I omit the -i). By running the same
>>> command without "virsh list", I'm given a shell instance, and within that
>>> shell I can see that $UID, $USER, and $EUID are all set, but
>>> $XDG_RUNTIME_DIR is not.
>>
>> Hmm, this appears to be caused by systemd_pam
>>
>> When using "su -" (similar seen with sudo)
>>
>> su[5870]: pam_systemd(su-l:session): pam-systemd initializing
>> su[5870]: pam_systemd(su-l:session): New sd-bus connection (system-bus-pam-systemd-5870) opened.
>> su[5870]: pam_systemd(su-l:session): Asking logind to create session: uid=1001 pid=5870 service=su-l type=tty class=user desktop= seat= vtnr=0 tty=pts/3 display= remote=no remote_user=root remote_host=
>> su[5870]: pam_systemd(su-l:session): Session limits: memory_max=n/a tasks_max=n/a cpu_weight=n/a io_weight=n/a runtime_max_sec=n/a
>> su[5870]: pam_systemd(su-l:session): Not creating session: Already running in a session or user slice
>> su[5870]: pam_systemd(su-l:session): pam-systemd shutting down
>>
>> vs used with ssh:
>>
>> sshd-session[5937]: pam_systemd(sshd:session): pam-systemd initializing
>> sshd-session[5937]: pam_systemd(sshd:session): New sd-bus connection (system-bus-pam-systemd-5937) opened.
>> sshd-session[5937]: pam_systemd(sshd:session): Asking logind to create session: uid=0 pid=5937 service=sshd type=tty class=user desktop= seat= vtnr=0 tty= display= remote=yes remote_user= remote_host=10.42.28.158
>> sshd-session[5937]: pam_systemd(sshd:session): Session limits: memory_max=n/a tasks_max=n/a cpu_weight=n/a io_weight=n/a runtime_max_sec=n/a
>> sshd-session[5937]: pam_systemd(sshd:session): Reply from logind: id=12 object_path=/org/freedesktop/login1/session/_312 runtime_path=/run/user/0 session_fd=9 seat= vtnr=0 original_uid=0
>>
>>
>> So if the current user is already in a login sesssion, it'll refuse to
>> start a new session.
>>
>> I struggle to understand the rationale for this behaviour. It seems
>> guaranteed to break stuff...
> 
> Apparently, 'su -' and 'sudo' shouldn't be used anymore if you
> want a shell running as a different user which can run arbitrary
> apps. Instead you're expected to use
> 
>     machinectl shell username@
> 
> Or
> 
>     sudo machinectl shell username@
> 
> which will get the full set of env info setup.
> 
> I find it somewhat dubious to simply re-declare that decades of usage
> of 'su' and 'sudo' is now wrong but that's the documented answer:
> 
>    https://github.com/systemd/systemd/issues/7451#issuecomment-346787237
> 
> Likewise in context of RHEL:
> 
>    https://access.redhat.com/solutions/6634751
> 
> Anyway, given that this is deliberate behaviour, I'm not convinced that
> it is libvirt's job to workaround, even if we think that behaviour is
> sub-optimal.

Especially since my workaround ends up being a "breakaround" if 
/run/user/$OTHER_UID doesn't already exist (i.e. if there isn't already 
a user session created for that user in some other way - we try to 
create /run/user/$OTHER_UID/libvirt, and fail), I definitely agree.

Rich suggested online last week that maybe we could log a warning if 
XDG_RUNTIME_DIR hasn't been set in the environment (thus indicating that 
we're being executed via "sudo -u $user" or similar). I made an example 
patch of that and am sending it as an RFC so that anyone who wants can 
try it out and see if it's useful (and also suggest a better warning 
message, maybe pointing to a knowledgebase article with more details).
Re: [PATCH RFC] util: pick a better runtime directory when XDG_RUNTIME_DIR isn't set
Posted by Laine Stump 10 months ago
On 2/17/25 3:11 PM, Laine Stump wrote:
> On 2/17/25 5:28 AM, Daniel P. Berrangé wrote:
>> On Mon, Feb 17, 2025 at 02:14:49AM -0500, Laine Stump wrote:
>>> But sometimes XDG_RUNTIME_DIR isn't set in the user's environment.
>>
>> Do you have examples of scenarios in which this happens, and
>> yet the /run/user/NNNN directory is still being created, as
>> that rather sounds like something is broken outside of libvirt.
> 
> After seeing the bug report, I replicated the situation by ssh'ing in as 
> a user that hadn't previously logged in, and then unsetting 
> XDG_RUNTIME_DIR. I hadn't thought there might be some other case where 
> the user could be logged in but XDG_RUNTIME_DIR had never been set.
> 
> But after seeing your question I tried running
> 
>    sudo $someuser virsh list

actually it was "sudo -u $someuser virsh list"

> 
> where $someuser was a user than hadn't logged in since the last host 
> reboot (and so therefore /run/user/$someuser-id didn't exist) and then 
> checking for /run/user/$someuser-id, and you're correct that it wasn't 
> created :-/ (and also :-) I guess)
> 
> Of course if that user had previously logged in "normally" since the 
> last host reboot, then that login session *would* create /run/user/ 
> $someuser-id. So what I'm doing in this patch would help in some 
> situations, but not all (at least it doesn't make any situation *worse* 
> though - if /root/user/$id doesn't exist then it falls back to the 
> current behavior).
> 
> (As a matter of fact, it seems like we have a problem now that if 
> someone does login as userX o a normal session, starts up some guests, 
> and the virtqemud process is terminated for some reason and then later 
> they use "su userX virsh list" (or whatever) then the newly started 
> virtqemud will incorrectly look in $HOME/.cache to find the list of 
> currently active guests.)
> 
> So other than sticking with the current behavior and requesting an 
> SELinux change to allow us to use $HOME/.cache (which wouldn't solve the 
> problem in the previous paragraph), I suppose we could just decide 
> that /tmp/libvirt-${uid} shouldn't be used by anybody but us, and make 
> *that* our backup directory (maybe all the time so that it's consistent).
> 
>>
>>>                 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 could make our status files robust against host reboots by
>> storing "/proc/sys/kernel/random/boot_id" in them. If boot_id
>> has changed, we know the pidfile is stale.
> 
> I haven't checked if there is an *actual* problem of us believing stale 
> status files are valid (that was just an idle supposition on my part of 
> something that *might* be problematic due to using a misbehaving runtime 
> dir), but in general that sounds like a good idea (at least on Linux 
> systems). Doesn't solve the SELinux (and presumably AppArmor) problem 
> though...
> 
> (I guess I need to try something like
> 
> 1) start a guest with "sudo $user virsh start blah"
> 2) restart the host
>     [we now have a $HOME/.cache populated with status files]
> 3) again use sudo $user to try several virsh commands that
>     would get info from status files to see if anything fails.
> 
> )

An example of the opposite that I tried - if I login as $someuser and 
"virsh start blah", then separately (without rebooting the host) as a 
different user I run "sudo -u $someuser virsh list", I will get back an 
empty list (because 1) for some reason (also probably related to lack of 
environment) a 2nd virtqemud process is started, and 2) the new instance 
of virtqemud doesn't have XDG_RUNTIME_DIR set, so it's looking in 
$HOME/.cache)

> 
> Fortunately this is only an issue for unprivileged libvirt, and most of 
> the drivers don't work unprivileged anyway (right? the only other driver 
> I've ever paid attention wrt to this was the network driver).
Re: [PATCH RFC] util: pick a better runtime directory when XDG_RUNTIME_DIR isn't set
Posted by Martin Kletzander 10 months ago
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

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

>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
>
>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.
>
>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@redhat.com>
>---
> 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
>
Re: [PATCH RFC] util: pick a better runtime directory when XDG_RUNTIME_DIR isn't set
Posted by Laine Stump 10 months ago
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@redhat.com>
>> ---
>> 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
>>