Improve LocationProviderProxy package guessing

Take current connected service package into account when guessing
possible package names.

Test: presubmits
Change-Id: I2abd1a32beed7b1af9c39b01f650a657d856bd2c
This commit is contained in:
Soonil Nagarkar
2020-07-24 11:51:40 -07:00
parent e76bbe2da2
commit 7e1afe23da
4 changed files with 52 additions and 34 deletions

View File

@@ -80,10 +80,16 @@ public class ServiceWatcher implements ServiceConnection {
default void onError() {}
}
/** Function to run on binder interface when first bound. */
public interface OnBindRunner {
/** Called to run client code with the binder. */
void run(IBinder binder, ComponentName service) throws RemoteException;
}
/**
* Information on the service ServiceWatcher has selected as the best option for binding.
*/
public static final class ServiceInfo implements Comparable<ServiceInfo> {
private static final class ServiceInfo implements Comparable<ServiceInfo> {
public static final ServiceInfo NONE = new ServiceInfo(Integer.MIN_VALUE, null,
UserHandle.USER_NULL, false);
@@ -179,25 +185,25 @@ public class ServiceWatcher implements ServiceConnection {
private final Handler mHandler;
private final Intent mIntent;
@Nullable private final BinderRunner mOnBind;
@Nullable private final OnBindRunner mOnBind;
@Nullable private final Runnable mOnUnbind;
// read/write from handler thread only
private int mCurrentUserId;
// write from handler thread only, read anywhere
private volatile ServiceInfo mServiceInfo;
private volatile ServiceInfo mTargetService;
private volatile IBinder mBinder;
public ServiceWatcher(Context context, String action,
@Nullable BinderRunner onBind, @Nullable Runnable onUnbind,
@Nullable OnBindRunner onBind, @Nullable Runnable onUnbind,
@BoolRes int enableOverlayResId, @StringRes int nonOverlayPackageResId) {
this(context, FgThread.getHandler(), action, onBind, onUnbind, enableOverlayResId,
nonOverlayPackageResId);
}
public ServiceWatcher(Context context, Handler handler, String action,
@Nullable BinderRunner onBind, @Nullable Runnable onUnbind,
@Nullable OnBindRunner onBind, @Nullable Runnable onUnbind,
@BoolRes int enableOverlayResId, @StringRes int nonOverlayPackageResId) {
mContext = context;
mHandler = handler;
@@ -214,7 +220,7 @@ public class ServiceWatcher implements ServiceConnection {
mCurrentUserId = UserHandle.USER_NULL;
mServiceInfo = ServiceInfo.NONE;
mTargetService = ServiceInfo.NONE;
mBinder = null;
}
@@ -304,7 +310,7 @@ public class ServiceWatcher implements ServiceConnection {
}
}
if (forceRebind || !bestServiceInfo.equals(mServiceInfo)) {
if (forceRebind || !bestServiceInfo.equals(mTargetService)) {
rebind(bestServiceInfo);
}
}
@@ -312,32 +318,32 @@ public class ServiceWatcher implements ServiceConnection {
private void rebind(ServiceInfo newServiceInfo) {
Preconditions.checkState(Looper.myLooper() == mHandler.getLooper());
if (!mServiceInfo.equals(ServiceInfo.NONE)) {
if (!mTargetService.equals(ServiceInfo.NONE)) {
if (D) {
Log.i(TAG, "[" + mIntent.getAction() + "] unbinding from " + mServiceInfo);
Log.i(TAG, "[" + mIntent.getAction() + "] unbinding from " + mTargetService);
}
mContext.unbindService(this);
onServiceDisconnected(mServiceInfo.component);
mServiceInfo = ServiceInfo.NONE;
onServiceDisconnected(mTargetService.component);
mTargetService = ServiceInfo.NONE;
}
mServiceInfo = newServiceInfo;
if (mServiceInfo.equals(ServiceInfo.NONE)) {
mTargetService = newServiceInfo;
if (mTargetService.equals(ServiceInfo.NONE)) {
return;
}
Preconditions.checkState(mServiceInfo.component != null);
Preconditions.checkState(mTargetService.component != null);
if (D) {
Log.i(TAG, getLogPrefix() + " binding to " + mServiceInfo);
Log.i(TAG, getLogPrefix() + " binding to " + mTargetService);
}
Intent bindIntent = new Intent(mIntent).setComponent(mServiceInfo.component);
Intent bindIntent = new Intent(mIntent).setComponent(mTargetService.component);
if (!mContext.bindServiceAsUser(bindIntent, this,
BIND_AUTO_CREATE | BIND_NOT_FOREGROUND | BIND_NOT_VISIBLE,
mHandler, UserHandle.of(mServiceInfo.userId))) {
mServiceInfo = ServiceInfo.NONE;
mHandler, UserHandle.of(mTargetService.userId))) {
mTargetService = ServiceInfo.NONE;
Log.e(TAG, getLogPrefix() + " unexpected bind failure - retrying later");
mHandler.postDelayed(() -> onBestServiceChanged(false), RETRY_DELAY_MS);
}
@@ -355,11 +361,11 @@ public class ServiceWatcher implements ServiceConnection {
mBinder = binder;
if (mOnBind != null) {
try {
mOnBind.run(binder);
mOnBind.run(binder, component);
} catch (RuntimeException | RemoteException e) {
// binders may propagate some specific non-RemoteExceptions from the other side
// through the binder as well - we cannot allow those to crash the system server
Log.e(TAG, getLogPrefix() + " exception running on " + mServiceInfo, e);
Log.e(TAG, getLogPrefix() + " exception running on " + component, e);
}
}
}
@@ -406,7 +412,7 @@ public class ServiceWatcher implements ServiceConnection {
void onPackageChanged(String packageName) {
// force a rebind if the changed package was the currently connected package
onBestServiceChanged(packageName.equals(mServiceInfo.getPackageName()));
onBestServiceChanged(packageName.equals(mTargetService.getPackageName()));
}
/**
@@ -425,7 +431,7 @@ public class ServiceWatcher implements ServiceConnection {
} catch (RuntimeException | RemoteException e) {
// binders may propagate some specific non-RemoteExceptions from the other side
// through the binder as well - we cannot allow those to crash the system server
Log.e(TAG, getLogPrefix() + " exception running on " + mServiceInfo, e);
Log.e(TAG, getLogPrefix() + " exception running on " + mTargetService, e);
runner.onError();
}
});
@@ -437,14 +443,14 @@ public class ServiceWatcher implements ServiceConnection {
@Override
public String toString() {
return mServiceInfo.toString();
return mTargetService.toString();
}
/**
* Dump for debugging.
*/
public void dump(FileDescriptor fd, PrintWriter pw, String[] args) {
pw.println("service=" + mServiceInfo);
pw.println("target service=" + mTargetService);
pw.println("connected=" + (mBinder != null));
}
}

View File

@@ -17,6 +17,7 @@
package com.android.server.location;
import android.annotation.Nullable;
import android.content.ComponentName;
import android.content.Context;
import android.hardware.location.ActivityRecognitionHardware;
import android.hardware.location.IActivityRecognitionHardwareClient;
@@ -77,7 +78,7 @@ public class HardwareActivityRecognitionProxy {
return mServiceWatcher.register();
}
private void onBind(IBinder binder) throws RemoteException {
private void onBind(IBinder binder, ComponentName service) throws RemoteException {
String descriptor = binder.getInterfaceDescriptor();
if (IActivityRecognitionHardwareWatcher.class.getCanonicalName().equals(descriptor)) {

View File

@@ -19,6 +19,7 @@ package com.android.server.location;
import static com.android.internal.util.ConcurrentUtils.DIRECT_EXECUTOR;
import android.annotation.Nullable;
import android.content.ComponentName;
import android.content.Context;
import android.location.Location;
import android.location.util.identity.CallerIdentity;
@@ -32,10 +33,12 @@ import com.android.internal.location.ILocationProvider;
import com.android.internal.location.ILocationProviderManager;
import com.android.internal.location.ProviderProperties;
import com.android.internal.location.ProviderRequest;
import com.android.internal.util.ArrayUtils;
import com.android.server.ServiceWatcher;
import java.io.FileDescriptor;
import java.io.PrintWriter;
import java.util.Objects;
/**
* Proxy for ILocationProvider implementations.
@@ -65,6 +68,8 @@ public class LocationProviderProxy extends AbstractLocationProvider {
@GuardedBy("mLock")
Proxy mProxy;
@GuardedBy("mLock")
@Nullable ComponentName mService;
private volatile ProviderRequest mRequest;
@@ -86,11 +91,12 @@ public class LocationProviderProxy extends AbstractLocationProvider {
return mServiceWatcher.register();
}
private void onBind(IBinder binder) throws RemoteException {
private void onBind(IBinder binder, ComponentName service) throws RemoteException {
ILocationProvider provider = ILocationProvider.Stub.asInterface(binder);
synchronized (mLock) {
mProxy = new Proxy();
mService = service;
provider.setLocationProviderManager(mProxy);
ProviderRequest request = mRequest;
@@ -103,6 +109,7 @@ public class LocationProviderProxy extends AbstractLocationProvider {
private void onUnbind() {
synchronized (mLock) {
mProxy = null;
mService = null;
setState(State.EMPTY_STATE);
}
}
@@ -111,16 +118,16 @@ public class LocationProviderProxy extends AbstractLocationProvider {
public void onSetRequest(ProviderRequest request) {
mRequest = request;
mServiceWatcher.runOnBinder(binder -> {
ILocationProvider service = ILocationProvider.Stub.asInterface(binder);
service.setRequest(request, request.workSource);
ILocationProvider provider = ILocationProvider.Stub.asInterface(binder);
provider.setRequest(request, request.workSource);
});
}
@Override
public void onExtraCommand(int uid, int pid, String command, Bundle extras) {
mServiceWatcher.runOnBinder(binder -> {
ILocationProvider service = ILocationProvider.Stub.asInterface(binder);
service.sendExtraCommand(command, extras);
ILocationProvider provider = ILocationProvider.Stub.asInterface(binder);
provider.sendExtraCommand(command, extras);
});
}
@@ -129,12 +136,14 @@ public class LocationProviderProxy extends AbstractLocationProvider {
mServiceWatcher.dump(fd, pw, args);
}
private static String guessPackageName(Context context, int uid) {
private static String guessPackageName(Context context, int uid, String packageName) {
String[] packageNames = context.getPackageManager().getPackagesForUid(uid);
if (packageNames == null || packageNames.length == 0) {
// illegal state exception will propagate back through binders
throw new IllegalStateException(
"location provider from uid " + uid + " has no package information");
} else if (ArrayUtils.contains(packageNames, packageName)) {
return packageName;
} else {
return packageNames[0];
}
@@ -154,7 +163,8 @@ public class LocationProviderProxy extends AbstractLocationProvider {
CallerIdentity identity;
if (packageName == null) {
packageName = guessPackageName(mContext, Binder.getCallingUid());
packageName = guessPackageName(mContext, Binder.getCallingUid(),
Objects.requireNonNull(mService).getPackageName());
// unsafe is ok since the package is coming direct from the package manager here
identity = CallerIdentity.fromBinderUnsafe(packageName, attributionTag);
} else {
@@ -175,7 +185,8 @@ public class LocationProviderProxy extends AbstractLocationProvider {
// if no identity is set yet, set it now
if (getIdentity() == null) {
String packageName = guessPackageName(mContext, Binder.getCallingUid());
String packageName = guessPackageName(mContext, Binder.getCallingUid(),
Objects.requireNonNull(mService).getPackageName());
// unsafe is ok since the package is coming direct from the package manager here
setIdentity(CallerIdentity.fromBinderUnsafe(packageName, null));
}

View File

@@ -63,7 +63,7 @@ public final class GeofenceProxy {
private GeofenceProxy(Context context, IGpsGeofenceHardware gpsGeofence) {
mGpsGeofenceHardware = Objects.requireNonNull(gpsGeofence);
mServiceWatcher = new ServiceWatcher(context, SERVICE_ACTION,
this::updateGeofenceHardware, null,
(binder, service) -> updateGeofenceHardware(binder), null,
com.android.internal.R.bool.config_enableGeofenceOverlay,
com.android.internal.R.string.config_geofenceProviderPackageName);