Loading core/java/android/os/BugreportManager.java +8 −4 Original line number Diff line number Diff line Loading @@ -26,7 +26,6 @@ import android.annotation.SystemApi; import android.annotation.SystemService; import android.app.ActivityManager; import android.content.Context; import android.os.Handler; import android.util.Log; import android.widget.Toast; Loading Loading @@ -189,13 +188,18 @@ public final class BugreportManager { } } /* * Cancels a currently running bugreport. /** * Cancels the currently running bugreport. * * <p>Apps are only able to cancel their own bugreports. App A cannot cancel a bugreport started * by app B. * * @throws SecurityException if trying to cancel another app's bugreport in progress */ @RequiresPermission(android.Manifest.permission.DUMP) public void cancelBugreport() { try { mBinder.cancelBugreport(); mBinder.cancelBugreport(-1 /* callingUid */, mContext.getOpPackageName()); } catch (RemoteException e) { throw e.rethrowFromSystemServer(); } Loading core/tests/bugreports/src/com/android/os/bugreports/tests/BugreportManagerTest.java +120 −5 Original line number Diff line number Diff line Loading @@ -23,7 +23,10 @@ import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; import android.Manifest; import android.content.BroadcastReceiver; import android.content.Context; import android.content.Intent; import android.content.IntentFilter; import android.os.BugreportManager; import android.os.BugreportManager.BugreportCallback; import android.os.BugreportParams; Loading @@ -31,7 +34,9 @@ import android.os.FileUtils; import android.os.Handler; import android.os.HandlerThread; import android.os.ParcelFileDescriptor; import android.os.Process; import android.os.StrictMode; import android.text.TextUtils; import android.util.Log; import androidx.annotation.NonNull; Loading @@ -53,10 +58,11 @@ import org.junit.runner.RunWith; import org.junit.runners.JUnit4; import java.io.File; import java.io.IOException; import java.util.concurrent.CountDownLatch; import java.util.concurrent.Executor; import java.util.concurrent.TimeUnit; /** * Tests for BugreportManager API. */ Loading @@ -67,8 +73,16 @@ public class BugreportManagerTest { private static final String TAG = "BugreportManagerTest"; private static final long BUGREPORT_TIMEOUT_MS = TimeUnit.MINUTES.toMillis(10); private static final long DUMPSTATE_STARTUP_TIMEOUT_MS = TimeUnit.SECONDS.toMillis(10); private static final long UIAUTOMATOR_TIMEOUT_MS = TimeUnit.SECONDS.toMillis(10); // Sent by Shell when its bugreport finishes (contains final bugreport/screenshot file name // associated with the bugreport). private static final String INTENT_BUGREPORT_FINISHED = "com.android.internal.intent.action.BUGREPORT_FINISHED"; private static final String EXTRA_BUGREPORT = "android.intent.extra.BUGREPORT"; private static final String EXTRA_SCREENSHOT = "android.intent.extra.SCREENSHOT"; private Handler mHandler; private Executor mExecutor; private BugreportManager mBrm; Loading Loading @@ -211,6 +225,48 @@ public class BugreportManagerTest { assertFdsAreClosed(mBugreportFd, mScreenshotFd); } @Test public void cancelBugreport_noReportStarted() throws Exception { // Without the native DumpstateService running, we don't get a SecurityException. mBrm.cancelBugreport(); } @LargeTest @Test public void cancelBugreport_fromDifferentUid() throws Exception { assertThat(Process.myUid()).isNotEqualTo(Process.SHELL_UID); // Start a bugreport through ActivityManager's shell command - this starts a BR from the // shell UID rather than our own. BugreportBroadcastReceiver br = new BugreportBroadcastReceiver(); InstrumentationRegistry.getContext() .registerReceiver(br, new IntentFilter(INTENT_BUGREPORT_FINISHED)); UiDevice.getInstance(InstrumentationRegistry.getInstrumentation()) .executeShellCommand("am bug-report"); // The command triggers the report through a broadcast, so wait until dumpstate actually // starts up, which may take a bit. waitTillDumpstateRunningOrTimeout(); try { mBrm.cancelBugreport(); fail("Expected cancelBugreport to throw SecurityException when report started by " + "different UID"); } catch (SecurityException expected) { } finally { // Do this in the finally block so that even if this test case fails, we don't break // other test cases unexpectedly due to the still-running shell report. try { // The shell's BR is still running and should complete successfully. br.waitForBugreportFinished(); } finally { // The latch may fail for a number of reasons but we still need to unregister the // BroadcastReceiver. InstrumentationRegistry.getContext().unregisterReceiver(br); } } } @Test public void insufficientPermissions_throwsException() throws Exception { dropPermissions(); Loading Loading @@ -347,6 +403,28 @@ public class BugreportManagerTest { .adoptShellPermissionIdentity(Manifest.permission.DUMP); } private static boolean isDumpstateRunning() { String[] output; try { output = UiDevice.getInstance(InstrumentationRegistry.getInstrumentation()) .executeShellCommand("ps -A -o NAME | grep dumpstate") .trim() .split("\n"); } catch (IOException e) { Log.w(TAG, "Failed to check if dumpstate is running", e); return false; } for (String line : output) { // Check for an exact match since there may be other things that contain "dumpstate" as // a substring (e.g. the dumpstate HAL). if (TextUtils.equals("dumpstate", line)) { return true; } } return false; } private static void assertFdIsClosed(ParcelFileDescriptor pfd) { try { int fd = pfd.getFd(); Loading @@ -365,18 +443,25 @@ public class BugreportManagerTest { return System.currentTimeMillis(); } private static boolean shouldTimeout(long startTimeMs) { return now() - startTimeMs >= BUGREPORT_TIMEOUT_MS; private static void waitTillDumpstateRunningOrTimeout() throws Exception { long startTimeMs = now(); while (!isDumpstateRunning()) { Thread.sleep(500 /* .5s */); if (now() - startTimeMs >= DUMPSTATE_STARTUP_TIMEOUT_MS) { break; } Log.d(TAG, "Waited " + (now() - startTimeMs) + "ms for dumpstate to start"); } } private static void waitTillDoneOrTimeout(BugreportCallbackImpl callback) throws Exception { long startTimeMs = now(); while (!callback.isDone()) { Thread.sleep(1000 /* 1s */); if (shouldTimeout(startTimeMs)) { if (now() - startTimeMs >= BUGREPORT_TIMEOUT_MS) { break; } Log.d(TAG, "Waited " + (now() - startTimeMs) + "ms"); Log.d(TAG, "Waited " + (now() - startTimeMs) + "ms for bugreport to finish"); } } Loading Loading @@ -451,6 +536,36 @@ public class BugreportManagerTest { assertTrue(device.wait(Until.gone(consentTitleObj), UIAUTOMATOR_TIMEOUT_MS)); } private class BugreportBroadcastReceiver extends BroadcastReceiver { Intent mBugreportFinishedIntent = null; final CountDownLatch mLatch; BugreportBroadcastReceiver() { mLatch = new CountDownLatch(1); } @Override public void onReceive(Context context, Intent intent) { setBugreportFinishedIntent(intent); mLatch.countDown(); } private void setBugreportFinishedIntent(Intent intent) { mBugreportFinishedIntent = intent; } public Intent getBugreportFinishedIntent() { return mBugreportFinishedIntent; } public void waitForBugreportFinished() throws Exception { if (!mLatch.await(BUGREPORT_TIMEOUT_MS, TimeUnit.MILLISECONDS)) { throw new Exception("Failed to receive BUGREPORT_FINISHED in " + BUGREPORT_TIMEOUT_MS + " ms."); } } } /** * A rule to change strict mode vm policy temporarily till test method finished. * Loading services/core/java/com/android/server/os/BugreportManagerServiceImpl.java +10 −3 Original line number Diff line number Diff line Loading @@ -94,9 +94,12 @@ class BugreportManagerServiceImpl extends IDumpstate.Stub { @Override @RequiresPermission(android.Manifest.permission.DUMP) public void cancelBugreport() { public void cancelBugreport(int callingUidUnused, String callingPackage) { mContext.enforceCallingOrSelfPermission(android.Manifest.permission.DUMP, "cancelBugreport"); int callingUid = Binder.getCallingUid(); mAppOps.checkPackage(callingUid, callingPackage); synchronized (mLock) { IDumpstate ds = getDumpstateBinderServiceLocked(); if (ds == null) { Loading @@ -104,7 +107,11 @@ class BugreportManagerServiceImpl extends IDumpstate.Stub { return; } try { ds.cancelBugreport(); // Note: this may throw SecurityException back out to the caller if they aren't // allowed to cancel the report, in which case we should NOT be setting ctl.stop, // since that would unintentionally kill some other app's bugreport, which we // specifically disallow. ds.cancelBugreport(callingUid, callingPackage); } catch (RemoteException e) { Slog.e(TAG, "RemoteException in cancelBugreport", e); } Loading Loading @@ -182,7 +189,7 @@ class BugreportManagerServiceImpl extends IDumpstate.Stub { // lifecycle correctly. If we don't subsequent callers will get // BUGREPORT_ERROR_ANOTHER_REPORT_IN_PROGRESS error. // Note that listener will be notified by the death recipient below. cancelBugreport(); cancelBugreport(callingUid, callingPackage); } } Loading Loading
core/java/android/os/BugreportManager.java +8 −4 Original line number Diff line number Diff line Loading @@ -26,7 +26,6 @@ import android.annotation.SystemApi; import android.annotation.SystemService; import android.app.ActivityManager; import android.content.Context; import android.os.Handler; import android.util.Log; import android.widget.Toast; Loading Loading @@ -189,13 +188,18 @@ public final class BugreportManager { } } /* * Cancels a currently running bugreport. /** * Cancels the currently running bugreport. * * <p>Apps are only able to cancel their own bugreports. App A cannot cancel a bugreport started * by app B. * * @throws SecurityException if trying to cancel another app's bugreport in progress */ @RequiresPermission(android.Manifest.permission.DUMP) public void cancelBugreport() { try { mBinder.cancelBugreport(); mBinder.cancelBugreport(-1 /* callingUid */, mContext.getOpPackageName()); } catch (RemoteException e) { throw e.rethrowFromSystemServer(); } Loading
core/tests/bugreports/src/com/android/os/bugreports/tests/BugreportManagerTest.java +120 −5 Original line number Diff line number Diff line Loading @@ -23,7 +23,10 @@ import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; import android.Manifest; import android.content.BroadcastReceiver; import android.content.Context; import android.content.Intent; import android.content.IntentFilter; import android.os.BugreportManager; import android.os.BugreportManager.BugreportCallback; import android.os.BugreportParams; Loading @@ -31,7 +34,9 @@ import android.os.FileUtils; import android.os.Handler; import android.os.HandlerThread; import android.os.ParcelFileDescriptor; import android.os.Process; import android.os.StrictMode; import android.text.TextUtils; import android.util.Log; import androidx.annotation.NonNull; Loading @@ -53,10 +58,11 @@ import org.junit.runner.RunWith; import org.junit.runners.JUnit4; import java.io.File; import java.io.IOException; import java.util.concurrent.CountDownLatch; import java.util.concurrent.Executor; import java.util.concurrent.TimeUnit; /** * Tests for BugreportManager API. */ Loading @@ -67,8 +73,16 @@ public class BugreportManagerTest { private static final String TAG = "BugreportManagerTest"; private static final long BUGREPORT_TIMEOUT_MS = TimeUnit.MINUTES.toMillis(10); private static final long DUMPSTATE_STARTUP_TIMEOUT_MS = TimeUnit.SECONDS.toMillis(10); private static final long UIAUTOMATOR_TIMEOUT_MS = TimeUnit.SECONDS.toMillis(10); // Sent by Shell when its bugreport finishes (contains final bugreport/screenshot file name // associated with the bugreport). private static final String INTENT_BUGREPORT_FINISHED = "com.android.internal.intent.action.BUGREPORT_FINISHED"; private static final String EXTRA_BUGREPORT = "android.intent.extra.BUGREPORT"; private static final String EXTRA_SCREENSHOT = "android.intent.extra.SCREENSHOT"; private Handler mHandler; private Executor mExecutor; private BugreportManager mBrm; Loading Loading @@ -211,6 +225,48 @@ public class BugreportManagerTest { assertFdsAreClosed(mBugreportFd, mScreenshotFd); } @Test public void cancelBugreport_noReportStarted() throws Exception { // Without the native DumpstateService running, we don't get a SecurityException. mBrm.cancelBugreport(); } @LargeTest @Test public void cancelBugreport_fromDifferentUid() throws Exception { assertThat(Process.myUid()).isNotEqualTo(Process.SHELL_UID); // Start a bugreport through ActivityManager's shell command - this starts a BR from the // shell UID rather than our own. BugreportBroadcastReceiver br = new BugreportBroadcastReceiver(); InstrumentationRegistry.getContext() .registerReceiver(br, new IntentFilter(INTENT_BUGREPORT_FINISHED)); UiDevice.getInstance(InstrumentationRegistry.getInstrumentation()) .executeShellCommand("am bug-report"); // The command triggers the report through a broadcast, so wait until dumpstate actually // starts up, which may take a bit. waitTillDumpstateRunningOrTimeout(); try { mBrm.cancelBugreport(); fail("Expected cancelBugreport to throw SecurityException when report started by " + "different UID"); } catch (SecurityException expected) { } finally { // Do this in the finally block so that even if this test case fails, we don't break // other test cases unexpectedly due to the still-running shell report. try { // The shell's BR is still running and should complete successfully. br.waitForBugreportFinished(); } finally { // The latch may fail for a number of reasons but we still need to unregister the // BroadcastReceiver. InstrumentationRegistry.getContext().unregisterReceiver(br); } } } @Test public void insufficientPermissions_throwsException() throws Exception { dropPermissions(); Loading Loading @@ -347,6 +403,28 @@ public class BugreportManagerTest { .adoptShellPermissionIdentity(Manifest.permission.DUMP); } private static boolean isDumpstateRunning() { String[] output; try { output = UiDevice.getInstance(InstrumentationRegistry.getInstrumentation()) .executeShellCommand("ps -A -o NAME | grep dumpstate") .trim() .split("\n"); } catch (IOException e) { Log.w(TAG, "Failed to check if dumpstate is running", e); return false; } for (String line : output) { // Check for an exact match since there may be other things that contain "dumpstate" as // a substring (e.g. the dumpstate HAL). if (TextUtils.equals("dumpstate", line)) { return true; } } return false; } private static void assertFdIsClosed(ParcelFileDescriptor pfd) { try { int fd = pfd.getFd(); Loading @@ -365,18 +443,25 @@ public class BugreportManagerTest { return System.currentTimeMillis(); } private static boolean shouldTimeout(long startTimeMs) { return now() - startTimeMs >= BUGREPORT_TIMEOUT_MS; private static void waitTillDumpstateRunningOrTimeout() throws Exception { long startTimeMs = now(); while (!isDumpstateRunning()) { Thread.sleep(500 /* .5s */); if (now() - startTimeMs >= DUMPSTATE_STARTUP_TIMEOUT_MS) { break; } Log.d(TAG, "Waited " + (now() - startTimeMs) + "ms for dumpstate to start"); } } private static void waitTillDoneOrTimeout(BugreportCallbackImpl callback) throws Exception { long startTimeMs = now(); while (!callback.isDone()) { Thread.sleep(1000 /* 1s */); if (shouldTimeout(startTimeMs)) { if (now() - startTimeMs >= BUGREPORT_TIMEOUT_MS) { break; } Log.d(TAG, "Waited " + (now() - startTimeMs) + "ms"); Log.d(TAG, "Waited " + (now() - startTimeMs) + "ms for bugreport to finish"); } } Loading Loading @@ -451,6 +536,36 @@ public class BugreportManagerTest { assertTrue(device.wait(Until.gone(consentTitleObj), UIAUTOMATOR_TIMEOUT_MS)); } private class BugreportBroadcastReceiver extends BroadcastReceiver { Intent mBugreportFinishedIntent = null; final CountDownLatch mLatch; BugreportBroadcastReceiver() { mLatch = new CountDownLatch(1); } @Override public void onReceive(Context context, Intent intent) { setBugreportFinishedIntent(intent); mLatch.countDown(); } private void setBugreportFinishedIntent(Intent intent) { mBugreportFinishedIntent = intent; } public Intent getBugreportFinishedIntent() { return mBugreportFinishedIntent; } public void waitForBugreportFinished() throws Exception { if (!mLatch.await(BUGREPORT_TIMEOUT_MS, TimeUnit.MILLISECONDS)) { throw new Exception("Failed to receive BUGREPORT_FINISHED in " + BUGREPORT_TIMEOUT_MS + " ms."); } } } /** * A rule to change strict mode vm policy temporarily till test method finished. * Loading
services/core/java/com/android/server/os/BugreportManagerServiceImpl.java +10 −3 Original line number Diff line number Diff line Loading @@ -94,9 +94,12 @@ class BugreportManagerServiceImpl extends IDumpstate.Stub { @Override @RequiresPermission(android.Manifest.permission.DUMP) public void cancelBugreport() { public void cancelBugreport(int callingUidUnused, String callingPackage) { mContext.enforceCallingOrSelfPermission(android.Manifest.permission.DUMP, "cancelBugreport"); int callingUid = Binder.getCallingUid(); mAppOps.checkPackage(callingUid, callingPackage); synchronized (mLock) { IDumpstate ds = getDumpstateBinderServiceLocked(); if (ds == null) { Loading @@ -104,7 +107,11 @@ class BugreportManagerServiceImpl extends IDumpstate.Stub { return; } try { ds.cancelBugreport(); // Note: this may throw SecurityException back out to the caller if they aren't // allowed to cancel the report, in which case we should NOT be setting ctl.stop, // since that would unintentionally kill some other app's bugreport, which we // specifically disallow. ds.cancelBugreport(callingUid, callingPackage); } catch (RemoteException e) { Slog.e(TAG, "RemoteException in cancelBugreport", e); } Loading Loading @@ -182,7 +189,7 @@ class BugreportManagerServiceImpl extends IDumpstate.Stub { // lifecycle correctly. If we don't subsequent callers will get // BUGREPORT_ERROR_ANOTHER_REPORT_IN_PROGRESS error. // Note that listener will be notified by the death recipient below. cancelBugreport(); cancelBugreport(callingUid, callingPackage); } } Loading