Fix unprotected variable access by serializing.

Craig Mautner · Craig Mautner · commit 5642a48fbba8 · 2012-08-23T14:25:34.000-07:00
The variables mKeyguardDisabled and mAllowDisableKeyguard were
being modified unprotected by mKeyguardTokenWatcher. Fix is to
serialize accesses to these variables by only referencing them
from the same Handler that mKeyguardTokenWatcher uses. Eliminates
synchronization blocks and mKeyguardDisabled variable.

Fixes bug 7045624.

Change-Id: I6355aa393507408296316bee61e178dc81e2a172
diff --git a/services/java/com/android/server/wm/KeyguardDisableHandler.java b/services/java/com/android/server/wm/KeyguardDisableHandler.java
@@ -0,0 +1,107 @@
+/*
+ * Copyright (C) 2012 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.android.server.wm;
+
+import android.app.admin.DevicePolicyManager;
+import android.content.Context;
+import android.os.Handler;
+import android.os.IBinder;
+import android.os.Message;
+import android.os.TokenWatcher;
+import android.util.Log;
+import android.util.Pair;
+import android.view.WindowManagerPolicy;
+
+public class KeyguardDisableHandler extends Handler {
+    private static final String TAG = "KeyguardDisableHandler";
+
+    private static final int ALLOW_DISABLE_YES = 1;
+    private static final int ALLOW_DISABLE_NO = 0;
+    private static final int ALLOW_DISABLE_UNKNOWN = -1; // check with DevicePolicyManager
+    private int mAllowDisableKeyguard = ALLOW_DISABLE_UNKNOWN; // sync'd by mKeyguardTokenWatcher
+
+    // Message.what constants
+    static final int KEYGUARD_DISABLE = 1;
+    static final int KEYGUARD_REENABLE = 2;
+    static final int KEYGUARD_POLICY_CHANGED = 3;
+
+    final Context mContext;
+    final WindowManagerPolicy mPolicy;
+    KeyguardTokenWatcher mKeyguardTokenWatcher;
+
+    public KeyguardDisableHandler(final Context context, final WindowManagerPolicy policy) {
+        mContext = context;
+        mPolicy = policy;
+    }
+
+    @SuppressWarnings("unchecked")
+    @Override
+    public void handleMessage(Message msg) {
+        if (mKeyguardTokenWatcher == null) {
+            mKeyguardTokenWatcher = new KeyguardTokenWatcher(this);
+        }
+
+        switch (msg.what) {
+            case KEYGUARD_DISABLE:
+                final Pair<IBinder, String> pair = (Pair<IBinder, String>)msg.obj;
+                mKeyguardTokenWatcher.acquire(pair.first, pair.second);
+                break;
+
+            case KEYGUARD_REENABLE:
+                mKeyguardTokenWatcher.release((IBinder)msg.obj);
+                break;
+
+            case KEYGUARD_POLICY_CHANGED:
+                mPolicy.enableKeyguard(true);
+                // lazily evaluate this next time we're asked to disable keyguard
+                mAllowDisableKeyguard = ALLOW_DISABLE_UNKNOWN;
+                break;
+        }
+    }
+
+    class KeyguardTokenWatcher extends TokenWatcher {
+
+        public KeyguardTokenWatcher(final Handler handler) {
+            super(handler, TAG);
+        }
+
+        @Override
+        public void acquired() {
+            // We fail safe and prevent disabling keyguard in the unlikely event this gets
+            // called before DevicePolicyManagerService has started.
+            if (mAllowDisableKeyguard == ALLOW_DISABLE_UNKNOWN) {
+                DevicePolicyManager dpm = (DevicePolicyManager) mContext.getSystemService(
+                        Context.DEVICE_POLICY_SERVICE);
+                if (dpm != null) {
+                    mAllowDisableKeyguard = dpm.getPasswordQuality(null)
+                            == DevicePolicyManager.PASSWORD_QUALITY_UNSPECIFIED ?
+                                    ALLOW_DISABLE_YES : ALLOW_DISABLE_NO;
+                }
+            }
+            if (mAllowDisableKeyguard == ALLOW_DISABLE_YES) {
+                mPolicy.enableKeyguard(false);
+            } else {
+                Log.v(TAG, "Not disabling keyguard since device policy is enforced");
+            }
+        }
+
+        @Override
+        public void released() {
+            mPolicy.enableKeyguard(true);
+        }
+    }
+}
diff --git a/services/java/com/android/server/wm/WindowManagerService.java b/services/java/com/android/server/wm/WindowManagerService.java
@@ -75,7 +75,6 @@
 import android.graphics.RectF;
 import android.graphics.Region;
 import android.hardware.display.DisplayManager;
-import android.os.BatteryStats;
 import android.os.Binder;
 import android.os.Bundle;
 import android.os.Debug;
@@ -93,7 +92,6 @@
 import android.os.StrictMode;
 import android.os.SystemClock;
 import android.os.SystemProperties;
-import android.os.TokenWatcher;
 import android.os.Trace;
 import android.os.WorkSource;
 import android.provider.Settings;
@@ -279,55 +277,19 @@ public class WindowManagerService extends IWindowManager.Stub
     private static final String SYSTEM_SECURE = "ro.secure";
     private static final String SYSTEM_DEBUGGABLE = "ro.debuggable";
 
-    /**
-     * Condition waited on by {@link #reenableKeyguard} to know the call to
-     * the window policy has finished.
-     * This is set to true only if mKeyguardTokenWatcher.acquired() has
-     * actually disabled the keyguard.
-     */
-    private boolean mKeyguardDisabled = false;
+    final private KeyguardDisableHandler mKeyguardDisableHandler;
 
     private final boolean mHeadless;
 
-    private static final int ALLOW_DISABLE_YES = 1;
-    private static final int ALLOW_DISABLE_NO = 0;
-    private static final int ALLOW_DISABLE_UNKNOWN = -1; // check with DevicePolicyManager
-    private int mAllowDisableKeyguard = ALLOW_DISABLE_UNKNOWN; // sync'd by mKeyguardTokenWatcher
-
     private static final float THUMBNAIL_ANIMATION_DECELERATE_FACTOR = 1.5f;
 
-    final TokenWatcher mKeyguardTokenWatcher = new TokenWatcher(
-            new Handler(), "WindowManagerService.mKeyguardTokenWatcher") {
-        @Override
-        public void acquired() {
-            if (shouldAllowDisableKeyguard()) {
-                mPolicy.enableKeyguard(false);
-                mKeyguardDisabled = true;
-            } else {
-                Log.v(TAG, "Not disabling keyguard since device policy is enforced");
-            }
-        }
-        @Override
-        public void released() {
-            mPolicy.enableKeyguard(true);
-            synchronized (mKeyguardTokenWatcher) {
-                mKeyguardDisabled = false;
-                mKeyguardTokenWatcher.notifyAll();
-            }
-        }
-    };
-
     final BroadcastReceiver mBroadcastReceiver = new BroadcastReceiver() {
         @Override
         public void onReceive(Context context, Intent intent) {
             final String action = intent.getAction();
             if (DevicePolicyManager.ACTION_DEVICE_POLICY_MANAGER_STATE_CHANGED.equals(action)) {
-                mPolicy.enableKeyguard(true);
-                synchronized(mKeyguardTokenWatcher) {
-                    // lazily evaluate this next time we're asked to disable keyguard
-                    mAllowDisableKeyguard = ALLOW_DISABLE_UNKNOWN;
-                    mKeyguardDisabled = false;
-                }
+                mKeyguardDisableHandler.sendEmptyMessage(
+                    KeyguardDisableHandler.KEYGUARD_POLICY_CHANGED);
             } else if (Intent.ACTION_USER_SWITCHED.equals(action)) {
                 final int newUserId = intent.getIntExtra(Intent.EXTRA_USER_HANDLE, 0);
                 Slog.v(TAG, "Switching user from " + mCurrentUserId + " to " + newUserId);
@@ -898,6 +860,8 @@ private WindowManagerService(Context context, PowerManagerService pm,
         mDisplayManager = DisplayManager.getInstance();
         mHeadless = displayManager.isHeadless();
 
+        mKeyguardDisableHandler = new KeyguardDisableHandler(mContext, mPolicy);
+
         mPowerManager = pm;
         mPowerManager.setPolicy(mPolicy);
         PowerManager pmc = (PowerManager)context.getSystemService(Context.POWER_SERVICE);
@@ -5062,59 +5026,26 @@ public void moveAppTokensToBottom(List<IBinder> tokens) {
     // Misc IWindowSession methods
     // -------------------------------------------------------------
 
-    private boolean shouldAllowDisableKeyguard()
-    {
-        // We fail safe and prevent disabling keyguard in the unlikely event this gets
-        // called before DevicePolicyManagerService has started.
-        if (mAllowDisableKeyguard == ALLOW_DISABLE_UNKNOWN) {
-            DevicePolicyManager dpm = (DevicePolicyManager) mContext.getSystemService(
-                    Context.DEVICE_POLICY_SERVICE);
-            if (dpm != null) {
-                mAllowDisableKeyguard = dpm.getPasswordQuality(null)
-                        == DevicePolicyManager.PASSWORD_QUALITY_UNSPECIFIED ?
-                                ALLOW_DISABLE_YES : ALLOW_DISABLE_NO;
-            }
-        }
-        return mAllowDisableKeyguard == ALLOW_DISABLE_YES;
-    }
-
+    @Override
     public void disableKeyguard(IBinder token, String tag) {
         if (mContext.checkCallingOrSelfPermission(android.Manifest.permission.DISABLE_KEYGUARD)
             != PackageManager.PERMISSION_GRANTED) {
             throw new SecurityException("Requires DISABLE_KEYGUARD permission");
         }
 
-        synchronized (mKeyguardTokenWatcher) {
-            mKeyguardTokenWatcher.acquire(token, tag);
-        }
+        mKeyguardDisableHandler.sendMessage(mKeyguardDisableHandler.obtainMessage(
+                KeyguardDisableHandler.KEYGUARD_DISABLE, new Pair<IBinder, String>(token, tag)));
     }
 
+    @Override
     public void reenableKeyguard(IBinder token) {
         if (mContext.checkCallingOrSelfPermission(android.Manifest.permission.DISABLE_KEYGUARD)
             != PackageManager.PERMISSION_GRANTED) {
             throw new SecurityException("Requires DISABLE_KEYGUARD permission");
         }
 
-        synchronized (mKeyguardTokenWatcher) {
-            mKeyguardTokenWatcher.release(token);
-
-            if (!mKeyguardTokenWatcher.isAcquired()) {
-                // If we are the last one to reenable the keyguard wait until
-                // we have actually finished reenabling until returning.
-                // It is possible that reenableKeyguard() can be called before
-                // the previous disableKeyguard() is handled, in which case
-                // neither mKeyguardTokenWatcher.acquired() or released() would
-                // be called. In that case mKeyguardDisabled will be false here
-                // and we have nothing to wait for.
-                while (mKeyguardDisabled) {
-                    try {
-                        mKeyguardTokenWatcher.wait();
-                    } catch (InterruptedException e) {
-                        Thread.currentThread().interrupt();
-                    }
-                }
-            }
-        }
+        mKeyguardDisableHandler.sendMessage(mKeyguardDisableHandler.obtainMessage(
+                KeyguardDisableHandler.KEYGUARD_REENABLE, token));
     }
 
     /**
@@ -10416,7 +10347,6 @@ public void dump(FileDescriptor fd, PrintWriter pw, String[] args) {
     // Called by the heartbeat to ensure locks are not held indefnitely (for deadlock detection).
     public void monitor() {
         synchronized (mWindowMap) { }
-        synchronized (mKeyguardTokenWatcher) { }
     }
 
     public interface OnHardKeyboardStatusChangeListener {
