From 520eb3e15bc87a43b37a6670b2f0f4e89039d6e3 Mon Sep 17 00:00:00 2001 From: Michal Privoznik Date: Tue, 22 Aug 2023 09:41:32 +0200 Subject: [PATCH] vircommand: Introduce virCommandMassCloseRange() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This is brand new way of closing FDs before exec(). We need to close all FDs except those we want to explicitly pass to avoid leaking FDs into the child. Historically, we've done this by either iterating over all opened FDs and closing them one by one (or preserving them), or by iterating over an FD interval [2 ... N] and closing them one by one followed by calling closefrom(N + 1). This is a lot of syscalls. That's why Linux kernel developers introduced new close_from syscall. It closes all FDs within given range, in a single syscall. Since we keep list of FDs we want to preserve and pass to the child process, we can use this syscall to close all FDs in between. We don't even need to care about opened FDs. Of course, we have to check whether the syscall is available and fall back to the old implementation if it isn't. Signed-off-by: Michal Privoznik Reviewed-by: Ján Tomko Reviewed-by: Kristina Hanicova --- src/util/vircommand.c | 77 ++++++++++++++++++++++++++++++++++++++++--- tests/commandtest.c | 2 ++ 2 files changed, 75 insertions(+), 4 deletions(-) diff --git a/src/util/vircommand.c b/src/util/vircommand.c index 867f45b57b..5f094c625a 100644 --- a/src/util/vircommand.c +++ b/src/util/vircommand.c @@ -527,10 +527,10 @@ virCommandMassCloseGetFDsGeneric(virCommand *cmd G_GNUC_UNUSED, # endif /* !__linux__ */ static int -virCommandMassClose(virCommand *cmd, - int childin, - int childout, - int childerr) +virCommandMassCloseFrom(virCommand *cmd, + int childin, + int childout, + int childerr) { g_autoptr(virBitmap) fds = NULL; int openmax = sysconf(_SC_OPEN_MAX); @@ -597,6 +597,75 @@ virCommandMassClose(virCommand *cmd, } +static int +virCommandMassCloseRange(virCommand *cmd, + int childin, + int childout, + int childerr) +{ + g_autoptr(virBitmap) fds = virBitmapNew(0); + ssize_t first; + ssize_t last; + size_t i; + + virBitmapSetBitExpand(fds, childin); + virBitmapSetBitExpand(fds, childout); + virBitmapSetBitExpand(fds, childerr); + + for (i = 0; i < cmd->npassfd; i++) { + int fd = cmd->passfd[i].fd; + + virBitmapSetBitExpand(fds, fd); + + if (virSetInherit(fd, true) < 0) { + virReportSystemError(errno, _("failed to preserve fd %1$d"), fd); + return -1; + } + } + + first = 2; + while ((last = virBitmapNextSetBit(fds, first)) >= 0) { + if (first + 1 == last) { + first = last; + continue; + } + + /* Preserve @first and @last and close everything in between. */ + if (virCloseRange(first + 1, last - 1) < 0) { + virReportSystemError(errno, + _("Unable to mass close FDs (first=%1$zd, last=%2$zd)"), + first + 1, last - 1); + return -1; + } + + first = last; + } + + if (virCloseRange(first + 1, ~0U) < 0) { + virReportSystemError(errno, + _("Unable to mass close FDs (first=%1$zd, last=%2$d"), + first + 1, ~0U); + return -1; + } + + return 0; +} + + + +static int +virCommandMassClose(virCommand *cmd, + int childin, + int childout, + int childerr) +{ + if (virCloseRangeIsSupported()) + return virCommandMassCloseRange(cmd, childin, childout, childerr); + + return virCommandMassCloseFrom(cmd, childin, childout, childerr); +} + + /* * virExec: * @cmd virCommand * containing all information about the program to diff --git a/tests/commandtest.c b/tests/commandtest.c index 688cf59160..aa108ce583 100644 --- a/tests/commandtest.c +++ b/tests/commandtest.c @@ -1247,6 +1247,8 @@ mymain(void) setpgid(0, 0); ignore_value(setsid()); + virCloseRangeInit(); + /* Our test expects particular fd values; to get that, we must not * leak fds that we inherited from a lazy parent. At the same * time, virInitialize may open some fds (perhaps via third-party