stan/jdk-24
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

8307990: jspawnhelper must close its writing side of a pipe before reading from it

Browse Source 
Reviewed-by: stuefe, rriggs
This commit is contained in:
Volker Simonis 2023-06-01 10:56:31 +00:00
parent 4460429d7a
commit 39f6d807db
5 changed files with 378 additions and 38 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

37
src/java.base/unix/native/jspawnhelper/jspawnhelper.c
View File

@ -1,5 +1,5 @@
/*
* Copyright (c) 2013, 2022, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2013, 2023, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@ -23,6 +23,7 @@
* questions.
*/
#include <assert.h>
#include <errno.h>
#include <fcntl.h>
#include <signal.h>
@ -83,11 +84,15 @@ void initChildStuff (int fdin, int fdout, ChildStuff *c) {
error (fdout, ERR_PIPE);
}
if (readFully (fdin, c, sizeof(*c)) == -1) {
#ifdef DEBUG
jtregSimulateCrash(0, 5);
#endif
if (readFully (fdin, c, sizeof(*c)) != sizeof(*c)) {
error (fdout, ERR_PIPE);
}
if (readFully (fdin, &sp, sizeof(sp)) == -1) {
if (readFully (fdin, &sp, sizeof(sp)) != sizeof(sp)) {
error (fdout, ERR_PIPE);
}
@ -96,7 +101,7 @@ void initChildStuff (int fdin, int fdout, ChildStuff *c) {
ALLOC(buf, bufsize);
if (readFully (fdin, buf, bufsize) == -1) {
if (readFully (fdin, buf, bufsize) != bufsize) {
error (fdout, ERR_PIPE);
}
@ -132,12 +137,15 @@ int main(int argc, char *argv[]) {
ChildStuff c;
struct stat buf;
/* argv[0] contains the fd number to read all the child info */
int r, fdin, fdout;
int r, fdinr, fdinw, fdout;
sigset_t unblock_signals;
r = sscanf (argv[argc-1], "%d:%d", &fdin, &fdout);
if (r == 2 && fcntl(fdin, F_GETFD) != -1) {
fstat(fdin, &buf);
#ifdef DEBUG
jtregSimulateCrash(0, 4);
#endif
r = sscanf (argv[argc-1], "%d:%d:%d", &fdinr, &fdinw, &fdout);
if (r == 3 && fcntl(fdinr, F_GETFD) != -1 && fcntl(fdinw, F_GETFD) != -1) {
fstat(fdinr, &buf);
if (!S_ISFIFO(buf.st_mode))
shutItDown();
} else {
@ -148,7 +156,18 @@ int main(int argc, char *argv[]) {
sigemptyset(&unblock_signals);
sigprocmask(SIG_SETMASK, &unblock_signals, NULL);
initChildStuff (fdin, fdout, &c);
// Close the writing end of the pipe we use for reading from the parent.
// We have to do this before we start reading from the parent to avoid
// blocking in the case the parent exits before we finished reading from it.
close(fdinw); // Deliberately ignore errors (see https://lwn.net/Articles/576478/).
initChildStuff (fdinr, fdout, &c);
// Now set the file descriptor for the pipe's writing end to -1
// for the case that somebody tries to close it again.
assert(c.childenv[1] == fdinw);
c.childenv[1] = -1;
// The file descriptor for reporting errors back to our parent we got on the command
// line should be the same like the one in the ChildStuff struct we've just read.
assert(c.fail[1] == fdout);
childProcess (&c);
return 0; /* NOT REACHED */

49
src/java.base/unix/native/libjava/ProcessImpl_md.c
View File

@ -1,5 +1,5 @@
/*
* Copyright (c) 1995, 2022, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 1995, 2023, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@ -487,14 +487,14 @@ static pid_t
spawnChild(JNIEnv *env, jobject process, ChildStuff *c, const char *helperpath) {
pid_t resultPid;
int i, offset, rval, bufsize, magic;
char *buf, buf1[(2 * 11) + 2]; // "%d:%d\0"
char *buf, buf1[(3 * 11) + 3]; // "%d:%d:%d\0"
char *hlpargs[2];
SpawnInfo sp;
/* need to tell helper which fd is for receiving the childstuff
* and which fd to send response back on
*/
snprintf(buf1, sizeof(buf1), "%d:%d", c->childenv[0], c->fail[1]);
snprintf(buf1, sizeof(buf1), "%d:%d:%d", c->childenv[0], c->childenv[1], c->fail[1]);
/* put the fd string as argument to the helper cmd */
hlpargs[0] = buf1;
hlpargs[1] = 0;
@ -527,7 +527,7 @@ spawnChild(JNIEnv *env, jobject process, ChildStuff *c, const char *helperpath)
if (c->fds[i] != -1) {
int flags = fcntl(c->fds[i], F_GETFD);
if (flags & FD_CLOEXEC) {
fcntl(c->fds[i], F_SETFD, flags & (~1));
fcntl(c->fds[i], F_SETFD, flags & (~FD_CLOEXEC));
}
}
}
@ -538,6 +538,10 @@ spawnChild(JNIEnv *env, jobject process, ChildStuff *c, const char *helperpath)
return -1;
}
#ifdef DEBUG
jtregSimulateCrash(resultPid, 1);
#endif
/* now the lengths are known, copy the data */
buf = NEW(char, bufsize);
if (buf == 0) {
@ -553,11 +557,24 @@ spawnChild(JNIEnv *env, jobject process, ChildStuff *c, const char *helperpath)
magic = magicNumber();
/* write the two structs and the data buffer */
write(c->childenv[1], (char *)&magic, sizeof(magic)); // magic number first
write(c->childenv[1], (char *)c, sizeof(*c));
write(c->childenv[1], (char *)&sp, sizeof(sp));
write(c->childenv[1], buf, bufsize);
if (writeFully(c->childenv[1], (char *)&magic, sizeof(magic)) != sizeof(magic)) { // magic number first
return -1;
}
#ifdef DEBUG
jtregSimulateCrash(resultPid, 2);
#endif
if (writeFully(c->childenv[1], (char *)c, sizeof(*c)) != sizeof(*c) ||
writeFully(c->childenv[1], (char *)&sp, sizeof(sp)) != sizeof(sp) ||
writeFully(c->childenv[1], buf, bufsize) != bufsize) {
return -1;
}
/* We're done. Let jspwanhelper know he can't expect any more data from us. */
close(c->childenv[1]);
c->childenv[1] = -1;
free(buf);
#ifdef DEBUG
jtregSimulateCrash(resultPid, 3);
#endif
/* In this mode an external main() in invoked which calls back into
* childProcess() in this file, rather than directly
@ -610,6 +627,8 @@ Java_java_lang_ProcessImpl_forkAndExec(JNIEnv *env,
in[0] = in[1] = out[0] = out[1] = err[0] = err[1] = fail[0] = fail[1] = -1;
childenv[0] = childenv[1] = -1;
// Reset errno to protect against bogus error messages
errno = 0;
if ((c = NEW(ChildStuff, 1)) == NULL) return -1;
c->argv = NULL;
@ -708,11 +727,9 @@ Java_java_lang_ProcessImpl_forkAndExec(JNIEnv *env,
goto Catch;
}
case sizeof(errnum):
assert(errnum == CHILD_IS_ALIVE);
if (errnum != CHILD_IS_ALIVE) {
/* Should never happen since the first thing the spawn
* helper should do is to send an alive ping to the parent,
* before doing any subsequent work. */
/* This can happen if the spawn helper encounters an error
* before or during the handshake with the parent. */
throwIOException(env, 0, "Bad code from spawn helper "
"(Failed to exec spawn helper)");
goto Catch;
@ -748,8 +765,12 @@ Java_java_lang_ProcessImpl_forkAndExec(JNIEnv *env,
/* Always clean up fail and childEnv descriptors */
closeSafely(fail[0]);
closeSafely(fail[1]);
closeSafely(childenv[0]);
closeSafely(childenv[1]);
/* We use 'c->childenv' here rather than 'childenv' because 'spawnChild()' might have
* already closed 'c->childenv[1]' and signaled this by setting 'c->childenv[1]' to '-1'.
* Otherwise 'c->childenv' and 'childenv' are the same because we just copied 'childenv'
* to 'c->childenv' (with 'copyPipe()') before calling 'startChild()'. */
closeSafely(c->childenv[0]);
closeSafely(c->childenv[1]);
releaseBytes(env, helperpath, phelperpath);
releaseBytes(env, prog, pprog);

78
src/java.base/unix/native/libjava/childproc.c
View File

@ -1,5 +1,5 @@
/*
* Copyright (c) 2013, 2020, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2013, 2023, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@ -36,14 +36,6 @@
const char * const *parentPathv;
ssize_t
restartableWrite(int fd, const void *buf, size_t count)
{
ssize_t result;
RESTARTABLE(write(fd, buf, count), result);
return result;
}
int
restartableDup2(int fd_from, int fd_to)
{
@ -163,6 +155,46 @@ readFully(int fd, void *buf, size_t nbyte)
}
}
/*
* Writes nbyte bytes from buf into file descriptor fd,
* The write operation is retried in case of EINTR or partial writes.
*
* Returns number of bytes written (normally nbyte).
* In case of write errors, returns -1 and sets errno.
*/
ssize_t
writeFully(int fd, const void *buf, size_t nbyte)
{
#ifdef DEBUG
/* This code is only used in debug builds for testing truncated writes
* during the handshake with the spawn helper for MODE_POSIX_SPAWN.
* See: test/jdk/java/lang/ProcessBuilder/JspawnhelperProtocol.java
*/
const char* env = getenv("JTREG_JSPAWNHELPER_PROTOCOL_TEST");
if (env != NULL && atoi(env) == 99 && nbyte == sizeof(ChildStuff)) {
printf("posix_spawn: truncating write of ChildStuff struct\n");
fflush(stdout);
nbyte = nbyte / 2;
}
#endif
ssize_t remaining = nbyte;
for (;;) {
ssize_t n = write(fd, buf, remaining);
if (n > 0) {
remaining -= n;
if (remaining <= 0)
return nbyte;
/* We were interrupted in the middle of writing the bytes.
* Unlikely, but possible. */
buf = (void *) (((char *)buf) + n);
} else if (n == -1 && errno == EINTR) {
/* Retry */
} else {
return -1;
}
}
}
void
initVectorFromBlock(const char**vector, const char* block, int count)
{
@ -210,7 +242,7 @@ execve_with_shell_fallback(int mode, const char *file,
const char *argv[],
const char *const envp[])
{
if (mode == MODE_CLONE || mode == MODE_VFORK) {
if (mode == MODE_VFORK) {
/* shared address space; be very careful. */
execve(file, (char **) argv, (char **) envp);
if (errno == ENOEXEC)
@ -321,9 +353,14 @@ childProcess(void *arg)
/* Child shall signal aliveness to parent at the very first
* moment. */
int code = CHILD_IS_ALIVE;
restartableWrite(fail_pipe_fd, &code, sizeof(code));
if (writeFully(fail_pipe_fd, &code, sizeof(code)) != sizeof(code)) {
goto WhyCantJohnnyExec;
}
}
#ifdef DEBUG
jtregSimulateCrash(0, 6);
#endif
/* 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. */
@ -390,9 +427,26 @@ childProcess(void *arg)
*/
{
int errnum = errno;
restartableWrite(fail_pipe_fd, &errnum, sizeof(errnum));
writeFully(fail_pipe_fd, &errnum, sizeof(errnum));
}
close(fail_pipe_fd);
_exit(-1);
return 0; /* Suppress warning "no return value from function" */
}
#ifdef DEBUG
/* This method is only used in debug builds for testing MODE_POSIX_SPAWN
* in the light of abnormal program termination of either the parent JVM
* or the newly created jspawnhelper child process during the execution of
* Java_java_lang_ProcessImpl_forkAndExec().
* See: test/jdk/java/lang/ProcessBuilder/JspawnhelperProtocol.java
*/
void jtregSimulateCrash(pid_t child, int stage) {
const char* env = getenv("JTREG_JSPAWNHELPER_PROTOCOL_TEST");
if (env != NULL && atoi(env) == stage) {
printf("posix_spawn:%d\n", child);
fflush(stdout);
_exit(stage);
}
}
#endif

15
src/java.base/unix/native/libjava/childproc.h
View File

@ -1,5 +1,5 @@
/*
* Copyright (c) 2013, 2020, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2013, 2023, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@ -85,7 +85,6 @@ extern char **environ;
#define MODE_FORK 1
#define MODE_POSIX_SPAWN 2
#define MODE_VFORK 3
#define MODE_CLONE 4
typedef struct _ChildStuff
{
@ -128,7 +127,7 @@ typedef struct _SpawnInfo {
*/
extern const char * const *parentPathv;
ssize_t restartableWrite(int fd, const void *buf, size_t count);
ssize_t writeFully(int fd, const void *buf, size_t count);
int restartableDup2(int fd_from, int fd_to);
int closeSafely(int fd);
int isAsciiDigit(char c);
@ -149,4 +148,14 @@ void JDK_execvpe(int mode, const char *file,
const char *const envp[]);
int childProcess(void *arg);
#ifdef DEBUG
/* This method is only used in debug builds for testing MODE_POSIX_SPAWN
* in the light of abnormal program termination of either the parent JVM
* or the newly created jspawnhelper child process during the execution of
* Java_java_lang_ProcessImpl_forkAndExec().
* See: test/jdk/java/lang/ProcessBuilder/JspawnhelperProtocol.java
*/
void jtregSimulateCrash(pid_t child, int stage);
#endif
#endif

237
test/jdk/java/lang/ProcessBuilder/JspawnhelperProtocol.java Normal file
View File

@ -0,0 +1,237 @@
/*
* Copyright Amazon.com Inc. or its affiliates. All Rights Reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
* under the terms of the GNU General Public License version 2 only, as
* published by the Free Software Foundation.
*
* This code is distributed in the hope that it will be useful, but WITHOUT
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
* version 2 for more details (a copy is included in the LICENSE file that
* accompanied this code).
*
* You should have received a copy of the GNU General Public License version
* 2 along with this work; if not, write to the Free Software Foundation,
* Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
*
* Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
* or visit www.oracle.com if you need additional information or have any
* questions.
*
*/
/*
* @test
* @bug 8307990
* @requires (os.family == "linux") | (os.family == "aix")
* @requires vm.debug
* @library /test/lib
* @run main/othervm/timeout=300 JspawnhelperProtocol
*/
import java.io.BufferedReader;
import java.io.IOException;
import java.nio.file.Path;
import java.util.Optional;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.TimeoutException;
import jdk.test.lib.process.ProcessTools;
public class JspawnhelperProtocol {
// Timout in seconds
private static final int TIMEOUT = 60;
// Base error code to communicate various error states from the parent process to the top-level test
private static final int ERROR = 10;
private static final String[] CMD = { "pwd" };
private static final String ENV_KEY = "JTREG_JSPAWNHELPER_PROTOCOL_TEST";
private static void parentCode(String arg) throws IOException, InterruptedException {
System.out.println("Recursively executing 'JspawnhelperProtocol " + arg + "'");
Process p = null;
try {
p = Runtime.getRuntime().exec(CMD);
} catch (Exception e) {
e.printStackTrace(System.out);
System.exit(ERROR);
}
if (!p.waitFor(TIMEOUT, TimeUnit.SECONDS)) {
System.out.println("Child process timed out");
System.exit(ERROR + 1);
}
if (p.exitValue() == 0) {
String pwd = p.inputReader().readLine();
String realPwd = Path.of("").toAbsolutePath().toString();
if (!realPwd.equals(pwd)) {
System.out.println("Child process returned '" + pwd + "' (expected '" + realPwd + "')");
System.exit(ERROR + 2);
}
System.out.println(" Successfully executed '" + CMD[0] + "'");
System.exit(0);
} else {
System.out.println(" Failed to executed '" + CMD[0] + "' (exitValue=" + p.exitValue() + ")");
System.exit(ERROR + 3);
}
}
private static void normalExec() throws Exception {
ProcessBuilder pb;
pb = ProcessTools.createJavaProcessBuilder("-Djdk.lang.Process.launchMechanism=posix_spawn",
"JspawnhelperProtocol",
"normalExec");
pb.inheritIO();
Process p = pb.start();
if (!p.waitFor(TIMEOUT, TimeUnit.SECONDS)) {
throw new Exception("Parent process timed out");
}
if (p.exitValue() != 0) {
throw new Exception("Parent process exited with " + p.exitValue());
}
}
private static void simulateCrashInChild(int stage) throws Exception {
ProcessBuilder pb;
pb = ProcessTools.createJavaProcessBuilder("-Djdk.lang.Process.launchMechanism=posix_spawn",
"JspawnhelperProtocol",
"simulateCrashInChild" + stage);
pb.environment().put(ENV_KEY, Integer.toString(stage));
Process p = pb.start();
boolean foundCrashInfo = false;
try (BufferedReader br = p.inputReader()) {
String line = br.readLine();
while (line != null) {
System.out.println(line);
if (line.equals("posix_spawn:0")) {
foundCrashInfo = true;
}
line = br.readLine();
}
}
if (!foundCrashInfo) {
throw new Exception("Wrong output from child process");
}
if (!p.waitFor(TIMEOUT, TimeUnit.SECONDS)) {
throw new Exception("Parent process timed out");
}
int ret = p.exitValue();
if (ret == 0) {
throw new Exception("Expected error during child execution");
}
System.out.println("Parent exit code: " + ret);
}
private static void simulateCrashInParent(int stage) throws Exception {
ProcessBuilder pb;
pb = ProcessTools.createJavaProcessBuilder("-Djdk.lang.Process.launchMechanism=posix_spawn",
"JspawnhelperProtocol",
"simulateCrashInParent" + stage);
pb.environment().put(ENV_KEY, Integer.toString(stage));
Process p = pb.start();
String line = null;
try (BufferedReader br = p.inputReader()) {
line = br.readLine();
while (line != null && !line.startsWith("posix_spawn:")) {
System.out.println(line);
line = br.readLine();
}
}
if (line == null) {
throw new Exception("Wrong output from parent process");
}
System.out.println(line);
long childPid = Integer.parseInt(line.substring(line.indexOf(':') + 1));
if (!p.waitFor(TIMEOUT, TimeUnit.SECONDS)) {
throw new Exception("Parent process timed out");
}
Optional<ProcessHandle> oph = ProcessHandle.of(childPid);
if (!oph.isEmpty()) {
ProcessHandle ph = oph.get();
try {
// Give jspawnhelper a chance to exit gracefully
ph.onExit().get(TIMEOUT, TimeUnit.SECONDS);
} catch (TimeoutException te) {
Optional<String> cmd = ph.info().command();
if (cmd.isPresent() && cmd.get().endsWith("jspawnhelper")) {
throw new Exception("jspawnhelper still alive after parent Java process terminated");
}
}
}
int ret = p.exitValue();
if (ret != stage) {
throw new Exception("Expected exit code " + stage + " but got " + ret);
}
System.out.println("Parent exit code: " + ret);
}
private static void simulateTruncatedWriteInParent(int stage) throws Exception {
ProcessBuilder pb;
pb = ProcessTools.createJavaProcessBuilder("-Djdk.lang.Process.launchMechanism=posix_spawn",
"JspawnhelperProtocol",
"simulateTruncatedWriteInParent" + stage);
pb.environment().put(ENV_KEY, Integer.toString(stage));
Process p = pb.start();
BufferedReader br = p.inputReader();
String line = br.readLine();
while (line != null && !line.startsWith("posix_spawn:")) {
System.out.println(line);
line = br.readLine();
}
if (line == null) {
throw new Exception("Wrong output from parent process");
}
System.out.println(line);
if (!p.waitFor(TIMEOUT, TimeUnit.SECONDS)) {
throw new Exception("Parent process timed out");
}
line = br.readLine();
while (line != null) {
System.out.println(line);
line = br.readLine();
}
int ret = p.exitValue();
if (ret != ERROR) {
throw new Exception("Expected exit code " + ERROR + " but got " + ret);
}
System.out.println("Parent exit code: " + ret);
}
public static void main(String[] args) throws Exception {
// This test works as follows:
// - jtreg executes the test class `JspawnhelperProtocol` without arguments.
// This is the initial "grandparent" process.
// - For each sub-test (i.e. `normalExec()`, `simulateCrashInParent()` and
// `simulateCrashInChild()`), a new sub-process (called the "parent") will be
// forked which executes `JspawnhelperProtocol` recursively with a corresponding
// command line argument.
// - The forked `JspawnhelperProtocol` process (i.e. the "parent") runs
// `JspawnhelperProtocol::parentCode()` which forks off yet another sub-process
// (called the "child").
// - The sub-tests in the "grandparent" check that various abnormal program
// terminations in the "parent" or the "child" process are handled gracefully and
// don't lead to deadlocks or zombie processes.
if (args.length > 0) {
// Entry point for recursive execution in the "parent" process
parentCode(args[0]);
} else {
// Main test entry for execution from jtreg
normalExec();
simulateCrashInParent(1);
simulateCrashInParent(2);
simulateCrashInParent(3);
simulateCrashInChild(4);
simulateCrashInChild(5);
simulateCrashInChild(6);
simulateTruncatedWriteInParent(99);
}
}
}