From 465088ed2f4591d08738b2306f213c5149b3484b Mon Sep 17 00:00:00 2001
From: junyulai <junyulai@google.com>
Date: Fri, 30 Aug 2019 16:44:45 +0800
Subject: Fix LockdownVpnTracker deadlock when resetting legacy Always-On VPN

When resetting legacy Always-On VPN, the intent is handled by
main thread. And it will also initialize VPN shutdown process,
which will cause networkInfo changed event on
ConnectivityService internal thread. These two events need to
hold their corresponding lock and ask for the other one, which
causes the deadlock.

This patch move the event handling to the same thread to prevent
such deadlock, and cleanup some unused variables.

Change-Id: I5b656c0d0381acb4e33409a11f502db9b180296c
Fix: 139122208
Test: atest FrameworksNetTests, manual test
---
 .../com/android/server/ConnectivityService.java    |  2 +-
 .../com/android/server/net/LockdownVpnTracker.java | 34 ++++++++++++----------
 2 files changed, 20 insertions(+), 16 deletions(-)

diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java
index 62db5578337c..d822879e253e 100644
--- a/services/core/java/com/android/server/ConnectivityService.java
+++ b/services/core/java/com/android/server/ConnectivityService.java
@@ -4550,7 +4550,7 @@ public class ConnectivityService extends IConnectivityManager.Stub
                     Slog.w(TAG, "VPN for user " + user + " not ready yet. Skipping lockdown");
                     return false;
                 }
-                setLockdownTracker(new LockdownVpnTracker(mContext, mNMS, this, vpn, profile));
+                setLockdownTracker(new LockdownVpnTracker(mContext, this, mHandler, vpn, profile));
             } else {
                 setLockdownTracker(null);
             }
diff --git a/services/core/java/com/android/server/net/LockdownVpnTracker.java b/services/core/java/com/android/server/net/LockdownVpnTracker.java
index 67a018a90f94..77fbe41ebb88 100644
--- a/services/core/java/com/android/server/net/LockdownVpnTracker.java
+++ b/services/core/java/com/android/server/net/LockdownVpnTracker.java
@@ -19,6 +19,8 @@ package com.android.server.net;
 import static android.Manifest.permission.CONNECTIVITY_INTERNAL;
 import static android.provider.Settings.ACTION_VPN_SETTINGS;
 
+import android.annotation.NonNull;
+import android.annotation.Nullable;
 import android.app.Notification;
 import android.app.NotificationManager;
 import android.app.PendingIntent;
@@ -32,7 +34,7 @@ import android.net.LinkProperties;
 import android.net.NetworkInfo;
 import android.net.NetworkInfo.DetailedState;
 import android.net.NetworkInfo.State;
-import android.os.INetworkManagementService;
+import android.os.Handler;
 import android.security.Credentials;
 import android.security.KeyStore;
 import android.text.TextUtils;
@@ -63,19 +65,18 @@ public class LockdownVpnTracker {
 
     private static final String ACTION_LOCKDOWN_RESET = "com.android.server.action.LOCKDOWN_RESET";
 
-    private static final int ROOT_UID = 0;
+    @NonNull private final Context mContext;
+    @NonNull private final ConnectivityService mConnService;
+    @NonNull private final Handler mHandler;
+    @NonNull private final Vpn mVpn;
+    @NonNull private final VpnProfile mProfile;
 
-    private final Context mContext;
-    private final INetworkManagementService mNetService;
-    private final ConnectivityService mConnService;
-    private final Vpn mVpn;
-    private final VpnProfile mProfile;
+    @NonNull private final Object mStateLock = new Object();
 
-    private final Object mStateLock = new Object();
-
-    private final PendingIntent mConfigIntent;
-    private final PendingIntent mResetIntent;
+    @NonNull private final PendingIntent mConfigIntent;
+    @NonNull private final PendingIntent mResetIntent;
 
+    @Nullable
     private String mAcceptedEgressIface;
 
     private int mErrorCount;
@@ -84,11 +85,14 @@ public class LockdownVpnTracker {
         return KeyStore.getInstance().contains(Credentials.LOCKDOWN_VPN);
     }
 
-    public LockdownVpnTracker(Context context, INetworkManagementService netService,
-            ConnectivityService connService, Vpn vpn, VpnProfile profile) {
+    public LockdownVpnTracker(@NonNull Context context,
+            @NonNull ConnectivityService connService,
+            @NonNull Handler handler,
+            @NonNull Vpn vpn,
+            @NonNull VpnProfile profile) {
         mContext = Preconditions.checkNotNull(context);
-        mNetService = Preconditions.checkNotNull(netService);
         mConnService = Preconditions.checkNotNull(connService);
+        mHandler = Preconditions.checkNotNull(handler);
         mVpn = Preconditions.checkNotNull(vpn);
         mProfile = Preconditions.checkNotNull(profile);
 
@@ -198,7 +202,7 @@ public class LockdownVpnTracker {
         mVpn.setLockdown(true);
 
         final IntentFilter resetFilter = new IntentFilter(ACTION_LOCKDOWN_RESET);
-        mContext.registerReceiver(mResetReceiver, resetFilter, CONNECTIVITY_INTERNAL, null);
+        mContext.registerReceiver(mResetReceiver, resetFilter, CONNECTIVITY_INTERNAL, mHandler);
 
         handleStateChangedLocked();
     }
-- 
cgit v1.2.3-59-g8ed1b