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.
This commit is contained in:
Andrew J. Hesford 2021-01-01 22:00:34 -05:00
parent f0aa9b299b
commit e17f291d75
3 changed files with 277 additions and 1 deletions

View file

@ -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 <ncopa 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 -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,

View file

@ -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 <ncopa alpinelinux org>
---
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__ */
/*

View file

@ -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