Android/frameworks_base
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Mitigate race conditions in PacService

Browse Source 
There are some design limitations in PacService (one-way aidl calls
from ConnectivityService) that causes it to be racy when PAC proxy are
set and cleared in quick succession. Attempt to mitigate them with the
following changes:

1. Make PacNative a singleton instead of one instance per binder. The
underlying v8 engine is singleton so it makes little sense to have
multiple instances of the PacNative wrapper.
2. Remove the startPacSystem and stopPacSystem API and bind the
PacNative lifecycle to the PacService. Otherwise the one-way
stopPacSystem() binder call could have raced with a next
startPacSystem() call when PAC proxy is cleared and then set.

For this change, startPacSystem() and stopPacSystem() and made no-op
only. They will be fully removed in the next change.

Test: atest --iterations 200 com.android.cts.devicepolicy.DeviceOwnerTest#testProxyPacProxyTest
Bug: 147359729
Change-Id: Ie3ce098167694421f8bd2a6dec85d7c437cfb0be
EDIT
This commit is contained in:
Rubin Xu
2020-02-07 12:02:00 +00:00
parent 05cf3e896c
commit b770235a86
2 changed files with 14 additions and 32 deletions
Download Patch File Download Diff File Expand all files Collapse all files

8
packages/services/PacProcessor/src/com/android/pacprocessor/PacNative.java
View File

@@ -23,6 +23,8 @@ import android.util.Log;
public class PacNative {
private static final String TAG = "PacProxy";
private static final PacNative sInstance = new PacNative();
private String mCurrentPac;
private boolean mIsActive;
@@ -39,10 +41,14 @@ public class PacNative {
System.loadLibrary("jni_pacprocessor");
}
PacNative() {
private PacNative() {
}
public static PacNative getInstance() {
return sInstance;
}
public synchronized boolean startPacSupport() {
if (createV8ParserNativeLocked()) {
Log.e(TAG, "Unable to Create v8 Proxy Parser.");

38
packages/services/PacProcessor/src/com/android/pacprocessor/PacService.java
View File

@@ -31,43 +31,27 @@ import java.net.URL;
public class PacService extends Service {
private static final String TAG = "PacService";
private PacNative mPacNative;
private ProxyServiceStub mStub;
private PacNative mPacNative = PacNative.getInstance();
private ProxyServiceStub mStub = new ProxyServiceStub();
@Override
public void onCreate() {
super.onCreate();
if (mPacNative == null) {
mPacNative = new PacNative();
mStub = new ProxyServiceStub(mPacNative);
}
mPacNative.startPacSupport();
}
@Override
public void onDestroy() {
mPacNative.stopPacSupport();
super.onDestroy();
if (mPacNative != null) {
mPacNative.stopPacSupport();
mPacNative = null;
mStub = null;
}
}
@Override
public IBinder onBind(Intent intent) {
if (mPacNative == null) {
mPacNative = new PacNative();
mStub = new ProxyServiceStub(mPacNative);
}
return mStub;
}
private static class ProxyServiceStub extends IProxyService.Stub {
private final PacNative mPacNative;
public ProxyServiceStub(PacNative pacNative) {
mPacNative = pacNative;
}
private class ProxyServiceStub extends IProxyService.Stub {
@Override
public String resolvePacFile(String host, String url) throws RemoteException {
@@ -102,20 +86,12 @@ public class PacService extends Service {
@Override
public void startPacSystem() throws RemoteException {
if (Binder.getCallingUid() != Process.SYSTEM_UID) {
Log.e(TAG, "Only system user is allowed to call startPacSystem");
throw new SecurityException();
}
mPacNative.startPacSupport();
//TODO: remove
}
@Override
public void stopPacSystem() throws RemoteException {
if (Binder.getCallingUid() != Process.SYSTEM_UID) {
Log.e(TAG, "Only system user is allowed to call stopPacSystem");
throw new SecurityException();
}
mPacNative.stopPacSupport();
//TODO: remove
}
}
}