Re: [libvirt] [PATCH v2 4/4] virCommand: use procfs to learn opened FDs

When spawning a child process, between fork() and exec() we close all file descriptors and keep only those the caller wants us to pass onto the child. The problem is how we do that. Currently, we get the limit of opened files and then iterate through each one of them and either close() it or make it survive exec(). This approach is suboptimal (although, not that much in default configurations where the limit is pretty low - 1024). We have /proc where we can learn what FDs we hold open and thus we can selectively close only those. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; --- src/util/vircommand.c | 86 +++++++++++++++++++++++++++++++++++++++---- 1 file changed, 78 insertions(+), 8 deletions(-) diff --git a/src/util/vircommand.c b/src/util/vircommand.c index 6cd7cbe065..bfc6c15cfb 100644 --- a/src/util/vircommand.c +++ b/src/util/vircommand.c @@ -418,27 +418,97 @@ virExecCommon(virCommandPtr cmd, gid_t *groups, int ngroups) static int virCommandMassClose(virCommandPtr cmd, int childin, int childout, int childerr) { + VIR_AUTOPTR(virBitmap) fds = NULL; int openmax = sysconf(_SC_OPEN_MAX); - int fd; - int tmpfd; + int fd = -1; - if (openmax < 0) { - virReportSystemError(errno, "%s", - _("sysconf(_SC_OPEN_MAX) failed")); + /* In general, it is not save to call malloc() between fork() and exec()

+ * because the child might have forked at the worst possible time, i.e. + * when another thread was in malloc() and thus held its lock. That is to + * say, POSIX does not mandate malloc() to be async-safe. Fortunately, + * glibc developers are aware of this and made malloc() async-safe. + * Therefore we can safely allocate memory here (and transitively call + * opendir/readdir) without a deadlock. */ + + if (!(fds = virBitmapNew(openmax))) + return -1; + +# ifdef __linux__ + if (virCommandMassCloseGetFDsLinux(cmd, fds) < 0) + return -1; +# else + if (virCommandMassCloseGetFDsGeneric(cmd, fds) < 0) return -1; - } +# endif - for (fd = 3; fd < openmax; fd++) { + fd = virBitmapNextSetBit(fds, -1); + for (; fd >= 0; fd = virBitmapNextSetBit(fds, fd)) {

if (fd == childin || fd == childout || fd == childerr) continue; if (!virCommandFDIsSet(cmd, fd)) { - tmpfd = fd; + int tmpfd = fd; VIR_MASS_CLOSE(tmpfd);