Skip to content

Commit c566b43

Browse files
author
Amith Yamasani
committed
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
1 parent 20b76b9 commit c566b43

File tree

7 files changed

+126
-41
lines changed

7 files changed

+126
-41
lines changed

‎core/java/android/appwidget/AppWidgetHost.java‎

Lines changed: 28 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -154,14 +154,24 @@ private static void bindService() {
154154
* becomes visible, i.e. from onStart() in your Activity.
155155
*/
156156
public void startListening() {
157+
startListeningAsUser(UserHandle.myUserId());
158+
}
159+
160+
/**
161+
* Start receiving onAppWidgetChanged calls for your AppWidgets. Call this when your activity
162+
* becomes visible, i.e. from onStart() in your Activity.
163+
* @hide
164+
*/
165+
public void startListeningAsUser(int userId) {
157166
int[] updatedIds;
158167
ArrayList<RemoteViews> updatedViews = new ArrayList<RemoteViews>();
159168

160169
try {
161170
if (mPackageName == null) {
162171
mPackageName = mContext.getPackageName();
163172
}
164-
updatedIds = sService.startListening(mCallbacks, mPackageName, mHostId, updatedViews);
173+
updatedIds = sService.startListeningAsUser(
174+
mCallbacks, mPackageName, mHostId, updatedViews, userId);
165175
}
166176
catch (RemoteException e) {
167177
throw new RuntimeException("system server dead?", e);
@@ -179,11 +189,27 @@ public void startListening() {
179189
*/
180190
public void stopListening() {
181191
try {
182-
sService.stopListening(mHostId);
192+
sService.stopListeningAsUser(mHostId, UserHandle.myUserId());
193+
}
194+
catch (RemoteException e) {
195+
throw new RuntimeException("system server dead?", e);
196+
}
197+
}
198+
199+
/**
200+
* Stop receiving onAppWidgetChanged calls for your AppWidgets. Call this when your activity is
201+
* no longer visible, i.e. from onStop() in your Activity.
202+
* @hide
203+
*/
204+
public void stopListeningAsUser(int userId) {
205+
try {
206+
sService.stopListeningAsUser(mHostId, userId);
183207
}
184208
catch (RemoteException e) {
185209
throw new RuntimeException("system server dead?", e);
186210
}
211+
// Also clear the views
212+
clearViews();
187213
}
188214

189215
/**

‎core/java/android/appwidget/AppWidgetManager.java‎

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
import android.os.IBinder;
2424
import android.os.RemoteException;
2525
import android.os.ServiceManager;
26+
import android.os.UserHandle;
2627
import android.util.DisplayMetrics;
2728
import android.util.TypedValue;
2829
import android.widget.RemoteViews;
@@ -749,11 +750,14 @@ public void setBindAppWidgetPermission(String packageName, boolean permission) {
749750
* @param intent The intent of the service which will be providing the data to the
750751
* RemoteViewsAdapter.
751752
* @param connection The callback interface to be notified when a connection is made or lost.
753+
* @param userHandle The user to bind to.
752754
* @hide
753755
*/
754-
public void bindRemoteViewsService(int appWidgetId, Intent intent, IBinder connection) {
756+
public void bindRemoteViewsService(int appWidgetId, Intent intent, IBinder connection,
757+
UserHandle userHandle) {
755758
try {
756-
sService.bindRemoteViewsService(appWidgetId, intent, connection);
759+
sService.bindRemoteViewsService(appWidgetId, intent, connection,
760+
userHandle.getIdentifier());
757761
}
758762
catch (RemoteException e) {
759763
throw new RuntimeException("system server dead?", e);
@@ -769,11 +773,12 @@ public void bindRemoteViewsService(int appWidgetId, Intent intent, IBinder conne
769773
* @param appWidgetId The AppWidget instance for which to bind the RemoteViewsService.
770774
* @param intent The intent of the service which will be providing the data to the
771775
* RemoteViewsAdapter.
776+
* @param userHandle The user to unbind from.
772777
* @hide
773778
*/
774-
public void unbindRemoteViewsService(int appWidgetId, Intent intent) {
779+
public void unbindRemoteViewsService(int appWidgetId, Intent intent, UserHandle userHandle) {
775780
try {
776-
sService.unbindRemoteViewsService(appWidgetId, intent);
781+
sService.unbindRemoteViewsService(appWidgetId, intent, userHandle.getIdentifier());
777782
}
778783
catch (RemoteException e) {
779784
throw new RuntimeException("system server dead?", e);

‎core/java/android/widget/RemoteViewsAdapter.java‎

Lines changed: 28 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,9 @@
2929
import android.os.IBinder;
3030
import android.os.Looper;
3131
import android.os.Message;
32+
import android.os.Process;
3233
import android.os.RemoteException;
34+
import android.os.UserHandle;
3335
import android.util.Log;
3436
import android.util.Pair;
3537
import android.view.LayoutInflater;
@@ -40,6 +42,7 @@
4042

4143
import com.android.internal.widget.IRemoteViewsAdapterConnection;
4244
import com.android.internal.widget.IRemoteViewsFactory;
45+
import com.android.internal.widget.LockPatternUtils;
4346

4447
/**
4548
* An adapter to a RemoteViewsService which fetches and caches RemoteViews
@@ -106,6 +109,8 @@ public class RemoteViewsAdapter extends BaseAdapter implements Handler.Callback
106109
// construction (happens when we have a cached FixedSizeRemoteViewsCache).
107110
private boolean mDataReady = false;
108111

112+
int mUserId;
113+
109114
/**
110115
* An interface for the RemoteAdapter to notify other classes when adapters
111116
* are actually connected to/disconnected from their actual services.
@@ -146,8 +151,16 @@ public RemoteViewsAdapterServiceConnection(RemoteViewsAdapter adapter) {
146151
public synchronized void bind(Context context, int appWidgetId, Intent intent) {
147152
if (!mIsConnecting) {
148153
try {
154+
RemoteViewsAdapter adapter;
149155
final AppWidgetManager mgr = AppWidgetManager.getInstance(context);
150-
mgr.bindRemoteViewsService(appWidgetId, intent, asBinder());
156+
if (Process.myUid() == Process.SYSTEM_UID
157+
&& (adapter = mAdapter.get()) != null) {
158+
mgr.bindRemoteViewsService(appWidgetId, intent, asBinder(),
159+
new UserHandle(adapter.mUserId));
160+
} else {
161+
mgr.bindRemoteViewsService(appWidgetId, intent, asBinder(),
162+
Process.myUserHandle());
163+
}
151164
mIsConnecting = true;
152165
} catch (Exception e) {
153166
Log.e("RemoteViewsAdapterServiceConnection", "bind(): " + e.getMessage());
@@ -159,8 +172,15 @@ public synchronized void bind(Context context, int appWidgetId, Intent intent) {
159172

160173
public synchronized void unbind(Context context, int appWidgetId, Intent intent) {
161174
try {
175+
RemoteViewsAdapter adapter;
162176
final AppWidgetManager mgr = AppWidgetManager.getInstance(context);
163-
mgr.unbindRemoteViewsService(appWidgetId, intent);
177+
if (Process.myUid() == Process.SYSTEM_UID
178+
&& (adapter = mAdapter.get()) != null) {
179+
mgr.unbindRemoteViewsService(appWidgetId, intent,
180+
new UserHandle(adapter.mUserId));
181+
} else {
182+
mgr.unbindRemoteViewsService(appWidgetId, intent, Process.myUserHandle());
183+
}
164184
mIsConnecting = false;
165185
} catch (Exception e) {
166186
Log.e("RemoteViewsAdapterServiceConnection", "unbind(): " + e.getMessage());
@@ -761,6 +781,12 @@ public RemoteViewsAdapter(Context context, Intent intent, RemoteAdapterConnectio
761781
}
762782
mRequestedViews = new RemoteViewsFrameLayoutRefSet();
763783

784+
if (Process.myUid() == Process.SYSTEM_UID) {
785+
mUserId = new LockPatternUtils(context).getCurrentUser();
786+
} else {
787+
mUserId = UserHandle.myUserId();
788+
}
789+
764790
// Strip the previously injected app widget id from service intent
765791
if (intent.hasExtra(RemoteViews.EXTRA_REMOTEADAPTER_APPWIDGET_ID)) {
766792
intent.removeExtra(RemoteViews.EXTRA_REMOTEADAPTER_APPWIDGET_ID);

‎core/java/com/android/internal/appwidget/IAppWidgetService.aidl‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,10 @@ interface IAppWidgetService {
3232
//
3333
int[] startListening(IAppWidgetHost host, String packageName, int hostId,
3434
out List<RemoteViews> updatedViews);
35+
int[] startListeningAsUser(IAppWidgetHost host, String packageName, int hostId,
36+
out List<RemoteViews> updatedViews, int userId);
3537
void stopListening(int hostId);
38+
void stopListeningAsUser(int hostId, int userId);
3639
int allocateAppWidgetId(String packageName, int hostId);
3740
void deleteAppWidgetId(int appWidgetId);
3841
void deleteHost(int hostId);
@@ -56,8 +59,8 @@ interface IAppWidgetService {
5659
void bindAppWidgetId(int appWidgetId, in ComponentName provider, in Bundle options);
5760
boolean bindAppWidgetIdIfAllowed(
5861
in String packageName, int appWidgetId, in ComponentName provider, in Bundle options);
59-
void bindRemoteViewsService(int appWidgetId, in Intent intent, in IBinder connection);
60-
void unbindRemoteViewsService(int appWidgetId, in Intent intent);
62+
void bindRemoteViewsService(int appWidgetId, in Intent intent, in IBinder connection, int userId);
63+
void unbindRemoteViewsService(int appWidgetId, in Intent intent, int userId);
6164
int[] getAppWidgetIds(in ComponentName provider);
6265

6366
}

‎policy/src/com/android/internal/policy/impl/keyguard/KeyguardHostView.java‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,8 @@ public class KeyguardHostView extends KeyguardViewBase {
101101
private boolean mSafeModeEnabled;
102102

103103
private boolean mUserSetupCompleted;
104+
// User for whom this host view was created
105+
private int mUserId;
104106

105107
/*package*/ interface TransportCallback {
106108
void onListenerDetached();
@@ -127,6 +129,7 @@ public KeyguardHostView(Context context) {
127129
public KeyguardHostView(Context context, AttributeSet attrs) {
128130
super(context, attrs);
129131
mLockPatternUtils = new LockPatternUtils(context);
132+
mUserId = mLockPatternUtils.getCurrentUser();
130133
mAppWidgetHost = new AppWidgetHost(
131134
context, APPWIDGET_HOST_ID, mOnClickHandler, Looper.myLooper());
132135
cleanupAppWidgetIds();
@@ -338,14 +341,14 @@ void setLockPatternUtils(LockPatternUtils utils) {
338341
@Override
339342
protected void onAttachedToWindow() {
340343
super.onAttachedToWindow();
341-
mAppWidgetHost.startListening();
344+
mAppWidgetHost.startListeningAsUser(mUserId);
342345
KeyguardUpdateMonitor.getInstance(mContext).registerCallback(mUpdateMonitorCallbacks);
343346
}
344347

345348
@Override
346349
protected void onDetachedFromWindow() {
347350
super.onDetachedFromWindow();
348-
mAppWidgetHost.stopListening();
351+
mAppWidgetHost.stopListeningAsUser(mUserId);
349352
KeyguardUpdateMonitor.getInstance(mContext).removeCallback(mUpdateMonitorCallbacks);
350353
}
351354

‎services/java/com/android/server/AppWidgetService.java‎

Lines changed: 37 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -196,9 +196,14 @@ public void setBindAppWidgetPermission(String packageName, boolean permission)
196196
}
197197

198198
@Override
199-
public void bindRemoteViewsService(int appWidgetId, Intent intent, IBinder connection)
200-
throws RemoteException {
201-
getImplForUser(getCallingOrCurrentUserId()).bindRemoteViewsService(
199+
public void bindRemoteViewsService(int appWidgetId, Intent intent, IBinder connection,
200+
int userId) throws RemoteException {
201+
if (Binder.getCallingPid() != android.os.Process.myPid()
202+
&& userId != UserHandle.getCallingUserId()) {
203+
throw new SecurityException("Call from non-system process. Calling uid = "
204+
+ Binder.getCallingUid());
205+
}
206+
getImplForUser(userId).bindRemoteViewsService(
202207
appWidgetId, intent, connection);
203208
}
204209

@@ -209,6 +214,17 @@ public int[] startListening(IAppWidgetHost host, String packageName, int hostId,
209214
packageName, hostId, updatedViews);
210215
}
211216

217+
@Override
218+
public int[] startListeningAsUser(IAppWidgetHost host, String packageName, int hostId,
219+
List<RemoteViews> updatedViews, int userId) throws RemoteException {
220+
if (Binder.getCallingPid() != android.os.Process.myPid()
221+
&& userId != UserHandle.getCallingUserId()) {
222+
throw new SecurityException("Call from non-system process. Calling uid = "
223+
+ Binder.getCallingUid());
224+
}
225+
return getImplForUser(userId).startListening(host, packageName, hostId, updatedViews);
226+
}
227+
212228
public void onUserRemoved(int userId) {
213229
if (userId < 1) return;
214230
synchronized (mAppWidgetServices) {
@@ -306,8 +322,24 @@ public void stopListening(int hostId) throws RemoteException {
306322
}
307323

308324
@Override
309-
public void unbindRemoteViewsService(int appWidgetId, Intent intent) throws RemoteException {
310-
getImplForUser(getCallingOrCurrentUserId()).unbindRemoteViewsService(
325+
public void stopListeningAsUser(int hostId, int userId) throws RemoteException {
326+
if (Binder.getCallingPid() != android.os.Process.myPid()
327+
&& userId != UserHandle.getCallingUserId()) {
328+
throw new SecurityException("Call from non-system process. Calling uid = "
329+
+ Binder.getCallingUid());
330+
}
331+
getImplForUser(userId).stopListening(hostId);
332+
}
333+
334+
@Override
335+
public void unbindRemoteViewsService(int appWidgetId, Intent intent, int userId)
336+
throws RemoteException {
337+
if (Binder.getCallingPid() != android.os.Process.myPid()
338+
&& userId != UserHandle.getCallingUserId()) {
339+
throw new SecurityException("Call from non-system process. Calling uid = "
340+
+ Binder.getCallingUid());
341+
}
342+
getImplForUser(userId).unbindRemoteViewsService(
311343
appWidgetId, intent);
312344
}
313345

‎services/java/com/android/server/AppWidgetServiceImpl.java‎

Lines changed: 14 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,15 @@ static class Host {
116116
boolean zombie; // if we're in safe mode, don't prune this just because nobody references it
117117

118118
int tag; // for use while saving state (the index)
119+
120+
boolean uidMatches(int callingUid) {
121+
if (UserHandle.getAppId(callingUid) == Process.myUid()) {
122+
// For a host that's in the system process, ignore the user id
123+
return UserHandle.isSameApp(this.uid, callingUid);
124+
} else {
125+
return this.uid == callingUid;
126+
}
127+
}
119128
}
120129

121130
static class AppWidgetId {
@@ -476,7 +485,7 @@ public void deleteAllHosts() {
476485
boolean changed = false;
477486
for (int i = N - 1; i >= 0; i--) {
478487
Host host = mHosts.get(i);
479-
if (host.uid == callingUid) {
488+
if (host.uidMatches(callingUid)) {
480489
deleteHostLocked(host);
481490
changed = true;
482491
}
@@ -744,8 +753,6 @@ public void unbindRemoteViewsService(int appWidgetId, Intent intent) {
744753
conn.disconnect();
745754
mContext.unbindService(conn);
746755
mBoundRemoteViewsServices.remove(key);
747-
} else {
748-
Log.e("AppWidgetService", "Error (unbindRemoteViewsService): Connection not bound");
749756
}
750757
}
751758
}
@@ -968,32 +975,15 @@ public void partiallyUpdateAppWidgetIds(int[] appWidgetIds, RemoteViews views) {
968975
for (int i = 0; i < N; i++) {
969976
AppWidgetId id = lookupAppWidgetIdLocked(appWidgetIds[i]);
970977
if (id == null) {
971-
String message = "AppWidgetId NPE: mUserId=" + mUserId
972-
+ ", callingUid=" + Binder.getCallingUid()
973-
+ ", appWidgetIds[i]=" + appWidgetIds[i]
974-
+ "\n mAppWidgets:\n" + getUserWidgets();
975-
throw new NullPointerException(message);
976-
}
977-
if (id.views != null) {
978+
Slog.w(TAG, "widget id " + appWidgetIds[i] + " not found!");
979+
} else if (id.views != null) {
978980
// Only trigger a partial update for a widget if it has received a full update
979981
updateAppWidgetInstanceLocked(id, views, true);
980982
}
981983
}
982984
}
983985
}
984986

985-
private String getUserWidgets() {
986-
StringBuffer sb = new StringBuffer();
987-
for (AppWidgetId widget: mAppWidgetIds) {
988-
sb.append(" id="); sb.append(widget.appWidgetId);
989-
sb.append(", hostUid="); sb.append(widget.host.uid);
990-
sb.append(", provider="); sb.append(widget.provider.info.provider.toString());
991-
sb.append("\n");
992-
}
993-
sb.append("\n");
994-
return sb.toString();
995-
}
996-
997987
public void notifyAppWidgetViewDataChanged(int[] appWidgetIds, int viewId) {
998988
if (appWidgetIds == null) {
999989
return;
@@ -1186,7 +1176,7 @@ public void stopListening(int hostId) {
11861176
}
11871177

11881178
boolean canAccessAppWidgetId(AppWidgetId id, int callingUid) {
1189-
if (id.host.uid == callingUid) {
1179+
if (id.host.uidMatches(callingUid)) {
11901180
// Apps hosting the AppWidget have access to it.
11911181
return true;
11921182
}
@@ -1229,7 +1219,7 @@ Host lookupHostLocked(int uid, int hostId) {
12291219
final int N = mHosts.size();
12301220
for (int i = 0; i < N; i++) {
12311221
Host h = mHosts.get(i);
1232-
if (h.uid == uid && h.hostId == hostId) {
1222+
if (h.uidMatches(uid) && h.hostId == hostId) {
12331223
return h;
12341224
}
12351225
}

0 commit comments

Comments
 (0)