8297451: ProcessHandleImpl should assert privilege when modifying reaper thread

Reviewed-by: chegar, alanb
This commit is contained in:
Ryan Ernst 2022-11-26 20:04:59 +00:00 committed by Chris Hegarty
parent 99d3840d36
commit 50f9043c69
2 changed files with 36 additions and 7 deletions
src/java.base/share/classes/java/lang
test/jdk/java/lang/ProcessBuilder

@ -100,7 +100,7 @@ final class ProcessHandleImpl implements ProcessHandle {
ThreadFactory threadFactory = grimReaper -> {
Thread t = InnocuousThread.newSystemThread("process reaper", grimReaper,
stackSize, Thread.MAX_PRIORITY);
t.setDaemon(true);
privilegedThreadSetDaemon(t, true);
return t;
};
@ -115,6 +115,22 @@ final class ProcessHandleImpl implements ProcessHandle {
}
}
@SuppressWarnings("removal")
private static void privilegedThreadSetName(Thread thread, String name) {
AccessController.doPrivileged((PrivilegedAction<Void>) () -> {
thread.setName(name);
return null;
});
}
@SuppressWarnings("removal")
private static void privilegedThreadSetDaemon(Thread thread, boolean on) {
AccessController.doPrivileged((PrivilegedAction<Void>) () -> {
thread.setDaemon(on);
return null;
});
}
/**
* Returns a CompletableFuture that completes with process exit status when
* the process completes.
@ -140,8 +156,9 @@ final class ProcessHandleImpl implements ProcessHandle {
processReaperExecutor.execute(new Runnable() {
// Use inner class to avoid lambda stack overhead
public void run() {
String threadName = Thread.currentThread().getName();
Thread.currentThread().setName("process reaper (pid " + pid + ")");
Thread t = Thread.currentThread();
String threadName = t.getName();
privilegedThreadSetName(t, "process reaper (pid " + pid + ")");
try {
int exitValue = waitForProcessExit0(pid, shouldReap);
if (exitValue == NOT_A_CHILD) {
@ -172,7 +189,7 @@ final class ProcessHandleImpl implements ProcessHandle {
completions.remove(pid, newCompletion);
} finally {
// Restore thread name
Thread.currentThread().setName(threadName);
privilegedThreadSetName(t, threadName);
}
}
});

@ -24,7 +24,7 @@
/*
* @test
* @bug 6980747
* @bug 6980747 8297451
* @summary Check that Process-related classes have the proper
* doPrivileged blocks, and can be initialized with an adversarial
* security manager.
@ -52,6 +52,17 @@ public class SecurityManagerClinit {
}
}
// Security manager that unconditionally performs Thread Modify Access checks.
@SuppressWarnings("removal")
private static class TMACSecurityManager extends SecurityManager {
static final RuntimePermission MODIFY_THREAD_PERMISSION =
new RuntimePermission("modifyThread");
@Override
public void checkAccess(Thread thread) {
checkPermission(MODIFY_THREAD_PERMISSION);
}
}
public static void main(String[] args) throws Throwable {
String javaExe =
System.getProperty("java.home") +
@ -60,10 +71,11 @@ public class SecurityManagerClinit {
final SimplePolicy policy =
new SimplePolicy
(new FilePermission("<<ALL FILES>>", "execute"),
new RuntimePermission("setSecurityManager"));
new RuntimePermission("setSecurityManager"),
new RuntimePermission("modifyThread"));
Policy.setPolicy(policy);
System.setSecurityManager(new SecurityManager());
System.setSecurityManager(new TMACSecurityManager());
try {
String[] cmd = { javaExe, "-version" };