Skip to content
This repository has been archived by the owner on Sep 19, 2023. It is now read-only.
/ jdk19u Public archive

Commit

Permalink
8297451: ProcessHandleImpl should assert privilege when modifying rea…
Browse files Browse the repository at this point in the history
…per thread

Backport-of: 50f9043c6965360c426b187e47c49c42481a2549
  • Loading branch information
ChrisHegarty committed Nov 28, 2022
1 parent 0959a65 commit 998a6f5
Show file tree
Hide file tree
Showing 2 changed files with 36 additions and 7 deletions.
25 changes: 21 additions & 4 deletions src/java.base/share/classes/java/lang/ProcessHandleImpl.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
};

Expand All @@ -115,6 +115,22 @@ private static class ExitCompletion extends CompletableFuture<Integer> {
}
}

@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.
Expand All @@ -140,8 +156,9 @@ static CompletableFuture<Integer> completion(long pid, boolean shouldReap) {
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) {
Expand Down Expand Up @@ -172,7 +189,7 @@ public void run() {
completions.remove(pid, newCompletion);
} finally {
// Restore thread name
Thread.currentThread().setName(threadName);
privilegedThreadSetName(t, threadName);
}
}
});
Expand Down
18 changes: 15 additions & 3 deletions test/jdk/java/lang/ProcessBuilder/SecurityManagerClinit.java
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -52,6 +52,17 @@ public boolean implies(ProtectionDomain pd, Permission p) {
}
}

// 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") +
Expand All @@ -60,10 +71,11 @@ public static void main(String[] args) throws Throwable {
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" };
Expand Down

1 comment on commit 998a6f5

@openjdk-notifier
Copy link

Choose a reason for hiding this comment

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

Please sign in to comment.