On Tue, Jul 16, 2024 at 04:27:31PM +0900, Akihiko Odaki wrote:
> qemu_get_runtime_dir() returns a dynamically allocated directory path
> that is appropriate for storing runtime files. It corresponds to "run"
> directory in Unix.
>
> With a tree-wide search, it was found that there are several cases
> where such a functionality is implemented so let's have one as a common
> utlity function.
>
> A notable feature of qemu_get_runtime_dir() is that it uses
> $XDG_RUNTIME_DIR if available. While the function is often called by
> executables which requires root privileges, it is still possible that
> they are called from a user without privilege to write the system
> runtime directory. In fact, I decided to write this patch when I ran
> virtiofsd in a Linux namespace created by a normal user and realized
> it tries to write the system runtime directory, not writable in this
> case. $XDG_RUNTIME_DIR should provide a writable directory in such
> cases.
>
> This function does not use qemu_get_local_state_dir() or its logic
> for Windows. Actually the implementation of qemu_get_local_state_dir()
> for Windows seems not right as it calls g_get_system_data_dirs(),
> which refers to $XDG_DATA_DIRS. In Unix terminology, it is basically
> "/usr/share", not "/var", which qemu_get_local_state_dir() is intended
> to provide. Instead, this function try to use the following in order:
> - $XDG_RUNTIME_DIR
> - LocalAppData folder
> - get_relocated_path(CONFIG_QEMU_LOCALSTATEDIR "/run")
>
> This function does not use g_get_user_runtime_dir() either as it
> falls back to g_get_user_cache_dir() when $XDG_DATA_DIRS is not
> available. In the case, we rather use:
> get_relocated_path(CONFIG_QEMU_LOCALSTATEDIR "/run")
>
> Signed-off-by: Akihiko Odaki <akihiko.od...@daynix.com>
> Message-Id: <20230921075425.16738-2-akihiko.od...@daynix.com>
> ---
> include/qemu/osdep.h | 12 ++++++++++++
> util/oslib-posix.c | 11 +++++++++++
> util/oslib-win32.c | 26 ++++++++++++++++++++++++++
> 3 files changed, 49 insertions(+)
>
> diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
> index 191916f38e6d..fe8609fc1375 100644
> --- a/include/qemu/osdep.h
> +++ b/include/qemu/osdep.h
> @@ -670,6 +670,18 @@ void qemu_set_cloexec(int fd);
> */
> char *qemu_get_local_state_dir(void);
>
> +/**
> + * qemu_get_runtime_dir:
> + *
> + * Return a dynamically allocated directory path that is appropriate for
> storing
> + * runtime files. It corresponds to "run" directory in Unix, and uses
> + * $XDG_RUNTIME_DIR if available.
> + *
> + * The caller is responsible for releasing the value returned with g_free()
> + * after use.
> + */
> +char *qemu_get_runtime_dir(void);
> +
> /**
> * qemu_getauxval:
> * @type: the auxiliary vector key to lookup
> diff --git a/util/oslib-posix.c b/util/oslib-posix.c
> index e76441695bdc..9599509a9aa7 100644
> --- a/util/oslib-posix.c
> +++ b/util/oslib-posix.c
> @@ -278,6 +278,17 @@ qemu_get_local_state_dir(void)
> return get_relocated_path(CONFIG_QEMU_LOCALSTATEDIR);
> }
>
> +char *
> +qemu_get_runtime_dir(void)
> +{
> + char *env = getenv("XDG_RUNTIME_DIR");
> + if (env) {
> + return g_strdup(env);
> + }
> +
> + return get_relocated_path(CONFIG_QEMU_LOCALSTATEDIR "/run");
> +}
I'm not convinced this is the correct logic to be following.
In the cover letter you mention not using g_get_user_runtime_dir()
because it falls back to XDG_CACHE_HOME, and we need to fallback
to LOCALSTATEDIR/run. This is not right for normal users though,
where falling back to LOCALSTATEDIR/run is always wrong, as it
won't be writable - the g_get_user_runtime_dir() fallback is
desirable for non-root users.
IMHO we should be doing something more like this
#ifndef WIN32
if (geteuid() == 0) {
return get_relocated_path(CONFIG_QEMU_LOCALSTATEDIR "/run");
} else {
#endif
return g_get_user_runtime_dir();
#ifndef WIN32
}
#endif
> +
> void qemu_set_tty_echo(int fd, bool echo)
> {
> struct termios tty;
> diff --git a/util/oslib-win32.c b/util/oslib-win32.c
> index b623830d624f..8c5a02ee881d 100644
> --- a/util/oslib-win32.c
> +++ b/util/oslib-win32.c
> @@ -27,6 +27,8 @@
> */
>
> #include "qemu/osdep.h"
> +#include <shlobj.h>
> +#include <wchar.h>
> #include <windows.h>
> #include "qapi/error.h"
> #include "qemu/main-loop.h"
> @@ -237,6 +239,30 @@ qemu_get_local_state_dir(void)
> return g_strdup(data_dirs[0]);
> }
>
> +char *
> +qemu_get_runtime_dir(void)
> +{
> + size_t size = GetEnvironmentVariableA("XDG_RUNTIME_DIR", NULL, 0);
> + if (size) {
> + char *env = g_malloc(size);
> + GetEnvironmentVariableA("XDG_RUNTIME_DIR", env, size);
> + return env;
> + }
> +
> + PWSTR wpath;
> + const wchar_t *cwpath;
> + if (!SHGetKnownFolderPath(&FOLDERID_LocalAppData, KF_FLAG_DEFAULT, NULL,
> &wpath)) {
> + cwpath = wpath;
> + size = wcsrtombs(NULL, &cwpath, 0, &(mbstate_t){0}) + 1;
> + char *path = g_malloc(size);
> + wcsrtombs(path, &cwpath, size, &(mbstate_t){0});
> + CoTaskMemFree(wpath);
> + return path;
> + }
> +
> + return get_relocated_path(CONFIG_QEMU_LOCALSTATEDIR "/run");
> +}
> +
> void qemu_set_tty_echo(int fd, bool echo)
> {
> HANDLE handle = (HANDLE)_get_osfhandle(fd);
>
> --
> 2.45.2
>
>
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 :|
