[PATCH 1/2] util: avoid free() when reset log after fork

Natanael Copa ncopa at alpinelinux.org
Wed Aug 19 10:03:40 UTC 2020


Doing malloc/free after fork is techincally not allowed in POSIX and
deadlocks[1] with musl libc.

[1]: https://gitlab.com/libvirt/libvirt/-/issues/52

Signed-off-by: Natanael Copa <ncopa at alpinelinux.org>
---
 src/util/vircommand.c |  4 ++--
 src/util/virlog.c     | 44 +++++++++++++++++++++++++++++++++----------
 src/util/virlog.h     |  1 +
 3 files changed, 37 insertions(+), 12 deletions(-)

diff --git a/src/util/vircommand.c b/src/util/vircommand.c
index 76f7eb9a3d..17e5bb00d3 100644
--- a/src/util/vircommand.c
+++ b/src/util/vircommand.c
@@ -304,7 +304,7 @@ virFork(void)
         /* Make sure any hook logging is sent to stderr, since child
          * process may close the logfile FDs */
         logprio = virLogGetDefaultPriority();
-        virLogReset();
+        virLogResetWithoutFree();
         virLogSetDefaultPriority(logprio);
 
         /* Clear out all signal handlers from parent so nothing
@@ -861,7 +861,7 @@ virExec(virCommandPtr cmd)
        goto fork_error;
 
     /* Close logging again to ensure no FDs leak to child */
-    virLogReset();
+    virLogResetWithoutFree();
 
     if (cmd->env)
         execve(binary, cmd->args, cmd->env);
diff --git a/src/util/virlog.c b/src/util/virlog.c
index 3217e5eb73..3959de5ca7 100644
--- a/src/util/virlog.c
+++ b/src/util/virlog.c
@@ -108,8 +108,8 @@ static size_t virLogNbOutputs;
  */
 static virLogPriority virLogDefaultPriority = VIR_LOG_DEFAULT;
 
-static void virLogResetFilters(void);
-static void virLogResetOutputs(void);
+static void virLogResetFilters(bool freemem);
+static void virLogResetOutputs(bool freemem);
 static void virLogOutputToFd(virLogSourcePtr src,
                              virLogPriority priority,
                              const char *filename,
@@ -284,8 +284,30 @@ virLogReset(void)
         return -1;
 
     virLogLock();
-    virLogResetFilters();
-    virLogResetOutputs();
+    virLogResetFilters(true);
+    virLogResetOutputs(true);
+    virLogDefaultPriority = VIR_LOG_DEFAULT;
+    virLogUnlock();
+    return 0;
+}
+
+/**
+ * virLogResetWithoutFree:
+ *
+ * Reset the logging module to its default initial state, but avoid doing
+ * free() so it can be used after fork and before exec.
+ *
+ * Returns 0 if successful, and -1 in case or error
+ */
+int
+virLogResetWithoutFree(void)
+{
+    if (virLogInitialize() < 0)
+        return -1;
+
+    virLogLock();
+    virLogResetFilters(false);
+    virLogResetOutputs(false);
     virLogDefaultPriority = VIR_LOG_DEFAULT;
     virLogUnlock();
     return 0;
@@ -324,9 +346,10 @@ virLogSetDefaultPriority(virLogPriority priority)
  * Removes the set of logging filters defined.
  */
 static void
-virLogResetFilters(void)
+virLogResetFilters(bool freemem)
 {
-    virLogFilterListFree(virLogFilters, virLogNbFilters);
+    if (freemem)
+        virLogFilterListFree(virLogFilters, virLogNbFilters);
     virLogFilters = NULL;
     virLogNbFilters = 0;
     virLogFiltersSerial++;
@@ -371,9 +394,10 @@ virLogFilterListFree(virLogFilterPtr *list, int count)
  * Removes the set of logging output defined.
  */
 static void
-virLogResetOutputs(void)
+virLogResetOutputs(bool freemem)
 {
-    virLogOutputListFree(virLogOutputs, virLogNbOutputs);
+    if (freemem)
+        virLogOutputListFree(virLogOutputs, virLogNbOutputs);
     virLogOutputs = NULL;
     virLogNbOutputs = 0;
 }
@@ -1392,7 +1416,7 @@ virLogDefineOutputs(virLogOutputPtr *outputs, size_t noutputs)
         return -1;
 
     virLogLock();
-    virLogResetOutputs();
+    virLogResetOutputs(true);
 
 #if HAVE_SYSLOG_H
     /* syslog needs to be special-cased, since it keeps the fd in private */
@@ -1435,7 +1459,7 @@ virLogDefineFilters(virLogFilterPtr *filters, size_t nfilters)
         return -1;
 
     virLogLock();
-    virLogResetFilters();
+    virLogResetFilters(true);
     virLogFilters = filters;
     virLogNbFilters = nfilters;
     virLogUnlock();
diff --git a/src/util/virlog.h b/src/util/virlog.h
index 984a9d5a43..69f7b1ef94 100644
--- a/src/util/virlog.h
+++ b/src/util/virlog.h
@@ -168,6 +168,7 @@ void virLogSetDefaultOutput(const char *fname, bool godaemon, bool privileged);
 void virLogLock(void);
 void virLogUnlock(void);
 int virLogReset(void);
+int virLogResetWithoutFree(void);
 int virLogParseDefaultPriority(const char *priority);
 int virLogPriorityFromSyslog(int priority);
 void virLogMessage(virLogSourcePtr source,
-- 
2.28.0





More information about the libvir-list mailing list