[PATCH 0/4] softmmu: move and refactor -runas, -chroot and -daemonize
This small series was motivated by my thoughts on the proposals in https://lists.gnu.org/archive/html/qemu-devel/2022-03/msg01135.html It demostrates the approach I mention there, and has the further benefit of untangling and isolating the implementation of UID changing, chrooting and daemonized, from the parsing of the corresponding command line options. Daniel P. Berrangé (4): softmmu: remove deprecated --enable-fips option os-posix: refactor code handling the -runas argument os-posix: refactor code handling the -chroot argument softmmu: move parsing of -runas, -chroot and -daemonize code docs/about/deprecated.rst | 12 -- docs/about/removed-features.rst | 11 ++ include/qemu/osdep.h | 3 - include/sysemu/os-posix.h | 4 +- include/sysemu/os-win32.h | 1 - os-posix.c | 222 ++++++++++---------------------- os-win32.c | 9 -- qemu-options.hx | 10 -- softmmu/vl.c | 76 ++++++++++- ui/vnc.c | 7 - util/osdep.c | 28 ---- 11 files changed, 154 insertions(+), 229 deletions(-) -- 2.34.1
Users requiring FIPS support must build QEMU with either the libgcrypt or gnutls libraries for as the crytography backend. Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- docs/about/deprecated.rst | 12 ------------ docs/about/removed-features.rst | 11 +++++++++++ include/qemu/osdep.h | 3 --- os-posix.c | 8 -------- qemu-options.hx | 10 ---------- ui/vnc.c | 7 ------- util/osdep.c | 28 ---------------------------- 7 files changed, 11 insertions(+), 68 deletions(-) diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst index 26d00812ba..a458dd453c 100644 --- a/docs/about/deprecated.rst +++ b/docs/about/deprecated.rst @@ -67,18 +67,6 @@ and will cause a warning. The replacement for the ``nodelay`` short-form boolean option is ``nodelay=on`` rather than ``delay=off``. -``--enable-fips`` (since 6.0) -''''''''''''''''''''''''''''' - -This option restricts usage of certain cryptographic algorithms when -the host is operating in FIPS mode. - -If FIPS compliance is required, QEMU should be built with the ``libgcrypt`` -library enabled as a cryptography provider. - -Neither the ``nettle`` library, or the built-in cryptography provider are -supported on FIPS enabled hosts. - ``-writeconfig`` (since 6.0) ''''''''''''''''''''''''''''' diff --git a/docs/about/removed-features.rst b/docs/about/removed-features.rst index cb0575fd49..6ca66f658d 100644 --- a/docs/about/removed-features.rst +++ b/docs/about/removed-features.rst @@ -336,6 +336,17 @@ for the RISC-V ``virt`` machine and ``sifive_u`` machine. The ``-no-quit`` was a synonym for ``-display ...,window-close=off`` which should be used instead. +``--enable-fips`` (removed in 7.0) +'''''''''''''''''''''''''''''''''' + +This option restricted usage of certain cryptographic algorithms when +the host is operating in FIPS mode. + +If FIPS compliance is required, QEMU should be built with the ``libgcrypt`` +or ``gnutls`` library enabled as a cryptography provider. + +Neither the ``nettle`` library, or the built-in cryptography provider are +supported on FIPS enabled hosts. QEMU Machine Protocol (QMP) commands ------------------------------------ diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h index 7bcce3bceb..66e70e24ff 100644 --- a/include/qemu/osdep.h +++ b/include/qemu/osdep.h @@ -534,9 +534,6 @@ static inline void qemu_timersub(const struct timeval *val1, void qemu_set_cloexec(int fd); -void fips_set_state(bool requested); -bool fips_get_state(void); - /* Return a dynamically allocated pathname denoting a file or directory that is * appropriate for storing local state. * diff --git a/os-posix.c b/os-posix.c index ae6c9f2a5e..7cd662098e 100644 --- a/os-posix.c +++ b/os-posix.c @@ -151,14 +151,6 @@ int os_parse_cmd_args(int index, const char *optarg) case QEMU_OPTION_daemonize: daemonize = 1; break; -#if defined(CONFIG_LINUX) - case QEMU_OPTION_enablefips: - warn_report("-enable-fips is deprecated, please build QEMU with " - "the `libgcrypt` library as the cryptography provider " - "to enable FIPS compliance"); - fips_set_state(true); - break; -#endif default: return -1; } diff --git a/qemu-options.hx b/qemu-options.hx index 094a6c1d7c..cb0c58904b 100644 --- a/qemu-options.hx +++ b/qemu-options.hx @@ -4655,16 +4655,6 @@ HXCOMM Internal use DEF("qtest", HAS_ARG, QEMU_OPTION_qtest, "", QEMU_ARCH_ALL) DEF("qtest-log", HAS_ARG, QEMU_OPTION_qtest_log, "", QEMU_ARCH_ALL) -#ifdef __linux__ -DEF("enable-fips", 0, QEMU_OPTION_enablefips, - "-enable-fips enable FIPS 140-2 compliance\n", - QEMU_ARCH_ALL) -#endif -SRST -``-enable-fips`` - Enable FIPS 140-2 compliance mode. -ERST - DEF("msg", HAS_ARG, QEMU_OPTION_msg, "-msg [timestamp[=on|off]][,guest-name=[on|off]]\n" " control error message format\n" diff --git a/ui/vnc.c b/ui/vnc.c index 3ccd33dedc..82b28aec95 100644 --- a/ui/vnc.c +++ b/ui/vnc.c @@ -4051,13 +4051,6 @@ void vnc_display_open(const char *id, Error **errp) password = qemu_opt_get_bool(opts, "password", false); } if (password) { - if (fips_get_state()) { - error_setg(errp, - "VNC password auth disabled due to FIPS mode, " - "consider using the VeNCrypt or SASL authentication " - "methods as an alternative"); - goto fail; - } if (!qcrypto_cipher_supports( QCRYPTO_CIPHER_ALG_DES, QCRYPTO_CIPHER_MODE_ECB)) { error_setg(errp, diff --git a/util/osdep.c b/util/osdep.c index 723cdcb004..456df9e81a 100644 --- a/util/osdep.c +++ b/util/osdep.c @@ -43,8 +43,6 @@ extern int madvise(char *, size_t, int); #include "qemu/hw-version.h" #include "monitor/monitor.h" -static bool fips_enabled = false; - static const char *hw_version = QEMU_HW_VERSION; int socket_set_cork(int fd, int v) @@ -526,32 +524,6 @@ const char *qemu_hw_version(void) return hw_version; } -void fips_set_state(bool requested) -{ -#ifdef __linux__ - if (requested) { - FILE *fds = fopen("/proc/sys/crypto/fips_enabled", "r"); - if (fds != NULL) { - fips_enabled = (fgetc(fds) == '1'); - fclose(fds); - } - } -#else - fips_enabled = false; -#endif /* __linux__ */ - -#ifdef _FIPS_DEBUG - fprintf(stderr, "FIPS mode %s (requested %s)\n", - (fips_enabled ? "enabled" : "disabled"), - (requested ? "enabled" : "disabled")); -#endif -} - -bool fips_get_state(void) -{ - return fips_enabled; -} - #ifdef _WIN32 static void socket_cleanup(void) { -- 2.34.1
On 4/3/22 12:56, Daniel P. Berrangé wrote:
Users requiring FIPS support must build QEMU with either the libgcrypt or gnutls libraries for as the crytography backend.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- docs/about/deprecated.rst | 12 ------------ docs/about/removed-features.rst | 11 +++++++++++ include/qemu/osdep.h | 3 --- os-posix.c | 8 -------- qemu-options.hx | 10 ---------- ui/vnc.c | 7 ------- util/osdep.c | 28 ---------------------------- 7 files changed, 11 insertions(+), 68 deletions(-)
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
On Fri, Mar 04, 2022 at 11:56:54AM +0000, Daniel P. Berrangé wrote:
Users requiring FIPS support must build QEMU with either the libgcrypt or gnutls libraries for as the crytography backend.
s/for //
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- docs/about/deprecated.rst | 12 ------------ docs/about/removed-features.rst | 11 +++++++++++ include/qemu/osdep.h | 3 --- os-posix.c | 8 -------- qemu-options.hx | 10 ---------- ui/vnc.c | 7 ------- util/osdep.c | 28 ---------------------------- 7 files changed, 11 insertions(+), 68 deletions(-)
-- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
Change the change_process_uid() function so that it takes its input as parameters instead of relying on static global variables. Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- os-posix.c | 83 +++++++++++++++++++++++++----------------------------- 1 file changed, 39 insertions(+), 44 deletions(-) diff --git a/os-posix.c b/os-posix.c index 7cd662098e..5a127feee2 100644 --- a/os-posix.c +++ b/os-posix.c @@ -42,13 +42,9 @@ #include <sys/prctl.h> #endif -/* - * Must set all three of these at once. - * Legal combinations are unset by name by uid - */ -static struct passwd *user_pwd; /* NULL non-NULL NULL */ -static uid_t user_uid = (uid_t)-1; /* -1 -1 >=0 */ -static gid_t user_gid = (gid_t)-1; /* -1 -1 >=0 */ +static char *user_name; +static uid_t user_uid = (uid_t)-1; +static gid_t user_gid = (gid_t)-1; static const char *chroot_dir; static int daemonize; @@ -100,7 +96,8 @@ void os_set_proc_name(const char *s) } -static bool os_parse_runas_uid_gid(const char *optarg) +static bool os_parse_runas_uid_gid(const char *optarg, + uid_t *runas_uid, gid_t *runas_gid) { unsigned long lv; const char *ep; @@ -120,9 +117,8 @@ static bool os_parse_runas_uid_gid(const char *optarg) return false; } - user_pwd = NULL; - user_uid = got_uid; - user_gid = got_gid; + *runas_uid = got_uid; + *runas_gid = got_gid; return true; } @@ -132,13 +128,18 @@ static bool os_parse_runas_uid_gid(const char *optarg) */ int os_parse_cmd_args(int index, const char *optarg) { + struct passwd *user_pwd; + switch (index) { case QEMU_OPTION_runas: user_pwd = getpwnam(optarg); if (user_pwd) { - user_uid = -1; - user_gid = -1; - } else if (!os_parse_runas_uid_gid(optarg)) { + user_uid = user_pwd->pw_uid; + user_gid = user_pwd->pw_gid; + user_name = g_strdup(user_pwd->pw_name); + } else if (!os_parse_runas_uid_gid(optarg, + &user_uid, + &user_gid)) { error_report("User \"%s\" doesn't exist" " (and is not <uid>:<gid>)", optarg); @@ -158,41 +159,33 @@ int os_parse_cmd_args(int index, const char *optarg) return 0; } -static void change_process_uid(void) +static void change_process_uid(uid_t uid, gid_t gid, const char *name) { - assert((user_uid == (uid_t)-1) || user_pwd == NULL); - assert((user_uid == (uid_t)-1) == - (user_gid == (gid_t)-1)); - - if (user_pwd || user_uid != (uid_t)-1) { - gid_t intended_gid = user_pwd ? user_pwd->pw_gid : user_gid; - uid_t intended_uid = user_pwd ? user_pwd->pw_uid : user_uid; - if (setgid(intended_gid) < 0) { - error_report("Failed to setgid(%d)", intended_gid); - exit(1); - } - if (user_pwd) { - if (initgroups(user_pwd->pw_name, user_pwd->pw_gid) < 0) { - error_report("Failed to initgroups(\"%s\", %d)", - user_pwd->pw_name, user_pwd->pw_gid); - exit(1); - } - } else { - if (setgroups(1, &user_gid) < 0) { - error_report("Failed to setgroups(1, [%d])", - user_gid); - exit(1); - } - } - if (setuid(intended_uid) < 0) { - error_report("Failed to setuid(%d)", intended_uid); + if (setgid(gid) < 0) { + error_report("Failed to setgid(%d)", gid); + exit(1); + } + if (name) { + if (initgroups(name, gid) < 0) { + error_report("Failed to initgroups(\"%s\", %d)", + name, gid); exit(1); } - if (setuid(0) != -1) { - error_report("Dropping privileges failed"); + } else { + if (setgroups(1, &gid) < 0) { + error_report("Failed to setgroups(1, [%d])", + gid); exit(1); } } + if (setuid(uid) < 0) { + error_report("Failed to setuid(%d)", uid); + exit(1); + } + if (setuid(0) != -1) { + error_report("Dropping privileges failed"); + exit(1); + } } static void change_root(void) @@ -275,7 +268,9 @@ void os_setup_post(void) } change_root(); - change_process_uid(); + if (user_uid != -1 && user_gid != -1) { + change_process_uid(user_uid, user_gid, user_name); + } if (daemonize) { uint8_t status = 0; -- 2.34.1
On Fri, Mar 04, 2022 at 11:56:55AM +0000, Daniel P. Berrangé wrote:
Change the change_process_uid() function so that it takes its input as parameters instead of relying on static global variables.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- os-posix.c | 83 +++++++++++++++++++++++++----------------------------- 1 file changed, 39 insertions(+), 44 deletions(-)
Reviewed-by: Eric Blake <eblake@redhat.com> -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
Change the change_root() function so that it takes its input as parameters instead of relying on static global variables. Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- os-posix.c | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/os-posix.c b/os-posix.c index 5a127feee2..30da1a1491 100644 --- a/os-posix.c +++ b/os-posix.c @@ -188,19 +188,16 @@ static void change_process_uid(uid_t uid, gid_t gid, const char *name) } } -static void change_root(void) +static void change_root(const char *root) { - if (chroot_dir) { - if (chroot(chroot_dir) < 0) { - error_report("chroot failed"); - exit(1); - } - if (chdir("/")) { - error_report("not able to chdir to /: %s", strerror(errno)); - exit(1); - } + if (chroot(root) < 0) { + error_report("chroot failed"); + exit(1); + } + if (chdir("/")) { + error_report("not able to chdir to /: %s", strerror(errno)); + exit(1); } - } void os_daemonize(void) @@ -267,7 +264,9 @@ void os_setup_post(void) } } - change_root(); + if (chroot_dir) { + change_root(chroot_dir); + } if (user_uid != -1 && user_gid != -1) { change_process_uid(user_uid, user_gid, user_name); } -- 2.34.1
On 4/3/22 12:56, Daniel P. Berrangé wrote:
Change the change_root() function so that it takes its input as parameters instead of relying on static global variables.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- os-posix.c | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-)
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
With the future intent to try to move to a fully QAPI driven configuration system, we want to have any current command parsing well isolated from logic that applies the resulting configuration. We also don't want os-posix.c to contain code that is specific to the system emulators, as this file is linked to other binaries too. To satisfy these goals, we move parsing of the -runas, -chroot and -daemonize code out of the os-posix.c helper code, and pass the parsed data into APIs in os-posix.c. As a side benefit the 'os_daemonize()' function now lives upto to its name and will always daemonize, instead of using global state to decide to be a no-op sometimes. Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- include/sysemu/os-posix.h | 4 +- include/sysemu/os-win32.h | 1 - os-posix.c | 148 +++++++++++--------------------------- os-win32.c | 9 --- softmmu/vl.c | 76 ++++++++++++++++++-- 5 files changed, 113 insertions(+), 125 deletions(-) diff --git a/include/sysemu/os-posix.h b/include/sysemu/os-posix.h index 2edf33658a..390f3f8396 100644 --- a/include/sysemu/os-posix.h +++ b/include/sysemu/os-posix.h @@ -46,7 +46,9 @@ void os_set_line_buffering(void); void os_set_proc_name(const char *s); void os_setup_signal_handling(void); void os_daemonize(void); -void os_setup_post(void); +void os_setup_post(const char *chroot_dir, + uid_t uid, gid_t gid, + const char *username); int os_mlock(void); #define closesocket(s) close(s) diff --git a/include/sysemu/os-win32.h b/include/sysemu/os-win32.h index 43f569b5c2..4879f8731d 100644 --- a/include/sysemu/os-win32.h +++ b/include/sysemu/os-win32.h @@ -61,7 +61,6 @@ struct tm *localtime_r(const time_t *timep, struct tm *result); static inline void os_setup_signal_handling(void) {} static inline void os_daemonize(void) {} -static inline void os_setup_post(void) {} void os_set_line_buffering(void); static inline void os_set_proc_name(const char *dummy) {} diff --git a/os-posix.c b/os-posix.c index 30da1a1491..f598a52a4f 100644 --- a/os-posix.c +++ b/os-posix.c @@ -42,11 +42,6 @@ #include <sys/prctl.h> #endif -static char *user_name; -static uid_t user_uid = (uid_t)-1; -static gid_t user_gid = (gid_t)-1; - -static const char *chroot_dir; static int daemonize; static int daemon_pipe; @@ -96,69 +91,6 @@ void os_set_proc_name(const char *s) } -static bool os_parse_runas_uid_gid(const char *optarg, - uid_t *runas_uid, gid_t *runas_gid) -{ - unsigned long lv; - const char *ep; - uid_t got_uid; - gid_t got_gid; - int rc; - - rc = qemu_strtoul(optarg, &ep, 0, &lv); - got_uid = lv; /* overflow here is ID in C99 */ - if (rc || *ep != ':' || got_uid != lv || got_uid == (uid_t)-1) { - return false; - } - - rc = qemu_strtoul(ep + 1, 0, 0, &lv); - got_gid = lv; /* overflow here is ID in C99 */ - if (rc || got_gid != lv || got_gid == (gid_t)-1) { - return false; - } - - *runas_uid = got_uid; - *runas_gid = got_gid; - return true; -} - -/* - * Parse OS specific command line options. - * return 0 if option handled, -1 otherwise - */ -int os_parse_cmd_args(int index, const char *optarg) -{ - struct passwd *user_pwd; - - switch (index) { - case QEMU_OPTION_runas: - user_pwd = getpwnam(optarg); - if (user_pwd) { - user_uid = user_pwd->pw_uid; - user_gid = user_pwd->pw_gid; - user_name = g_strdup(user_pwd->pw_name); - } else if (!os_parse_runas_uid_gid(optarg, - &user_uid, - &user_gid)) { - error_report("User \"%s\" doesn't exist" - " (and is not <uid>:<gid>)", - optarg); - exit(1); - } - break; - case QEMU_OPTION_chroot: - chroot_dir = optarg; - break; - case QEMU_OPTION_daemonize: - daemonize = 1; - break; - default: - return -1; - } - - return 0; -} - static void change_process_uid(uid_t uid, gid_t gid, const char *name) { if (setgid(gid) < 0) { @@ -202,54 +134,56 @@ static void change_root(const char *root) void os_daemonize(void) { - if (daemonize) { - pid_t pid; - int fds[2]; + pid_t pid; + int fds[2]; - if (pipe(fds) == -1) { - exit(1); - } + if (pipe(fds) == -1) { + exit(1); + } - pid = fork(); - if (pid > 0) { - uint8_t status; - ssize_t len; + pid = fork(); + if (pid > 0) { + uint8_t status; + ssize_t len; - close(fds[1]); + close(fds[1]); - do { - len = read(fds[0], &status, 1); - } while (len < 0 && errno == EINTR); + do { + len = read(fds[0], &status, 1); + } while (len < 0 && errno == EINTR); - /* only exit successfully if our child actually wrote - * a one-byte zero to our pipe, upon successful init */ - exit(len == 1 && status == 0 ? 0 : 1); + /* only exit successfully if our child actually wrote + * a one-byte zero to our pipe, upon successful init */ + exit(len == 1 && status == 0 ? 0 : 1); - } else if (pid < 0) { - exit(1); - } + } else if (pid < 0) { + exit(1); + } - close(fds[0]); - daemon_pipe = fds[1]; - qemu_set_cloexec(daemon_pipe); + close(fds[0]); + daemon_pipe = fds[1]; + qemu_set_cloexec(daemon_pipe); - setsid(); + setsid(); - pid = fork(); - if (pid > 0) { + pid = fork(); + if (pid > 0) { exit(0); - } else if (pid < 0) { - exit(1); - } - umask(027); - - signal(SIGTSTP, SIG_IGN); - signal(SIGTTOU, SIG_IGN); - signal(SIGTTIN, SIG_IGN); + } else if (pid < 0) { + exit(1); } + umask(027); + + signal(SIGTSTP, SIG_IGN); + signal(SIGTTOU, SIG_IGN); + signal(SIGTTIN, SIG_IGN); + + daemonize = true; } -void os_setup_post(void) +void os_setup_post(const char *root_dir, + uid_t runas_uid, gid_t runas_gid, + const char *runas_name) { int fd = 0; @@ -264,11 +198,11 @@ void os_setup_post(void) } } - if (chroot_dir) { - change_root(chroot_dir); + if (root_dir != NULL) { + change_root(root_dir); } - if (user_uid != -1 && user_gid != -1) { - change_process_uid(user_uid, user_gid, user_name); + if (runas_uid != -1 && runas_gid != -1) { + change_process_uid(runas_uid, runas_gid, runas_name); } if (daemonize) { diff --git a/os-win32.c b/os-win32.c index e31c921983..6f21b57841 100644 --- a/os-win32.c +++ b/os-win32.c @@ -61,12 +61,3 @@ void os_set_line_buffering(void) setbuf(stdout, NULL); setbuf(stderr, NULL); } - -/* - * Parse OS specific command line options. - * return 0 if option handled, -1 otherwise - */ -int os_parse_cmd_args(int index, const char *optarg) -{ - return -1; -} diff --git a/softmmu/vl.c b/softmmu/vl.c index 1fe028800f..cdf27b6027 100644 --- a/softmmu/vl.c +++ b/softmmu/vl.c @@ -2602,11 +2602,13 @@ static void qemu_process_help_options(void) } } -static void qemu_maybe_daemonize(const char *pid_file) +static void qemu_maybe_daemonize(bool daemonize, const char *pid_file) { Error *err = NULL; - os_daemonize(); + if (daemonize) { + os_daemonize(); + } rcu_disable_atfork(); if (pid_file && !qemu_write_pidfile(pid_file, &err)) { @@ -2768,6 +2770,35 @@ void qmp_x_exit_preconfig(Error **errp) } } +#ifndef WIN32 +static bool os_parse_runas_uid_gid(const char *optarg, + uid_t *runas_uid, + gid_t *runas_gid) +{ + unsigned long lv; + const char *ep; + uid_t got_uid; + gid_t got_gid; + int rc; + + rc = qemu_strtoul(optarg, &ep, 0, &lv); + got_uid = lv; /* overflow here is ID in C99 */ + if (rc || *ep != ':' || got_uid != lv || got_uid == (uid_t)-1) { + return false; + } + + rc = qemu_strtoul(ep + 1, 0, 0, &lv); + got_gid = lv; /* overflow here is ID in C99 */ + if (rc || got_gid != lv || got_gid == (gid_t)-1) { + return false; + } + + *runas_gid = got_gid; + *runas_uid = got_uid; + return true; +} +#endif /* !WIN32 */ + void qemu_init(int argc, char **argv, char **envp) { QemuOpts *opts; @@ -2778,6 +2809,14 @@ void qemu_init(int argc, char **argv, char **envp) MachineClass *machine_class; bool userconfig = true; FILE *vmstate_dump_file = NULL; + bool daemonize = false; +#ifndef WIN32 + struct passwd *pwd; + uid_t runas_uid = -1; + gid_t runas_gid = -1; + g_autofree char *runas_name = NULL; + const char *chroot_dir = NULL; +#endif /* !WIN32 */ qemu_add_opts(&qemu_drive_opts); qemu_add_drive_opts(&qemu_legacy_drive_opts); @@ -3659,11 +3698,32 @@ void qemu_init(int argc, char **argv, char **envp) case QEMU_OPTION_nouserconfig: /* Nothing to be parsed here. Especially, do not error out below. */ break; - default: - if (os_parse_cmd_args(popt->index, optarg)) { - error_report("Option not supported in this build"); +#ifndef WIN32 + case QEMU_OPTION_runas: + pwd = getpwnam(optarg); + if (pwd) { + runas_uid = pwd->pw_uid; + runas_gid = pwd->pw_gid; + runas_name = g_strdup(pwd->pw_name); + } else if (!os_parse_runas_uid_gid(optarg, + &runas_uid, + &runas_gid)) { + error_report("User \"%s\" doesn't exist" + " (and is not <uid>:<gid>)", + optarg); exit(1); } + break; + case QEMU_OPTION_chroot: + chroot_dir = optarg; + break; + case QEMU_OPTION_daemonize: + daemonize = 1; + break; +#endif /* !WIN32 */ + default: + error_report("Option not supported in this build"); + exit(1); } } } @@ -3683,7 +3743,7 @@ void qemu_init(int argc, char **argv, char **envp) qemu_process_early_options(); qemu_process_help_options(); - qemu_maybe_daemonize(pid_file); + qemu_maybe_daemonize(daemonize, pid_file); /* * The trace backend must be initialized after daemonizing. @@ -3778,6 +3838,8 @@ void qemu_init(int argc, char **argv, char **envp) } qemu_init_displays(); accel_setup_post(current_machine); - os_setup_post(); +#ifndef WIN32 + os_setup_post(chroot_dir, runas_uid, runas_gid, runas_name); +#endif /* !WIN32 */ resume_mux_open(); } -- 2.34.1
On Fri, Mar 04, 2022 at 11:56:57AM +0000, Daniel P. Berrangé wrote:
With the future intent to try to move to a fully QAPI driven configuration system, we want to have any current command parsing well isolated from logic that applies the resulting configuration.
We also don't want os-posix.c to contain code that is specific to the system emulators, as this file is linked to other binaries too.
To satisfy these goals, we move parsing of the -runas, -chroot and -daemonize code out of the os-posix.c helper code, and pass the parsed data into APIs in os-posix.c.
As a side benefit the 'os_daemonize()' function now lives upto to its name and will always daemonize, instead of using global state to decide to be a no-op sometimes.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- include/sysemu/os-posix.h | 4 +- include/sysemu/os-win32.h | 1 - os-posix.c | 148 +++++++++++--------------------------- os-win32.c | 9 --- softmmu/vl.c | 76 ++++++++++++++++++-- 5 files changed, 113 insertions(+), 125 deletions(-)
diff --git a/softmmu/vl.c b/softmmu/vl.c index 1fe028800f..cdf27b6027 100644 --- a/softmmu/vl.c +++ b/softmmu/vl.c @@ -2602,11 +2602,13 @@ static void qemu_process_help_options(void) } }
-static void qemu_maybe_daemonize(const char *pid_file) +static void qemu_maybe_daemonize(bool daemonize, const char *pid_file) { Error *err = NULL;
- os_daemonize(); + if (daemonize) { + os_daemonize(); + } rcu_disable_atfork();
if (pid_file && !qemu_write_pidfile(pid_file, &err)) {
@@ -3683,7 +3743,7 @@ void qemu_init(int argc, char **argv, char **envp) qemu_process_early_options();
qemu_process_help_options(); - qemu_maybe_daemonize(pid_file); + qemu_maybe_daemonize(daemonize, pid_file);
This commit is a bit flawed, because we're until we call the os_daemonize() method, the is_daemonized() method won't return true. Unfortunately some callers rely is_daemonized() returning true merely for the request, even though we've not yet put it into action. ie the method would have been better called is_daemonize_requested() The upshot is that we fail to properly close stderr. I'll send a v2 that handles this by fully removing the is_daemonize() method. 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 :|
On Fri, Mar 04, 2022 at 02:54:54PM +0000, Daniel P. Berrangé wrote:
On Fri, Mar 04, 2022 at 11:56:57AM +0000, Daniel P. Berrangé wrote:
With the future intent to try to move to a fully QAPI driven configuration system, we want to have any current command parsing well isolated from logic that applies the resulting configuration.
We also don't want os-posix.c to contain code that is specific to the system emulators, as this file is linked to other binaries too.
To satisfy these goals, we move parsing of the -runas, -chroot and -daemonize code out of the os-posix.c helper code, and pass the parsed data into APIs in os-posix.c.
As a side benefit the 'os_daemonize()' function now lives upto to
up to
its name and will always daemonize, instead of using global state to decide to be a no-op sometimes.
Yay.
@@ -3683,7 +3743,7 @@ void qemu_init(int argc, char **argv, char **envp) qemu_process_early_options();
qemu_process_help_options(); - qemu_maybe_daemonize(pid_file); + qemu_maybe_daemonize(daemonize, pid_file);
This commit is a bit flawed, because we're until we call the os_daemonize() method, the is_daemonized() method won't return true. Unfortunately some callers rely is_daemonized() returning true merely for the request, even though we've not yet put it into action. ie the method would have been better called is_daemonize_requested()
Eww, indeed.
The upshot is that we fail to properly close stderr.
I'll send a v2 that handles this by fully removing the is_daemonize() method.
Looking forward to it. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
participants (3)
-
Daniel P. Berrangé -
Eric Blake -
Philippe Mathieu-Daudé