[PATCH 1/2] util: avoid free() when reset log after fork
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(a)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