diff options
author | Amanieu d'Antras <amanieu@gmail.com> | 2022-05-05 15:11:29 +0100 |
---|---|---|
committer | Orion Hodson <oth@google.com> | 2022-05-06 12:45:17 +0100 |
commit | c88b39b4d1df84ce73a7a835f421f695649acbdd (patch) | |
tree | 3cf68cdeb74f7f8ef5462301ce75012de950d9be | |
parent | 32256c4bbe6cfaa286607cafe0bfc84c9a7748a1 (diff) | |
download | libcore-c88b39b4d1df84ce73a7a835f421f695649acbdd.tar.gz |
UnixProcess_md.c: avoid potential deadlock after vfork
opendir is not async-signal-safe and should not be called after forking.
This can cause a deadlock if both of these conditions are met:
- The program is running under a binary translation tool such as
Valgrind which emulates the vfork syscall using fork.
- The malloc mutex was locked at the time of the fork, which causes it
to remain permanently locked in the child process.
As a workaround, we access the directory directly with the getdents
syscall using a stack-allocated buffer.
Also, some applications have been found to hook the open/close libc
functions and end up deadlock in the child due to the use of locks in
the hooks. This is worked around by using raw syscalls in the child.
Bug: 230445494
Test: atest CtsLibcoreTestCases:org.apache.harmony.tests.java.lang.ProcessBuilderTest
Test: atest CtsLibcoreTestCases:libcore.java.lang.ProcessBuilderTest
Change-Id: I0f927c00f2a02f01be0e9f50388f5bc28dc693e1
Merged-In: I0f927c00f2a02f01be0e9f50388f5bc28dc693e1
(cherry picked from commit 3ed6e0b7f5c11a36b39e4d0867d95934fc03aef4)
-rw-r--r-- | ojluni/src/main/native/UNIXProcess_md.c | 101 |
1 files changed, 88 insertions, 13 deletions
diff --git a/ojluni/src/main/native/UNIXProcess_md.c b/ojluni/src/main/native/UNIXProcess_md.c index a3790e8c5f1..2d0a6763b90 100644 --- a/ojluni/src/main/native/UNIXProcess_md.c +++ b/ojluni/src/main/native/UNIXProcess_md.c @@ -58,6 +58,10 @@ #include <unistd.h> #include <fcntl.h> #include <limits.h> +// Android-added: Use raw syscalls instead of libc functions in the child. +#ifdef __linux__ +#include <sys/syscall.h> +#endif #ifdef __APPLE__ #include <crt_externs.h> @@ -400,16 +404,29 @@ restartableClose(int fd) return err; } +// Android-added: in the child process, we want to avoid using the libc +// close() function because it is sometimes intercepted by other libraries and +// could cause a deadlock. +static int closeInChild(int fd) +{ +#ifdef __linux__ + return syscall(__NR_close, fd); +#else + return close(fd); +#endif +} + static int closeSafely(int fd) { return (fd == -1) ? 0 : restartableClose(fd); } +// Android-added: See closeInChild. static int -isAsciiDigit(char c) +closeSafelyInChild(int fd) { - return c >= '0' && c <= '9'; + return (fd == -1) ? 0 : closeInChild(fd); } // Android-changed: Fuchsia: Alias *64 on Fuchsia builds. http://b/119496969 @@ -422,6 +439,63 @@ isAsciiDigit(char c) #define FD_DIR "/proc/self/fd" #endif +// Android-changed: opendir is not async-signal-safe and should not be called +// after forking. This can cause a deadlock if both of these conditions are met: +// - The program is running under a binary translation tool such as Valgrind +// which emulates the vfork syscall using fork. +// - The malloc mutex was locked at the time of the fork, which remains +// permanently locked in the child process. +// +// As a workaround, we access the directory directly with the getdents syscall +// using a stack-allocated buffer. +#ifdef __linux__ +static int +closeDescriptors(void) +{ + int dir_fd; + char buffer[4096]; + long available_bytes; + int from_fd = FAIL_FILENO + 1; + + // __NR_close_range may not be available on the host glibc. +#ifdef __NR_close_range + // On kernel v5.9+ we can close all file descriptors with a single syscall. + if (syscall(__NR_close_range, from_fd, ~0, 0) == 0) + return 1; +#endif + + // Close one file descriptor to guarantee that we have enough free FDs to + // open FD_DIR. + closeInChild(from_fd); + + if ((dir_fd = syscall(__NR_openat, AT_FDCWD, FD_DIR, O_CLOEXEC | O_DIRECTORY | O_RDONLY)) == -1) + return 0; + + // See closeInChild for why we are using a raw syscall here. + while ((available_bytes = syscall(__NR_getdents64, dir_fd, buffer, sizeof(buffer))) > 0) { + char *p = buffer; + while (available_bytes > 0) { + struct dirent64 *dirp = (struct dirent64 *)p; + p += dirp->d_reclen; + available_bytes -= dirp->d_reclen; + + int fd = atoi(dirp->d_name); + if (fd >= from_fd && fd != dir_fd) + closeInChild(fd); + } + } + + closeInChild(dir_fd); + + return 1; +} +#else +static int +isAsciiDigit(char c) +{ + return c >= '0' && c <= '9'; +} + static int closeDescriptors(void) { @@ -436,8 +510,8 @@ closeDescriptors(void) * the lowest numbered file descriptor, just like open(). So we * close a couple explicitly. */ - restartableClose(from_fd); /* for possible use by opendir() */ - restartableClose(from_fd + 1); /* another one for good luck */ + closeInChild(from_fd); /* for possible use by opendir() */ + closeInChild(from_fd + 1); /* another one for good luck */ if ((dp = opendir(FD_DIR)) == NULL) return 0; @@ -449,20 +523,21 @@ closeDescriptors(void) int fd; if (isAsciiDigit(dirp->d_name[0]) && (fd = strtol(dirp->d_name, NULL, 10)) >= from_fd + 2) - restartableClose(fd); + closeInChild(fd); } closedir(dp); return 1; } +#endif static int moveDescriptor(int fd_from, int fd_to) { if (fd_from != fd_to) { if ((restartableDup2(fd_from, fd_to) == -1) || - (restartableClose(fd_from) == -1)) + (closeInChild(fd_from) == -1)) return -1; } return 0; @@ -736,10 +811,10 @@ childProcess(void *arg) /* Close the parent sides of the pipes. Closing pipe fds here is redundant, since closeDescriptors() would do it anyways, but a little paranoia is a good thing. */ - if ((closeSafely(p->in[1]) == -1) || - (closeSafely(p->out[0]) == -1) || - (closeSafely(p->err[0]) == -1) || - (closeSafely(p->fail[0]) == -1)) + if ((closeSafelyInChild(p->in[1]) == -1) || + (closeSafelyInChild(p->out[0]) == -1) || + (closeSafelyInChild(p->err[0]) == -1) || + (closeSafelyInChild(p->fail[0]) == -1)) goto WhyCantJohnnyExec; /* Give the child sides of the pipes the right fileno's. */ @@ -751,7 +826,7 @@ childProcess(void *arg) goto WhyCantJohnnyExec; if (p->redirectErrorStream) { - if ((closeSafely(p->err[1]) == -1) || + if ((closeSafelyInChild(p->err[1]) == -1) || (restartableDup2(STDOUT_FILENO, STDERR_FILENO) == -1)) goto WhyCantJohnnyExec; } else { @@ -768,7 +843,7 @@ childProcess(void *arg) int max_fd = (int)sysconf(_SC_OPEN_MAX); int fd; for (fd = FAIL_FILENO + 1; fd < max_fd; fd++) - if (restartableClose(fd) == -1 && errno != EBADF) + if (closeInChild(fd) == -1 && errno != EBADF) goto WhyCantJohnnyExec; } @@ -796,7 +871,7 @@ childProcess(void *arg) int errnum = errno; restartableWrite(FAIL_FILENO, &errnum, sizeof(errnum)); } - restartableClose(FAIL_FILENO); + closeInChild(FAIL_FILENO); _exit(-1); return 0; /* Suppress warning "no return value from function" */ } |