RootShell: Use a lock object instead of sync methods

This avoids exposing the synchronization implementation details
in the class's interface.

Signed-off-by: Samuel Holland <samuel@sholland.org>
This commit is contained in:
Samuel Holland 2018-01-16 05:47:10 -06:00
parent 75aeec035c
commit d56eda2fd6

View File

@ -31,6 +31,7 @@ public class RootShell {
private final String deviceNotRootedMessage; private final String deviceNotRootedMessage;
private final File localBinaryDir; private final File localBinaryDir;
private final File localTemporaryDir; private final File localTemporaryDir;
private final Object lock = new Object();
private final String preamble; private final String preamble;
private Process process; private Process process;
private BufferedReader stderr; private BufferedReader stderr;
@ -57,16 +58,18 @@ public class RootShell {
return false; return false;
} }
public synchronized boolean isRunning() { private boolean isRunning() {
try { synchronized (lock) {
// Throws an exception if the process hasn't finished yet. try {
if (process != null) // Throws an exception if the process hasn't finished yet.
process.exitValue(); if (process != null)
} catch (final IllegalThreadStateException ignored) { process.exitValue();
// The existing process is still running. return false;
return true; } catch (final IllegalThreadStateException ignored) {
// The existing process is still running.
return true;
}
} }
return false;
} }
/** /**
@ -77,101 +80,108 @@ public class RootShell {
* @param command Command to run as root. * @param command Command to run as root.
* @return The exit value of the command. * @return The exit value of the command.
*/ */
public synchronized int run(final Collection<String> output, final String command) public int run(final Collection<String> output, final String command)
throws IOException, NoRootException { throws IOException, NoRootException {
start(); synchronized (lock) {
final String marker = UUID.randomUUID().toString(); /* Start inside synchronized block to prevent a concurrent call to stop(). */
final String script = "echo " + marker + "; echo " + marker + " >&2; (" + command + start();
"); ret=$?; echo " + marker + " $ret; echo " + marker + " $ret >&2\n"; final String marker = UUID.randomUUID().toString();
Log.v(TAG, "executing: " + command); final String script = "echo " + marker + "; echo " + marker + " >&2; (" + command +
stdin.write(script); "); ret=$?; echo " + marker + " $ret; echo " + marker + " $ret >&2\n";
stdin.flush(); Log.v(TAG, "executing: " + command);
String line; stdin.write(script);
int errnoStdout = Integer.MIN_VALUE; stdin.flush();
int errnoStderr = Integer.MAX_VALUE; String line;
int markersSeen = 0; int errnoStdout = Integer.MIN_VALUE;
while ((line = stdout.readLine()) != null) { int errnoStderr = Integer.MAX_VALUE;
if (line.startsWith(marker)) { int markersSeen = 0;
++markersSeen; while ((line = stdout.readLine()) != null) {
if (line.length() > marker.length() + 1) { if (line.startsWith(marker)) {
errnoStdout = Integer.valueOf(line.substring(marker.length() + 1)); ++markersSeen;
break; if (line.length() > marker.length() + 1) {
errnoStdout = Integer.valueOf(line.substring(marker.length() + 1));
break;
}
} else if (markersSeen > 0) {
if (output != null)
output.add(line);
Log.v(TAG, "stdout: " + line);
} }
} else if (markersSeen > 0) {
if (output != null)
output.add(line);
Log.v(TAG, "stdout: " + line);
} }
} while ((line = stderr.readLine()) != null) {
while ((line = stderr.readLine()) != null) { if (line.startsWith(marker)) {
if (line.startsWith(marker)) { ++markersSeen;
++markersSeen; if (line.length() > marker.length() + 1) {
if (line.length() > marker.length() + 1) { errnoStderr = Integer.valueOf(line.substring(marker.length() + 1));
errnoStderr = Integer.valueOf(line.substring(marker.length() + 1)); break;
break; }
} else if (markersSeen > 2) {
Log.v(TAG, "stderr: " + line);
} }
} else if (markersSeen > 2) {
Log.v(TAG, "stderr: " + line);
} }
if (markersSeen != 4)
throw new IOException("Expected 4 markers, received " + markersSeen);
if (errnoStdout != errnoStderr)
throw new IOException("Unable to read exit status");
Log.v(TAG, "exit: " + errnoStdout);
return errnoStdout;
} }
if (markersSeen != 4)
throw new IOException("Expected 4 markers, received " + markersSeen);
if (errnoStdout != errnoStderr)
throw new IOException("Unable to read exit status");
Log.v(TAG, "exit: " + errnoStdout);
return errnoStdout;
} }
public synchronized void start() throws IOException, NoRootException { public void start() throws IOException, NoRootException {
if (isRunning())
return;
if (!localBinaryDir.isDirectory() && !localBinaryDir.mkdirs())
throw new FileNotFoundException("Could not create local binary directory");
if (!localTemporaryDir.isDirectory() && !localTemporaryDir.mkdirs())
throw new FileNotFoundException("Could not create local temporary directory");
if (!isExecutableInPath(SU)) if (!isExecutableInPath(SU))
throw new NoRootException(deviceNotRootedMessage); throw new NoRootException(deviceNotRootedMessage);
try { synchronized (lock) {
final ProcessBuilder builder = new ProcessBuilder().command(SU); if (isRunning())
builder.environment().put("LC_ALL", "C"); return;
if (!localBinaryDir.isDirectory() && !localBinaryDir.mkdirs())
throw new FileNotFoundException("Could not create local binary directory");
if (!localTemporaryDir.isDirectory() && !localTemporaryDir.mkdirs())
throw new FileNotFoundException("Could not create local temporary directory");
try { try {
process = builder.start(); final ProcessBuilder builder = new ProcessBuilder().command(SU);
} catch (final IOException e) { builder.environment().put("LC_ALL", "C");
// A failure at this stage means the device isn't rooted. try {
throw new NoRootException(deviceNotRootedMessage, e); process = builder.start();
} } catch (final IOException e) {
stdin = new OutputStreamWriter(process.getOutputStream(), StandardCharsets.UTF_8); // A failure at this stage means the device isn't rooted.
stdout = new BufferedReader(new InputStreamReader(process.getInputStream(), throw new NoRootException(deviceNotRootedMessage, e);
StandardCharsets.UTF_8));
stderr = new BufferedReader(new InputStreamReader(process.getErrorStream(),
StandardCharsets.UTF_8));
stdin.write(preamble);
stdin.flush();
// Check that the shell started successfully.
final String uid = stdout.readLine();
if (!"0".equals(uid)) {
Log.w(TAG, "Root check did not return correct UID: " + uid);
throw new NoRootException(deviceNotRootedMessage);
}
if (!isRunning()) {
String line;
while ((line = stderr.readLine()) != null) {
Log.w(TAG, "Root check returned an error: " + line);
if (line.contains("Permission denied"))
throw new NoRootException(deviceNotRootedMessage);
} }
throw new IOException("Shell failed to start: " + process.exitValue()); stdin = new OutputStreamWriter(process.getOutputStream(), StandardCharsets.UTF_8);
stdout = new BufferedReader(new InputStreamReader(process.getInputStream(),
StandardCharsets.UTF_8));
stderr = new BufferedReader(new InputStreamReader(process.getErrorStream(),
StandardCharsets.UTF_8));
stdin.write(preamble);
stdin.flush();
// Check that the shell started successfully.
final String uid = stdout.readLine();
if (!"0".equals(uid)) {
Log.w(TAG, "Root check did not return correct UID: " + uid);
throw new NoRootException(deviceNotRootedMessage);
}
if (!isRunning()) {
String line;
while ((line = stderr.readLine()) != null) {
Log.w(TAG, "Root check returned an error: " + line);
if (line.contains("Permission denied"))
throw new NoRootException(deviceNotRootedMessage);
}
throw new IOException("Shell failed to start: " + process.exitValue());
}
} catch (final IOException | NoRootException e) {
stop();
throw e;
} }
} catch (final IOException | NoRootException e) {
stop();
throw e;
} }
} }
public synchronized void stop() throws IOException { public void stop() {
if (process != null) { synchronized (lock) {
process.destroy(); if (process != null) {
process = null; process.destroy();
process = null;
}
} }
} }