From e17f291d75190db025128bb7a96d2cd0c7926063 Mon Sep 17 00:00:00 2001 From: "Andrew J. Hesford" Date: Fri, 1 Jan 2021 22:00:34 -0500 Subject: [PATCH] libvirt: fix deadlocks with musl Alpine Linux includes two patches to fix deadlocks with musl. Because upstream is not interested in merging these patches, conditionally apply one only for musl archs. The other is guarded by conditional compilation directives, so there is no harm in applying the patch unconditionally. Closes #25905. --- srcpkgs/libvirt/files/musl-fork-nofree.patch | 137 ++++++++++++++++++ .../improve-generic-mass-close-of-fds.patch | 131 +++++++++++++++++ srcpkgs/libvirt/template | 10 +- 3 files changed, 277 insertions(+), 1 deletion(-) create mode 100644 srcpkgs/libvirt/files/musl-fork-nofree.patch create mode 100644 srcpkgs/libvirt/patches/improve-generic-mass-close-of-fds.patch diff --git a/srcpkgs/libvirt/files/musl-fork-nofree.patch b/srcpkgs/libvirt/files/musl-fork-nofree.patch new file mode 100644 index 00000000000..4e680840311 --- /dev/null +++ b/srcpkgs/libvirt/files/musl-fork-nofree.patch @@ -0,0 +1,137 @@ +https://www.redhat.com/archives/libvir-list/2020-August/msg00596.html + +Doing malloc/free after fork is technically 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 -ur src/util/vircommand.c src/util/vircommand.c +--- src/util/vircommand.c ++++ src/util/vircommand.c +@@ -304,7 +304,7 @@ + /* 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 +@@ -897,7 +897,7 @@ + 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 -ur src/util/virlog.c src/util/virlog.c +--- src/util/virlog.c ++++ src/util/virlog.c +@@ -108,8 +108,8 @@ + */ + 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 @@ + 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 @@ + * 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 @@ + * 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; + } +@@ -1379,7 +1403,7 @@ + return -1; + + virLogLock(); +- virLogResetOutputs(); ++ virLogResetOutputs(true); + + #if WITH_SYSLOG_H + /* syslog needs to be special-cased, since it keeps the fd in private */ +@@ -1422,7 +1446,7 @@ + return -1; + + virLogLock(); +- virLogResetFilters(); ++ virLogResetFilters(true); + virLogFilters = filters; + virLogNbFilters = nfilters; + virLogUnlock(); +diff -ur src/util/virlog.h src/util/virlog.h +--- src/util/virlog.h ++++ src/util/virlog.h +@@ -168,6 +168,7 @@ + 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, 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..6647588f3f1 --- /dev/null +++ b/srcpkgs/libvirt/patches/improve-generic-mass-close-of-fds.patch @@ -0,0 +1,131 @@ +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 -ur src/util/vircommand.c src/util/vircommand.c +--- src/util/vircommand.c ++++ src/util/vircommand.c +@@ -443,7 +443,7 @@ + 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 +@@ -478,17 +478,7 @@ + + return 0; + } +- +-# else /* !__linux__ */ +- +-static int +-virCommandMassCloseGetFDsGeneric(virCommandPtr cmd G_GNUC_UNUSED, +- virBitmapPtr fds) +-{ +- virBitmapSetAll(fds); +- return 0; +-} +-# endif /* !__linux__ */ ++# endif /* __linux__ && __GLIBC__ */ + + # ifdef __FreeBSD__ + +@@ -542,7 +532,7 @@ + return 0; + } + +-# else /* ! __FreeBSD__ */ ++# elif defined(__GLIBC__) /* ! __FreeBSD__ */ + + static int + virCommandMassClose(virCommandPtr cmd, +@@ -569,13 +559,8 @@ + + fds = virBitmapNew(openmax); + +-# 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)) { +@@ -593,6 +578,61 @@ + 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__ */ + + /* diff --git a/srcpkgs/libvirt/template b/srcpkgs/libvirt/template index 4e87007c5ea..09255adbda3 100644 --- a/srcpkgs/libvirt/template +++ b/srcpkgs/libvirt/template @@ -1,7 +1,7 @@ # Template file for 'libvirt' pkgname=libvirt version=6.10.0 -revision=1 +revision=2 build_style=meson configure_args="-Dqemu_user=libvirt -Dqemu_group=libvirt -Drunstatedir=/run" hostmakedepends="automake libtool perl pkg-config lvm2 parted gettext-devel @@ -62,6 +62,14 @@ make_dirs=" /var/libvirt/boot 0755 root root /var/cache/libvirt/qemu 0755 root root" +post_patch() { + # Upstream doesn't want to accept this patch from Alpine, but libvirt + # is broken on musl without it, so apply it only for musl + if [ "$XBPS_TARGET_LIBC" = "musl" ]; then + patch -Np0 < ${FILESDIR}/musl-fork-nofree.patch + fi +} + post_install() { # runit services vsv libvirtd