From c566b43d02596cba437e9a2723e9f989297cca72 Mon Sep 17 00:00:00 2001 From: Amith Yamasani Date: Fri, 30 Nov 2012 15:26:21 -0800 Subject: Fix crosstalk between users for widgets hosted in lockscreen This was initially about the Clock widget crashing repeatedly on some devices with multiple users. Turned out that there were race conditions when switching users that could result in remote views of one user calling back to the RemoteViewsAdapter in keyguard that in turn sent an incorrect widget id to a different user's widget, resulting in a crash. Since KeyguardHostView is instantiated in the same process for different users, it needs to carry a user identity to pass along to AppWidgetService so that remote views services were bound to the correct user and callbacks were attached and detached properly. Added some aidl calls that take the userId to do the binding properly. A more complete fix might be needed in the future so that all calls from Keyguard carry the user id. Also, there was a problem in comparing host uid for secondary users, since Settings for a secondary user has a different uid than keyguard. Not an issue on single-user systems. Changed the host.uid comparison to accomodate for the secondary user. Bug: 7450247 Change-Id: Idbc36e3c60023cac74174f6cb7f2b2130dd3052c --- .../java/com/android/server/AppWidgetService.java | 42 +++++++++++++++++++--- .../com/android/server/AppWidgetServiceImpl.java | 38 ++++++++------------ 2 files changed, 51 insertions(+), 29 deletions(-) (limited to 'services/java') diff --git a/services/java/com/android/server/AppWidgetService.java b/services/java/com/android/server/AppWidgetService.java index b94183c..06aeb29 100644 --- a/services/java/com/android/server/AppWidgetService.java +++ b/services/java/com/android/server/AppWidgetService.java @@ -196,9 +196,14 @@ class AppWidgetService extends IAppWidgetService.Stub } @Override - public void bindRemoteViewsService(int appWidgetId, Intent intent, IBinder connection) - throws RemoteException { - getImplForUser(getCallingOrCurrentUserId()).bindRemoteViewsService( + public void bindRemoteViewsService(int appWidgetId, Intent intent, IBinder connection, + int userId) throws RemoteException { + if (Binder.getCallingPid() != android.os.Process.myPid() + && userId != UserHandle.getCallingUserId()) { + throw new SecurityException("Call from non-system process. Calling uid = " + + Binder.getCallingUid()); + } + getImplForUser(userId).bindRemoteViewsService( appWidgetId, intent, connection); } @@ -209,6 +214,17 @@ class AppWidgetService extends IAppWidgetService.Stub packageName, hostId, updatedViews); } + @Override + public int[] startListeningAsUser(IAppWidgetHost host, String packageName, int hostId, + List updatedViews, int userId) throws RemoteException { + if (Binder.getCallingPid() != android.os.Process.myPid() + && userId != UserHandle.getCallingUserId()) { + throw new SecurityException("Call from non-system process. Calling uid = " + + Binder.getCallingUid()); + } + return getImplForUser(userId).startListening(host, packageName, hostId, updatedViews); + } + public void onUserRemoved(int userId) { if (userId < 1) return; synchronized (mAppWidgetServices) { @@ -306,8 +322,24 @@ class AppWidgetService extends IAppWidgetService.Stub } @Override - public void unbindRemoteViewsService(int appWidgetId, Intent intent) throws RemoteException { - getImplForUser(getCallingOrCurrentUserId()).unbindRemoteViewsService( + public void stopListeningAsUser(int hostId, int userId) throws RemoteException { + if (Binder.getCallingPid() != android.os.Process.myPid() + && userId != UserHandle.getCallingUserId()) { + throw new SecurityException("Call from non-system process. Calling uid = " + + Binder.getCallingUid()); + } + getImplForUser(userId).stopListening(hostId); + } + + @Override + public void unbindRemoteViewsService(int appWidgetId, Intent intent, int userId) + throws RemoteException { + if (Binder.getCallingPid() != android.os.Process.myPid() + && userId != UserHandle.getCallingUserId()) { + throw new SecurityException("Call from non-system process. Calling uid = " + + Binder.getCallingUid()); + } + getImplForUser(userId).unbindRemoteViewsService( appWidgetId, intent); } diff --git a/services/java/com/android/server/AppWidgetServiceImpl.java b/services/java/com/android/server/AppWidgetServiceImpl.java index 854185d..e1e9eaf 100644 --- a/services/java/com/android/server/AppWidgetServiceImpl.java +++ b/services/java/com/android/server/AppWidgetServiceImpl.java @@ -116,6 +116,15 @@ class AppWidgetServiceImpl { boolean zombie; // if we're in safe mode, don't prune this just because nobody references it int tag; // for use while saving state (the index) + + boolean uidMatches(int callingUid) { + if (UserHandle.getAppId(callingUid) == Process.myUid()) { + // For a host that's in the system process, ignore the user id + return UserHandle.isSameApp(this.uid, callingUid); + } else { + return this.uid == callingUid; + } + } } static class AppWidgetId { @@ -476,7 +485,7 @@ class AppWidgetServiceImpl { boolean changed = false; for (int i = N - 1; i >= 0; i--) { Host host = mHosts.get(i); - if (host.uid == callingUid) { + if (host.uidMatches(callingUid)) { deleteHostLocked(host); changed = true; } @@ -744,8 +753,6 @@ class AppWidgetServiceImpl { conn.disconnect(); mContext.unbindService(conn); mBoundRemoteViewsServices.remove(key); - } else { - Log.e("AppWidgetService", "Error (unbindRemoteViewsService): Connection not bound"); } } } @@ -968,13 +975,8 @@ class AppWidgetServiceImpl { for (int i = 0; i < N; i++) { AppWidgetId id = lookupAppWidgetIdLocked(appWidgetIds[i]); if (id == null) { - String message = "AppWidgetId NPE: mUserId=" + mUserId - + ", callingUid=" + Binder.getCallingUid() - + ", appWidgetIds[i]=" + appWidgetIds[i] - + "\n mAppWidgets:\n" + getUserWidgets(); - throw new NullPointerException(message); - } - if (id.views != null) { + Slog.w(TAG, "widget id " + appWidgetIds[i] + " not found!"); + } else if (id.views != null) { // Only trigger a partial update for a widget if it has received a full update updateAppWidgetInstanceLocked(id, views, true); } @@ -982,18 +984,6 @@ class AppWidgetServiceImpl { } } - private String getUserWidgets() { - StringBuffer sb = new StringBuffer(); - for (AppWidgetId widget: mAppWidgetIds) { - sb.append(" id="); sb.append(widget.appWidgetId); - sb.append(", hostUid="); sb.append(widget.host.uid); - sb.append(", provider="); sb.append(widget.provider.info.provider.toString()); - sb.append("\n"); - } - sb.append("\n"); - return sb.toString(); - } - public void notifyAppWidgetViewDataChanged(int[] appWidgetIds, int viewId) { if (appWidgetIds == null) { return; @@ -1186,7 +1176,7 @@ class AppWidgetServiceImpl { } boolean canAccessAppWidgetId(AppWidgetId id, int callingUid) { - if (id.host.uid == callingUid) { + if (id.host.uidMatches(callingUid)) { // Apps hosting the AppWidget have access to it. return true; } @@ -1229,7 +1219,7 @@ class AppWidgetServiceImpl { final int N = mHosts.size(); for (int i = 0; i < N; i++) { Host h = mHosts.get(i); - if (h.uid == uid && h.hostId == hostId) { + if (h.uidMatches(uid) && h.hostId == hostId) { return h; } } -- cgit v1.1