Skip to content

Emulate POSIX_SPAWN_CLOEXEC_DEFAULT in fork/exec #79

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 16 additions & 6 deletions Sources/Subprocess/Platforms/Subprocess+Unix.swift
Original file line number Diff line number Diff line change
Expand Up @@ -116,14 +116,24 @@ extension Execution {
// pidfd_send_signal does not support sending signal to process group
try _kill(pid, signal: signal)
} else {
guard _pidfd_send_signal(
// Try pidfd_send_signal first
if _pidfd_send_signal(
processIdentifier.processDescriptor,
signal.rawValue
) == 0 else {
throw SubprocessError(
code: .init(.failedToSendSignal(signal.rawValue)),
underlyingError: .init(rawValue: errno)
)
) == 0 {
return
} else {
if errno == ENOSYS {
// pidfd_send_signal is not implemented in this system
// fallback to _kill()
try _kill(pid, signal: signal)
} else {
// Throw all other errors
throw SubprocessError(
code: .init(.failedToSendSignal(signal.rawValue)),
underlyingError: .init(rawValue: errno)
)
}
}
}
#else
Expand Down
240 changes: 171 additions & 69 deletions Sources/_SubprocessCShims/process_shims.c
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,15 @@
#include <string.h>
#include <fcntl.h>
#include <pthread.h>

#include <dirent.h>
#include <stdio.h>
#include <limits.h>

#if __has_include(<linux/close_range.h>)
#include <linux/close_range.h>
#endif

#endif // TARGET_OS_WINDOWS

#if __has_include(<crt_externs.h>)
#include <crt_externs.h>
Expand All @@ -50,6 +57,7 @@ extern char **environ;
#include <mach/vm_page_size.h>
#endif

#if !TARGET_OS_WINDOWS
int _was_process_exited(int status) {
return WIFEXITED(status);
}
Expand All @@ -70,24 +78,8 @@ int _was_process_suspended(int status) {
return WIFSTOPPED(status);
}

#if TARGET_OS_LINUX
#include <stdio.h>

int _shims_snprintf(
char * _Nonnull str,
int len,
const char * _Nonnull format,
char * _Nonnull str1,
char * _Nonnull str2
) {
return snprintf(str, len, format, str1, str2);
}

int _pidfd_send_signal(int pidfd, int signal) {
return syscall(SYS_pidfd_send_signal, pidfd, signal, NULL, 0);
}
#endif // !TARGET_OS_WINDOWS

#endif

#if __has_include(<mach/vm_page_size.h>)
vm_size_t _subprocess_vm_size(void) {
Expand All @@ -96,40 +88,6 @@ vm_size_t _subprocess_vm_size(void) {
}
#endif

// MARK: - Private Helpers
static pthread_mutex_t _subprocess_fork_lock = PTHREAD_MUTEX_INITIALIZER;

static int _subprocess_block_everything_but_something_went_seriously_wrong_signals(sigset_t *old_mask) {
sigset_t mask;
int r = 0;
r |= sigfillset(&mask);
r |= sigdelset(&mask, SIGABRT);
r |= sigdelset(&mask, SIGBUS);
r |= sigdelset(&mask, SIGFPE);
r |= sigdelset(&mask, SIGILL);
r |= sigdelset(&mask, SIGKILL);
r |= sigdelset(&mask, SIGSEGV);
r |= sigdelset(&mask, SIGSTOP);
r |= sigdelset(&mask, SIGSYS);
r |= sigdelset(&mask, SIGTRAP);

r |= pthread_sigmask(SIG_BLOCK, &mask, old_mask);
return r;
}

#define _subprocess_precondition(__cond) do { \
int eval = (__cond); \
if (!eval) { \
__builtin_trap(); \
} \
} while(0)

#if __DARWIN_NSIG
# define _SUBPROCESS_SIG_MAX __DARWIN_NSIG
#else
# define _SUBPROCESS_SIG_MAX 32
#endif


// MARK: - Darwin (posix_spawn)
#if TARGET_OS_MAC
Expand Down Expand Up @@ -315,6 +273,16 @@ int _pidfd_open(pid_t pid) {
return syscall(SYS_pidfd_open, pid, 0);
}

// SYS_pidfd_send_signal is only defined on Linux Kernel 5.1 and above
// Define our dummy value if it's not available
#ifndef SYS_pidfd_send_signal
#define SYS_pidfd_send_signal 424
#endif

int _pidfd_send_signal(int pidfd, int signal) {
return syscall(SYS_pidfd_send_signal, pidfd, signal, NULL, 0);
}

// SYS_clone3 is only defined on Linux Kernel 5.3 and above
// Define our dummy value if it's not available (as is the case with Musl libc)
#ifndef SYS_clone3
Expand Down Expand Up @@ -351,6 +319,140 @@ static int _clone3(int *pidfd) {
return syscall(SYS_clone3, &args, sizeof(args));
}

struct linux_dirent64 {
unsigned long d_ino;
unsigned long d_off;
unsigned short d_reclen;
unsigned char d_type;
char d_name[];
};

static int _getdents64(int fd, struct linux_dirent64 *dirp, size_t nbytes) {
return syscall(SYS_getdents64, fd, dirp, nbytes);
}

static pthread_mutex_t _subprocess_fork_lock = PTHREAD_MUTEX_INITIALIZER;

static int _subprocess_make_critical_mask(sigset_t *old_mask) {
sigset_t mask;
int r = 0;
r |= sigfillset(&mask);
r |= sigdelset(&mask, SIGABRT);
r |= sigdelset(&mask, SIGBUS);
r |= sigdelset(&mask, SIGFPE);
r |= sigdelset(&mask, SIGILL);
r |= sigdelset(&mask, SIGKILL);
r |= sigdelset(&mask, SIGSEGV);
r |= sigdelset(&mask, SIGSTOP);
r |= sigdelset(&mask, SIGSYS);
r |= sigdelset(&mask, SIGTRAP);

r |= pthread_sigmask(SIG_BLOCK, &mask, old_mask);
return r;
}

#define _subprocess_precondition(__cond) do { \
int eval = (__cond); \
if (!eval) { \
__builtin_trap(); \
} \
} while(0)

#if __DARWIN_NSIG /* Darwin */
# define _SUBPROCESS_SIG_MAX __DARWIN_NSIG
#elif defined(NSIG_MAX) /* POSIX issue 8 */
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

super nit: the POSIX one should go first, being the standard thing, shouldn't it?

# define _SUBPROCESS_SIG_MAX NSIG_MAX
#elif defined(_SIG_MAXSIG) /* FreeBSD */
# define _SUBPROCESS_SIG_MAX _SIG_MAXSIG
#elif defined(_SIGMAX) /* QNX */
# define _SUBPROCESS_SIG_MAX (_SIGMAX + 1)
#elif defined(NSIG) /* 99% of everything else */
# define _SUBPROCESS_SIG_MAX NSIG
#else /* Last resort */
# define _SUBPROCESS_SIG_MAX (sizeof(sigset_t) * CHAR_BIT + 1)
#endif

int _shims_snprintf(
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need this shim?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On Linux this function isn't exposed via import Glibc

char * _Nonnull str,
int len,
const char * _Nonnull format,
char * _Nonnull str1,
char * _Nonnull str2
) {
return snprintf(str, len, format, str1, str2);
}

static int _positive_int_parse(const char *str) {
char *end;
long value = strtol(str, &end, 10);
if (end == str) {
// No digits found
return -1;
}
if (errno == ERANGE || value <= 0 || value > INT_MAX) {
// Out of range
return -1;
}
return (int)value;
}

// Linux-specific version that uses syscalls directly and doesn't allocate heap memory.
// Safe to use after vfork() and before execve()
static int _highest_possibly_open_fd_dir_linux(const char *fd_dir) {
int highest_fd_so_far = 0;
int dir_fd = open(fd_dir, O_RDONLY);
if (dir_fd < 0) {
// errno set by `open`.
return -1;
}

// Buffer for directory entries - allocated on stack, no heap allocation
char buffer[4096] = {0};
long bytes_read = -1;

while ((bytes_read = _getdents64(dir_fd, (struct linux_dirent64 *)buffer, sizeof(buffer))) > 0) {
if (bytes_read < 0) {
if (errno == EINTR) {
continue;
} else {
// `errno` set by _getdents64.
highest_fd_so_far = -1;
goto error;
}
}
long offset = 0;
while (offset < bytes_read) {
struct linux_dirent64 *entry = (struct linux_dirent64 *)(buffer + offset);

// Skip "." and ".." entries
if (entry->d_name[0] != '.') {
int number = _positive_int_parse(entry->d_name);
if (number > highest_fd_so_far) {
highest_fd_so_far = number;
}
}

offset += entry->d_reclen;
}
}

error:
close(dir_fd);
return highest_fd_so_far;
}

static int _highest_possibly_open_fd(void) {
#if defined(__linux__)
int hi = _highest_possibly_open_fd_dir_linux("/dev/fd");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: might be worth a comment that this is only needed for the Linux-older-than-5.9 path where we don't have close_range which makes this more efficient.

if (hi < 0) {
hi = sysconf(_SC_OPEN_MAX);
}
#else
int hi = sysconf(_SC_OPEN_MAX);
#endif
return hi;
}

int _subprocess_fork_exec(
pid_t * _Nonnull pid,
int * _Nonnull pidfd,
Expand Down Expand Up @@ -410,7 +512,7 @@ int _subprocess_fork_exec(
_subprocess_precondition(rc == 0);
// Block all signals on this thread
sigset_t old_sigmask;
rc = _subprocess_block_everything_but_something_went_seriously_wrong_signals(&old_sigmask);
rc = _subprocess_make_critical_mask(&old_sigmask);
if (rc != 0) {
close(pipefd[0]);
close(pipefd[1]);
Expand Down Expand Up @@ -530,20 +632,22 @@ int _subprocess_fork_exec(
if (rc < 0) {
write_error_and_exit;
}

// Close parent side
if (file_descriptors[1] >= 0) {
rc = close(file_descriptors[1]);
}
if (file_descriptors[3] >= 0) {
rc = close(file_descriptors[3]);
}
if (file_descriptors[5] >= 0) {
rc = close(file_descriptors[5]);
}

if (rc < 0) {
write_error_and_exit;
// Close all other file descriptors
rc = -1;
errno = ENOSYS;
#if __has_include(<linux/close_range.h>) || defined(__FreeBSD__)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, one more thing. This breaks the build on Android because it doesn't have close_range.

/Volumes/Sources/OpenSource/swift-subprocess/Sources/_SubprocessCShims/process_shims.c:640:14: error: call to undeclared function 'close_range'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
640 | rc = close_range(STDERR_FILENO + 1, pipefd[1] - 1, 0);
| ^
1 error generated.
[8/22] Compiling CSystem shims.c

It was introduced in NDK r34 so you need an availability guard...

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will fix it:

Suggested change
#if __has_include(<linux/close_range.h>) || defined(__FreeBSD__)
#if (__has_include(<linux/close_range.h>) && (!defined(__ANDROID__) || __ANDROID_API__ >= 34)) || defined(__FreeBSD__)

// We must NOT close pipefd[1] for writing errors
rc = close_range(STDERR_FILENO + 1, pipefd[1] - 1, 0);
rc |= close_range(pipefd[1] + 1, ~0U, 0);
#endif
if (rc != 0) {
// close_range failed (or doesn't exist), fall back to close()
for (int fd = STDERR_FILENO + 1; fd < _highest_possibly_open_fd(); fd++) {
// We must NOT close pipefd[1] for writing errors
if (fd != pipefd[1]) {
close(fd);
}
}
}

// Run custom configuratior
Expand Down Expand Up @@ -621,8 +725,6 @@ int _subprocess_fork_exec(

#endif // TARGET_OS_LINUX

#endif // !TARGET_OS_WINDOWS

#pragma mark - Environment Locking

#if __has_include(<libc_private.h>)
Expand Down
Loading