Correctly manage the lifecycle of IME InputChannels.

InputChannels are normally duplicated when sent to a remote process
over Binder but this does not happen if the recipient is running within
the system server process.  This causes problems for KeyGuard because the
InputMethodManagerService may accidentally dispose the channel
that KeyGuard is using.

Fixed the lifecycle of InputChannels that are managed by the IME
framework.  We now return a duplicate of the channel to the application
and then take care to dispose of the duplicate when necessary.
In particular, InputBindResult disposes its InputChannel automatically
when returned through Binder (using PARCELABLE_WRITE_RETURN_VALUE).

Bug: 8493879
Change-Id: I08ec3d13268c76f3b56706b4523508bcefa3be79

1 files changed, 18 insertions, 7 deletions

diff --git a/services/java/com/android/server/InputMethodManagerService.java b/services/java/com/android/server/InputMethodManagerService.java
index 3b541ec..a28c387 100644
--- a/services/java/com/android/server/InputMethodManagerService.java
+++ b/services/java/com/android/server/InputMethodManagerService.java
@@ -1070,7 +1070,8 @@ public class InputMethodManagerService extends IInputMethodManager.Stub
             if (DEBUG) Slog.v(TAG, "Attach new input asks to show input");
             showCurrentInputLocked(getAppShowFlags(), null);
         }
-        return new InputBindResult(session.session, session.channel, mCurId, mCurSeq);
+        return new InputBindResult(session.session,
+                session.channel != null ? session.channel.dup() : null, mCurId, mCurSeq);
     }
 
     InputBindResult startInputLocked(IInputMethodClient client,
@@ -2357,13 +2358,15 @@ public class InputMethodManagerService extends IInputMethodManager.Stub
                 return true;
             case MSG_CREATE_SESSION: {
                 args = (SomeArgs)msg.obj;
+                IInputMethod method = (IInputMethod)args.arg1;
                 InputChannel channel = (InputChannel)args.arg2;
                 try {
-                    ((IInputMethod)args.arg1).createSession(channel,
-                            (IInputSessionCallback)args.arg3);
+                    method.createSession(channel, (IInputSessionCallback)args.arg3);
                 } catch (RemoteException e) {
                 } finally {
-                    if (channel != null) {
+                    // Dispose the channel if the input method is not local to this process
+                    // because the remote proxy will get its own copy when unparceled.
+                    if (channel != null && Binder.isProxy(method)) {
                         channel.dispose();
                     }
                 }
@@ -2404,16 +2407,24 @@ public class InputMethodManagerService extends IInputMethodManager.Stub
                     // There is nothing interesting about the last client dying.
                 }
                 return true;
-            case MSG_BIND_METHOD:
+            case MSG_BIND_METHOD: {
                 args = (SomeArgs)msg.obj;
+                IInputMethodClient client = (IInputMethodClient)args.arg1;
+                InputBindResult res = (InputBindResult)args.arg2;
                 try {
-                    ((IInputMethodClient)args.arg1).onBindMethod(
-                            (InputBindResult)args.arg2);
+                    client.onBindMethod(res);
                 } catch (RemoteException e) {
                     Slog.w(TAG, "Client died receiving input method " + args.arg2);
+                } finally {
+                    // Dispose the channel if the input method is not local to this process
+                    // because the remote proxy will get its own copy when unparceled.
+                    if (res.channel != null && Binder.isProxy(client)) {
+                        res.channel.dispose();
+                    }
                 }
                 args.recycle();
                 return true;
+            }
             case MSG_SET_ACTIVE:
                 try {
                     ((ClientState)msg.obj).client.setActive(msg.arg1 != 0);