Merge "Add error handling and other improvements to Bugreporting API" am: cefdee8540 am: 98e6b6bb58 (7c9863e4) · Commits · e / os / android_frameworks_base

core/java/android/os/BugreportManager.java

+8 −10

Original line number	Original line	Diff line number	Diff line
	@@ -24,7 +24,8 @@ import android.annotation.RequiresPermission;
	import android.annotation.SystemApi;		import android.annotation.SystemApi;
	import android.annotation.SystemService;		import android.annotation.SystemService;
	import android.content.Context;		import android.content.Context;
	import android.os.IBinder.DeathRecipient;
			import com.android.internal.util.Preconditions;

	import java.io.FileDescriptor;		import java.io.FileDescriptor;
	import java.lang.annotation.Retention;		import java.lang.annotation.Retention;
	@@ -127,13 +128,16 @@ public class BugreportManager {
	@NonNull BugreportParams params,		@NonNull BugreportParams params,
	@NonNull @CallbackExecutor Executor executor,		@NonNull @CallbackExecutor Executor executor,
	@NonNull BugreportCallback callback) {		@NonNull BugreportCallback callback) {
	// TODO(b/111441001): Enforce android.Manifest.permission.DUMP if necessary.		Preconditions.checkNotNull(bugreportFd);
			Preconditions.checkNotNull(params);
			Preconditions.checkNotNull(executor);
			Preconditions.checkNotNull(callback);
	DumpstateListener dsListener = new DumpstateListener(executor, callback);		DumpstateListener dsListener = new DumpstateListener(executor, callback);
	try {		try {
	// Note: mBinder can get callingUid from the binder transaction.		// Note: mBinder can get callingUid from the binder transaction.
	mBinder.startBugreport(-1 /* callingUid */,		mBinder.startBugreport(-1 /* callingUid */,
	mContext.getOpPackageName(),		mContext.getOpPackageName(),
	(bugreportFd != null ? bugreportFd.getFileDescriptor() : new FileDescriptor()),		bugreportFd.getFileDescriptor(),
	(screenshotFd != null		(screenshotFd != null
	? screenshotFd.getFileDescriptor() : new FileDescriptor()),		? screenshotFd.getFileDescriptor() : new FileDescriptor()),
	params.getMode(), dsListener);		params.getMode(), dsListener);
	@@ -154,8 +158,7 @@ public class BugreportManager {
	}		}
	}		}

	private final class DumpstateListener extends IDumpstateListener.Stub		private final class DumpstateListener extends IDumpstateListener.Stub {
	implements DeathRecipient {
	private final Executor mExecutor;		private final Executor mExecutor;
	private final BugreportCallback mCallback;		private final BugreportCallback mCallback;

	@@ -164,11 +167,6 @@ public class BugreportManager {
	mCallback = callback;		mCallback = callback;
	}		}

	@Override
	public void binderDied() {
	// TODO(b/111441001): implement
	}

	@Override		@Override
	public void onProgress(int progress) throws RemoteException {		public void onProgress(int progress) throws RemoteException {
	final long identity = Binder.clearCallingIdentity();		final long identity = Binder.clearCallingIdentity();

services/core/java/com/android/server/os/BugreportManagerServiceImpl.java

+110 −29

Original line number	Original line	Diff line number	Diff line
	@@ -17,8 +17,10 @@
	package com.android.server.os;		package com.android.server.os;

	import android.annotation.RequiresPermission;		import android.annotation.RequiresPermission;
			import android.app.ActivityManager;
	import android.app.AppOpsManager;		import android.app.AppOpsManager;
	import android.content.Context;		import android.content.Context;
			import android.content.pm.UserInfo;
	import android.os.Binder;		import android.os.Binder;
	import android.os.BugreportParams;		import android.os.BugreportParams;
	import android.os.IDumpstate;		import android.os.IDumpstate;
	@@ -28,26 +30,29 @@ import android.os.RemoteException;
	import android.os.ServiceManager;		import android.os.ServiceManager;
	import android.os.SystemClock;		import android.os.SystemClock;
	import android.os.SystemProperties;		import android.os.SystemProperties;
			import android.os.UserManager;
	import android.util.Slog;		import android.util.Slog;

			import com.android.internal.annotations.GuardedBy;
			import com.android.internal.util.Preconditions;

	import java.io.FileDescriptor;		import java.io.FileDescriptor;

	// TODO(b/111441001):		// TODO(b/111441001):
	// 1. Handle the case where another bugreport is in progress		// Intercept onFinished() & implement death recipient here and shutdown
	// 2. Make everything threadsafe		// bugreportd service.
	// 3. Pass validation & other errors on listener

	/**		/**
	* Implementation of the service that provides a privileged API to capture and consume bugreports.		* Implementation of the service that provides a privileged API to capture and consume bugreports.
	*		*
	* <p>Delegates the actualy generation to a native implementation of {@code Dumpstate}.		* <p>Delegates the actualy generation to a native implementation of {@code IDumpstate}.
	*/		*/
	class BugreportManagerServiceImpl extends IDumpstate.Stub {		class BugreportManagerServiceImpl extends IDumpstate.Stub {
	private static final String TAG = "BugreportManagerService";		private static final String TAG = "BugreportManagerService";
	private static final String BUGREPORT_SERVICE = "bugreportd";		private static final String BUGREPORT_SERVICE = "bugreportd";
	private static final long DEFAULT_BUGREPORT_SERVICE_TIMEOUT_MILLIS = 30 * 1000;		private static final long DEFAULT_BUGREPORT_SERVICE_TIMEOUT_MILLIS = 30 * 1000;

	private IDumpstate mDs = null;		private final Object mLock = new Object();
	private final Context mContext;		private final Context mContext;
	private final AppOpsManager mAppOps;		private final AppOpsManager mAppOps;

	@@ -59,43 +64,44 @@ class BugreportManagerServiceImpl extends IDumpstate.Stub {
	@Override		@Override
	@RequiresPermission(android.Manifest.permission.DUMP)		@RequiresPermission(android.Manifest.permission.DUMP)
	public IDumpstateToken setListener(String name, IDumpstateListener listener,		public IDumpstateToken setListener(String name, IDumpstateListener listener,
	boolean getSectionDetails) throws RemoteException {		boolean getSectionDetails) {
	// TODO(b/111441001): Figure out if lazy setting of listener should be allowed
	// and if so how to handle it.
	throw new UnsupportedOperationException("setListener is not allowed on this service");		throw new UnsupportedOperationException("setListener is not allowed on this service");
	}		}

	// TODO(b/111441001): Intercept onFinished here in system server and shutdown
	// the bugreportd service.
	@Override		@Override
	@RequiresPermission(android.Manifest.permission.DUMP)		@RequiresPermission(android.Manifest.permission.DUMP)
	public void startBugreport(int callingUidUnused, String callingPackage,		public void startBugreport(int callingUidUnused, String callingPackage,
	FileDescriptor bugreportFd, FileDescriptor screenshotFd,		FileDescriptor bugreportFd, FileDescriptor screenshotFd,
	int bugreportMode, IDumpstateListener listener) throws RemoteException {		int bugreportMode, IDumpstateListener listener) {
	int callingUid = Binder.getCallingUid();		mContext.enforceCallingOrSelfPermission(android.Manifest.permission.DUMP, "startBugreport");
	// TODO(b/111441001): validate all arguments & ensure primary user		Preconditions.checkNotNull(callingPackage);
	validate(bugreportMode);		Preconditions.checkNotNull(bugreportFd);
			Preconditions.checkNotNull(listener);
			validateBugreportMode(bugreportMode);
			ensureIsPrimaryUser();

			int callingUid = Binder.getCallingUid();
	mAppOps.checkPackage(callingUid, callingPackage);		mAppOps.checkPackage(callingUid, callingPackage);
	mDs = getDumpstateService();
	if (mDs == null) {		synchronized (mLock) {
	Slog.w(TAG, "Unable to get bugreport service");		startBugreportLocked(callingUid, callingPackage, bugreportFd, screenshotFd,
	// TODO(b/111441001): pass error on listener		bugreportMode, listener);
	return;
	}		}
	mDs.startBugreport(callingUid, callingPackage,
	bugreportFd, screenshotFd, bugreportMode, listener);
	}		}

	@Override		@Override
	@RequiresPermission(android.Manifest.permission.DUMP)		@RequiresPermission(android.Manifest.permission.DUMP)
	public void cancelBugreport() throws RemoteException {		public void cancelBugreport() {
	// This tells init to cancel bugreportd service.		mContext.enforceCallingOrSelfPermission(android.Manifest.permission.DUMP, "startBugreport");
			// This tells init to cancel bugreportd service. Note that this is achieved through setting
			// a system property which is not thread-safe. So the lock here offers thread-safety only
			// among callers of the API.
			synchronized (mLock) {
	SystemProperties.set("ctl.stop", BUGREPORT_SERVICE);		SystemProperties.set("ctl.stop", BUGREPORT_SERVICE);
	mDs = null;		}
	}		}

	private boolean validate(@BugreportParams.BugreportMode int mode) {		private void validateBugreportMode(@BugreportParams.BugreportMode int mode) {
	if (mode != BugreportParams.BUGREPORT_MODE_FULL		if (mode != BugreportParams.BUGREPORT_MODE_FULL
	&& mode != BugreportParams.BUGREPORT_MODE_INTERACTIVE		&& mode != BugreportParams.BUGREPORT_MODE_INTERACTIVE
	&& mode != BugreportParams.BUGREPORT_MODE_REMOTE		&& mode != BugreportParams.BUGREPORT_MODE_REMOTE
	@@ -103,9 +109,66 @@ class BugreportManagerServiceImpl extends IDumpstate.Stub {
	&& mode != BugreportParams.BUGREPORT_MODE_TELEPHONY		&& mode != BugreportParams.BUGREPORT_MODE_TELEPHONY
	&& mode != BugreportParams.BUGREPORT_MODE_WIFI) {		&& mode != BugreportParams.BUGREPORT_MODE_WIFI) {
	Slog.w(TAG, "Unknown bugreport mode: " + mode);		Slog.w(TAG, "Unknown bugreport mode: " + mode);
	return false;		throw new IllegalArgumentException("Unknown bugreport mode: " + mode);
	}		}
	return true;		}

			/**
			* Validates that the current user is the primary user.
			*
			* @throws IllegalArgumentException if the current user is not the primary user
			*/
			private void ensureIsPrimaryUser() {
			UserInfo currentUser = null;
			try {
			currentUser = ActivityManager.getService().getCurrentUser();
			} catch (RemoteException e) {
			// Impossible to get RemoteException for an in-process call.
			}

			UserInfo primaryUser = UserManager.get(mContext).getPrimaryUser();
			if (currentUser == null) {
			logAndThrow("No current user. Only primary user is allowed to take bugreports.");
			}
			if (primaryUser == null) {
			logAndThrow("No primary user. Only primary user is allowed to take bugreports.");
			}
			if (primaryUser.id != currentUser.id) {
			logAndThrow("Current user not primary user. Only primary user"
			+ " is allowed to take bugreports.");
			}
			}

			@GuardedBy("mLock")
			private void startBugreportLocked(int callingUid, String callingPackage,
			FileDescriptor bugreportFd, FileDescriptor screenshotFd,
			int bugreportMode, IDumpstateListener listener) {
			if (isDumpstateBinderServiceRunningLocked()) {
			Slog.w(TAG, "'dumpstate' is already running. Cannot start a new bugreport"
			+ " while another one is currently in progress.");
			// TODO(b/111441001): Use a new error code; add this to the documentation of the API.
			reportError(listener, IDumpstateListener.BUGREPORT_ERROR_RUNTIME_ERROR);
			return;
			}

			IDumpstate ds = startAndGetDumpstateBinderServiceLocked();
			if (ds == null) {
			Slog.w(TAG, "Unable to get bugreport service");
			reportError(listener, IDumpstateListener.BUGREPORT_ERROR_RUNTIME_ERROR);
			return;
			}
			try {
			ds.startBugreport(callingUid, callingPackage,
			bugreportFd, screenshotFd, bugreportMode, listener);
			} catch (RemoteException e) {
			reportError(listener, IDumpstateListener.BUGREPORT_ERROR_RUNTIME_ERROR);
			}
			}

			@GuardedBy("mLock")
			private boolean isDumpstateBinderServiceRunningLocked() {
			IDumpstate ds = IDumpstate.Stub.asInterface(ServiceManager.getService("dumpstate"));
			return ds != null;
	}		}

	/*		/*
	@@ -115,8 +178,12 @@ class BugreportManagerServiceImpl extends IDumpstate.Stub {
	* <p>Generating bugreports requires root privileges. To limit the footprint		* <p>Generating bugreports requires root privileges. To limit the footprint
	* of the root access, the actual generation in Dumpstate binary is accessed as a		* of the root access, the actual generation in Dumpstate binary is accessed as a
	* oneshot service 'bugreport'.		* oneshot service 'bugreport'.
			*
			* <p>Note that starting the service is achieved through setting a system property, which is
			* not thread-safe. So the lock here offers thread-safety only among callers of the API.
	*/		*/
	private IDumpstate getDumpstateService() {		@GuardedBy("mLock")
			private IDumpstate startAndGetDumpstateBinderServiceLocked() {
	// Start bugreport service.		// Start bugreport service.
	SystemProperties.set("ctl.start", BUGREPORT_SERVICE);		SystemProperties.set("ctl.start", BUGREPORT_SERVICE);

	@@ -145,4 +212,18 @@ class BugreportManagerServiceImpl extends IDumpstate.Stub {
	}		}
	return ds;		return ds;
	}		}

			private void reportError(IDumpstateListener listener, int errorCode) {
			try {
			listener.onError(errorCode);
			} catch (RemoteException e) {
			// Something went wrong in binder or app process. There's nothing to do here.
			Slog.w(TAG, "onError() transaction threw RemoteException: " + e.getMessage());
			}
			}

			private void logAndThrow(String message) {
			Slog.w(TAG, message);
			throw new IllegalArgumentException(message);
			}
	}		}