From 20364ada6eca920f644c40aee10b9215b5a647c0 Mon Sep 17 00:00:00 2001 From: Kridsada Thanabulpong Date: Tue, 27 Oct 2020 07:37:50 +0900 Subject: [PATCH] libvirt: fix deadlock for musl --- ...avoid-free-when-reset-log-after-fork.patch | 142 ++++++++++++++++++ .../improve-generic-mass-close-of-fds.patch | 135 +++++++++++++++++ srcpkgs/libvirt/template | 2 +- 3 files changed, 278 insertions(+), 1 deletion(-) create mode 100644 srcpkgs/libvirt/patches/avoid-free-when-reset-log-after-fork.patch create mode 100644 srcpkgs/libvirt/patches/improve-generic-mass-close-of-fds.patch diff --git a/srcpkgs/libvirt/patches/avoid-free-when-reset-log-after-fork.patch b/srcpkgs/libvirt/patches/avoid-free-when-reset-log-after-fork.patch new file mode 100644 index 00000000000..06298c02aa6 --- /dev/null +++ b/srcpkgs/libvirt/patches/avoid-free-when-reset-log-after-fork.patch @@ -0,0 +1,142 @@ +https://www.redhat.com/archives/libvir-list/2020-August/msg00596.html + +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 +--- + 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 +--- src/util/vircommand.c ++++ 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 +--- src/util/virlog.c ++++ 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 +--- src/util/virlog.h ++++ 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 diff --git a/srcpkgs/libvirt/patches/improve-generic-mass-close-of-fds.patch b/srcpkgs/libvirt/patches/improve-generic-mass-close-of-fds.patch new file mode 100644 index 00000000000..bf2983401d7 --- /dev/null +++ b/srcpkgs/libvirt/patches/improve-generic-mass-close-of-fds.patch @@ -0,0 +1,135 @@ +https://www.redhat.com/archives/libvir-list/2020-August/msg00598.html + +Add a portable generic implementation of virMassClose as fallback on + +non-FreeBSD and non-glibc. + +This implementation uses poll(2) to look for open files to keep +performance reasonable while not using any mallocs. + +This solves a deadlock with musl libc. + +Signed-off-by: Natanael Copa +--- + src/util/vircommand.c | 76 +++++++++++++++++++++++++++++++++---------- + 1 file changed, 58 insertions(+), 18 deletions(-) + +diff --git a/src/util/vircommand.c b/src/util/vircommand.c +index 17e5bb00d3..06579cfb44 100644 +--- src/util/vircommand.c ++++ src/util/vircommand.c +@@ -443,7 +443,7 @@ virExecCommon(virCommandPtr cmd, gid_t *groups, int ngroups) + return 0; + } + +-# ifdef __linux__ ++# if defined(__linux__) && defined(__GLIBC__) + /* On Linux, we can utilize procfs and read the table of opened + * FDs and selectively close only those FDs we don't want to pass + * onto child process (well, the one we will exec soon since this +@@ -482,17 +482,7 @@ virCommandMassCloseGetFDsLinux(virCommandPtr cmd G_GNUC_UNUSED, + VIR_DIR_CLOSE(dp); + return ret; + } +- +-# else /* !__linux__ */ +- +-static int +-virCommandMassCloseGetFDsGeneric(virCommandPtr cmd G_GNUC_UNUSED, +- virBitmapPtr fds) +-{ +- virBitmapSetAll(fds); +- return 0; +-} +-# endif /* !__linux__ */ ++# endif /* __linux__ && __GLIBC__ */ + + # ifdef __FreeBSD__ + +@@ -546,7 +536,7 @@ virCommandMassClose(virCommandPtr cmd, + return 0; + } + +-# else /* ! __FreeBSD__ */ ++# elif defined(__GLIBC__) /* ! __FreeBSD__ */ + + static int + virCommandMassClose(virCommandPtr cmd, +@@ -574,13 +564,8 @@ virCommandMassClose(virCommandPtr cmd, + if (!(fds = virBitmapNew(openmax))) + return -1; + +-# ifdef __linux__ + if (virCommandMassCloseGetFDsLinux(cmd, fds) < 0) + return -1; +-# else +- if (virCommandMassCloseGetFDsGeneric(cmd, fds) < 0) +- return -1; +-# endif + + fd = virBitmapNextSetBit(fds, 2); + for (; fd >= 0; fd = virBitmapNextSetBit(fds, fd)) { +@@ -598,6 +583,61 @@ virCommandMassClose(virCommandPtr cmd, + return 0; + } + ++#else /* ! __FreeBSD__ && ! __GLIBC__ */ ++static int ++virCommandMassClose(virCommandPtr cmd, ++ int childin, ++ int childout, ++ int childerr) ++{ ++ static struct pollfd pfds[1024]; ++ int fd = 0; ++ int i, total; ++ int max_fd = sysconf(_SC_OPEN_MAX); ++ ++ if (max_fd < 0) { ++ virReportSystemError(errno, "%s", _("sysconf(_SC_OPEN_MAX) failed")); ++ return -1; ++ } ++ ++ total = max_fd - fd; ++ for (i = 0; i < (total < 1024 ? total : 1024); i++) ++ pfds[i].events = 0; ++ ++ while (fd < max_fd) { ++ int nfds, r = 0; ++ ++ total = max_fd - fd; ++ nfds = total < 1024 ? total : 1024; ++ ++ for (i = 0; i < nfds; i++) ++ pfds[i].fd = fd + i; ++ ++ do { ++ r = poll(pfds, nfds, 0); ++ } while (r == -1 && errno == EINTR); ++ ++ if (r < 0) { ++ virReportSystemError(errno, "%s", _("poll() failed")); ++ return -1; ++ } ++ ++ for (i = 0; i < nfds; i++) ++ if (pfds[i].revents != POLLNVAL) { ++ if (pfds[i].fd == childin || pfds[i].fd == childout || pfds[i].fd == childerr) ++ continue; ++ if (!virCommandFDIsSet(cmd, pfds[i].fd)) { ++ VIR_MASS_CLOSE(pfds[i].fd); ++ } else if (virSetInherit(pfds[i].fd, true) < 0) { ++ virReportSystemError(errno, _("failed to preserve fd %d"), pfds[i].fd); ++ return -1; ++ } ++ } ++ fd += nfds; ++ } ++ return 0; ++} ++ + # endif /* ! __FreeBSD__ */ + + /* +-- +22.28.0 diff --git a/srcpkgs/libvirt/template b/srcpkgs/libvirt/template index cc07eca3d0b..2f2d746c955 100644 --- a/srcpkgs/libvirt/template +++ b/srcpkgs/libvirt/template @@ -1,7 +1,7 @@ # Template file for 'libvirt' pkgname=libvirt version=6.6.0 -revision=4 +revision=5 build_wrksrc="build" build_style=gnu-configure configure_script="../configure"