[PATCH 6/8] util: Check for errors in virLogSetFromEnv
Martin Kletzander
mkletzan at redhat.com
Tue Jan 4 13:47:10 UTC 2022
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. Logging settings are also something that we want to report
errors on for example when it is being done over admin API.
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:
...
<deadlock>
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 <mkletzan at redhat.com>
---
src/admin/libvirt-admin.c | 3 ++-
src/libvirt.c | 3 ++-
src/lxc/lxc_controller.c | 3 ++-
src/remote/remote_ssh_helper.c | 3 ++-
src/security/virt-aa-helper.c | 3 ++-
src/util/vircommand.c | 3 ++-
src/util/virdaemon.c | 3 ++-
src/util/virlog.c | 26 ++++++++++++++++++--------
src/util/virlog.h | 2 +-
tests/testutils.c | 4 +++-
10 files changed, 36 insertions(+), 17 deletions(-)
diff --git a/src/admin/libvirt-admin.c b/src/admin/libvirt-admin.c
index a3164a2395cf..01546a7bc270 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 ef9fc403d083..35fd74fe08da 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 67ebed361ae0..3c930eaacdee 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 4f4dbff7d0ad..78f02020a745 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 b7ffb5e2c324..28717b7e38b8 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 ba5a2090768b..3b8c1c65c160 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 795206301227..5c0ca534cf16 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 5848940c6ccb..71913a2997f4 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 a04811e4083c..4f755c543b78 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 65f02e023199..322c3b94004c 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,
--
2.34.1
More information about the libvir-list
mailing list