gspawn: Optimize fd closing on AIX and BSDs

Instead of calling close or fcntl on all possible file descriptors,
which is slow on systems with very high limit or even no limit on open
file descriptors, we can use closefrom or fcntl with F_CLOSEM to close
all unwanted file descriptors with a single system call.

This change only improves the performance when GSpawnChildSetupFunc is
NULL because there are applications known to abuse GSpawnChildSetupFunc
to unset FD_CLOEXEC on file descriptors. Since the change mentioned
above requires closing file descriptors directly, it cannot be used when
the caller may want to keep some of them open.

This patch was written by Sebastian Schwarz <seschwar@gmail.com> and
uploaded to https://gitlab.gnome.org/GNOME/glib/merge_requests/574.
It was later modified by Ting-Wei Lan <lantw@src.gnome.org> to address
code review issues.

Fixes: https://gitlab.gnome.org/GNOME/glib/issues/1638
This commit is contained in:
Sebastian Schwarz 2019-09-15 12:53:38 +08:00 committed by Ting-Wei Lan
parent c8bad68884
commit 2d37239510

View File

@ -47,7 +47,7 @@
#include <sys/resource.h>
#endif /* HAVE_SYS_RESOURCE_H */
#ifdef __linux__
#if defined(__linux__) || defined(__DragonFly__)
#include <sys/syscall.h> /* for syscall and SYS_getdents64 */
#endif
@ -1126,6 +1126,28 @@ set_cloexec (void *data, gint fd)
return 0;
}
static gint
sane_close (gint fd)
{
gint ret;
retry:
ret = close (fd);
if (ret < 0 && errno == EINTR)
goto retry;
return ret;
}
G_GNUC_UNUSED static int
close_func (void *data, int fd)
{
if (fd >= GPOINTER_TO_INT (data))
(void) sane_close (fd);
return 0;
}
#ifndef HAVE_FDWALK
#ifdef __linux__
struct linux_dirent64
@ -1200,7 +1222,7 @@ fdwalk (int (*cb)(void *data, int fd), void *data)
}
}
close (dir_fd);
sane_close (dir_fd);
return res;
}
@ -1223,7 +1245,39 @@ fdwalk (int (*cb)(void *data, int fd), void *data)
return res;
}
#endif /* HAVE_FDWALK */
static void
safe_closefrom (int lowfd)
{
#if defined(__FreeBSD__) || defined(__OpenBSD__)
/* Use closefrom function provided by the system if it is known to be
* async-signal safe.
*
* FreeBSD: closefrom is included in the list of async-signal safe functions
* found in https://man.freebsd.org/sigaction(2).
*
* OpenBSD: closefrom is not included in the list, but a direct system call
* should be safe to use.
*/
(void) closefrom (lowfd);
#elif defined(__DragonFly__)
/* It is unclear whether closefrom function included in DragonFlyBSD libc_r
* is safe to use because it calls a lot of library functions. It is also
* unclear whether libc_r itself is still being used. Therefore, we do a
* direct system call here ourselves to avoid possible issues.
*/
(void) syscall (SYS_closefrom, lowfd);
#elif defined(F_CLOSEM)
/* NetBSD and AIX have a special fcntl command which does the same thing as
* closefrom. NetBSD also includes closefrom function, which seems to be a
* simple wrapper of the fcntl command.
*/
(void) fcntl (lowfd, F_CLOSEM);
#else
(void) fdwalk (close_func, GINT_TO_POINTER (lowfd));
#endif
}
static gint
sane_dup2 (gint fd1, gint fd2)
@ -1281,21 +1335,6 @@ do_exec (gint child_err_report_fd,
write_err_and_exit (child_err_report_fd,
CHILD_CHDIR_FAILED);
/* Close all file descriptors but stdin stdout and stderr as
* soon as we exec. Note that this includes
* child_err_report_fd, which keeps the parent from blocking
* forever on the other end of that pipe.
*/
if (close_descriptors)
{
fdwalk (set_cloexec, GINT_TO_POINTER(3));
}
else
{
/* We need to do child_err_report_fd anyway */
set_cloexec (GINT_TO_POINTER(0), child_err_report_fd);
}
/* Redirect pipes as required */
if (stdin_fd >= 0)
@ -1352,6 +1391,31 @@ do_exec (gint child_err_report_fd,
close_and_invalidate (&write_null);
}
/* Close all file descriptors but stdin, stdout and stderr
* before we exec. Note that this includes
* child_err_report_fd, which keeps the parent from blocking
* forever on the other end of that pipe.
*/
if (close_descriptors)
{
if (child_setup == NULL)
{
sane_dup2 (child_err_report_fd, 3);
set_cloexec (GINT_TO_POINTER (0), 3);
safe_closefrom (4);
child_err_report_fd = 3;
}
else
{
fdwalk (set_cloexec, GINT_TO_POINTER (3));
}
}
else
{
/* We need to do child_err_report_fd anyway */
set_cloexec (GINT_TO_POINTER (0), child_err_report_fd);
}
/* Call user function just before we exec */
if (child_setup)
{