From: Martin Kletzander Date: Wed, 15 Dec 2021 15:34:16 +0000 (+0100) Subject: util: Check for errors in virLogSetFromEnv X-Git-Tag: v8.0.0-rc1~47 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=9f6749dea0e3638bfea2b4e429b7dbba200331f3;p=thirdparty%2Flibvirt.git util: Check for errors in virLogSetFromEnv And make callers check the return value as well. This helps error out early for invalid environment variables. That is desirable because it could lead to deadlocks. This can happen when resetting logging after fork() reports translated errors because gettext functions are not reentrant. Well, it is not limited to resetting logging after fork(), it can be any translation at that phase, but parsing environment variables is easy to make fail on purpose to show the result, it can also happen just due to a typo. Before this commit it is possible to deadlock the daemon on startup with something like: LIBVIRT_LOG_FILTERS='1:*' LIBVIRT_LOG_OUTPUTS=1:stdout libvirtd where filters are used to enable more logging and hence make the race less rare and outputs are set to invalid Combined with the previous patches this changes the following from: ... to: ... libvirtd: initialisation failed The error message is improved in future commits and is also possible thanks to this patch. Signed-off-by: Martin Kletzander Reviewed-by: Erik Skultety --- diff --git a/src/admin/libvirt-admin.c b/src/admin/libvirt-admin.c index a3164a2395..01546a7bc2 100644 --- a/src/admin/libvirt-admin.c +++ b/src/admin/libvirt-admin.c @@ -55,7 +55,8 @@ virAdmGlobalInit(void) if (virErrorInitialize() < 0) goto error; - virLogSetFromEnv(); + if (virLogSetFromEnv() < 0) + goto error; #ifdef WITH_LIBINTL_H if (!bindtextdomain(PACKAGE, LOCALEDIR)) diff --git a/src/libvirt.c b/src/libvirt.c index ef9fc403d0..35fd74fe08 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -232,7 +232,8 @@ virGlobalInit(void) goto error; } - virLogSetFromEnv(); + if (virLogSetFromEnv() < 0) + goto error; virNetTLSInit(); diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c index 67ebed361a..3c930eaacd 100644 --- a/src/lxc/lxc_controller.c +++ b/src/lxc/lxc_controller.c @@ -2503,7 +2503,8 @@ int main(int argc, char *argv[]) } /* Initialize logging */ - virLogSetFromEnv(); + if (virLogSetFromEnv() < 0) + exit(EXIT_FAILURE); while (1) { int c; diff --git a/src/remote/remote_ssh_helper.c b/src/remote/remote_ssh_helper.c index 4f4dbff7d0..78f02020a7 100644 --- a/src/remote/remote_ssh_helper.c +++ b/src/remote/remote_ssh_helper.c @@ -402,7 +402,8 @@ int main(int argc, char **argv) virFileActivateDirOverrideForProg(argv[0]); /* Initialize the log system */ - virLogSetFromEnv(); + if (virLogSetFromEnv() < 0) + exit(EXIT_FAILURE); uri_str = argv[1]; VIR_DEBUG("Using URI %s", uri_str); diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c index b7ffb5e2c3..28717b7e38 100644 --- a/src/security/virt-aa-helper.c +++ b/src/security/virt-aa-helper.c @@ -1449,7 +1449,8 @@ main(int argc, char **argv) virFileActivateDirOverrideForProg(argv[0]); /* Initialize the log system */ - virLogSetFromEnv(); + if (virLogSetFromEnv() < 0) + exit(EXIT_FAILURE); /* clear the environment */ environ = NULL; diff --git a/src/util/vircommand.c b/src/util/vircommand.c index ba5a209076..3b8c1c65c1 100644 --- a/src/util/vircommand.c +++ b/src/util/vircommand.c @@ -751,7 +751,8 @@ virExec(virCommand *cmd) VIR_FORCE_CLOSE(null); /* Initialize full logging for a while */ - virLogSetFromEnv(); + if (virLogSetFromEnv() < 0) + goto fork_error; if (cmd->pidfile && virPipe(pipesync) < 0) diff --git a/src/util/virdaemon.c b/src/util/virdaemon.c index 7952063012..5c0ca534cf 100644 --- a/src/util/virdaemon.c +++ b/src/util/virdaemon.c @@ -183,7 +183,8 @@ virDaemonSetupLogging(const char *daemon_name, return -1; /* If there are some environment variables defined, use those instead */ - virLogSetFromEnv(); + if (virLogSetFromEnv() < 0) + return -1; /* * Command line override for --verbose diff --git a/src/util/virlog.c b/src/util/virlog.c index 5848940c6c..139dce8d02 100644 --- a/src/util/virlog.c +++ b/src/util/virlog.c @@ -1200,24 +1200,34 @@ virLogParseDefaultPriority(const char *priority) * * Sets virLogDefaultPriority, virLogFilters and virLogOutputs based on * environment variables. + * + * This function might fail which should also make the caller fail. */ -void +int virLogSetFromEnv(void) { const char *debugEnv; if (virLogInitialize() < 0) - return; + return -1; debugEnv = getenv("LIBVIRT_DEBUG"); - if (debugEnv && *debugEnv) - virLogSetDefaultPriority(virLogParseDefaultPriority(debugEnv)); + if (debugEnv && *debugEnv) { + int priority = virLogParseDefaultPriority(debugEnv); + if (priority < 0 || + virLogSetDefaultPriority(priority) < 0) + return -1; + } debugEnv = getenv("LIBVIRT_LOG_FILTERS"); - if (debugEnv && *debugEnv) - virLogSetFilters(debugEnv); + if (debugEnv && *debugEnv && + virLogSetFilters(debugEnv)) + return -1; debugEnv = getenv("LIBVIRT_LOG_OUTPUTS"); - if (debugEnv && *debugEnv) - virLogSetOutputs(debugEnv); + if (debugEnv && *debugEnv && + virLogSetOutputs(debugEnv)) + return -1; + + return 0; } diff --git a/src/util/virlog.h b/src/util/virlog.h index a04811e408..4f755c543b 100644 --- a/src/util/virlog.h +++ b/src/util/virlog.h @@ -146,7 +146,7 @@ char *virLogGetFilters(void); char *virLogGetOutputs(void); virLogPriority virLogGetDefaultPriority(void); int virLogSetDefaultPriority(virLogPriority priority); -void virLogSetFromEnv(void); +int virLogSetFromEnv(void) G_GNUC_WARN_UNUSED_RESULT; void virLogOutputFree(virLogOutput *output); void virLogOutputListFree(virLogOutput **list, int count); void virLogFilterFree(virLogFilter *filter); diff --git a/tests/testutils.c b/tests/testutils.c index 65f02e0231..322c3b9400 100644 --- a/tests/testutils.c +++ b/tests/testutils.c @@ -835,7 +835,9 @@ int virTestMain(int argc, if (virErrorInitialize() < 0) return EXIT_FAILURE; - virLogSetFromEnv(); + if (virLogSetFromEnv() < 0) + return EXIT_FAILURE; + if (!getenv("LIBVIRT_DEBUG") && !virLogGetNbOutputs()) { if (!(output = virLogOutputNew(virtTestLogOutput, virtTestLogClose, &testLog, VIR_LOG_DEBUG,