Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit cb768bcb authored by Yohei Yukawa's avatar Yohei Yukawa
Browse files

Fix local Binder call scenario of IMMS#addClient() 

This is a follow up CL to my previous CL [1] that enabled per-display
InputMethodManager (IMM) instance.

One thing I mistakenly assumed in my previous CL is that there is
always a process boundary between IMM#createRealInstance() and
IMMS#addClient(), which is not true when IMM is being instantiated in
the system server process.  This means that IMMS may associate an IME
client to wrong PID/UID/DisplayID when certain conditions are met.

With this CL, IMM#createRealInstance() always clears calling identity
so chat IMMS#addClient() can work correctly even for local Binder call
scenarios.

 [1]: I78ad7cccb9586474c83f7e2f90c0bcabb221c47b
      4052a10f

Bug: 115893206
Bug: 117594679
Fix: 118335706
Test: atest ActivityManagerMultiDisplayTests
Test: atest CtsInputMethodTestCases CtsInputMethodServiceHostTestCases
Test: atest FrameworksCoreTests:android.view.inputmethod.InputMethodManagerTest
Test: Manually tested with manually simulating the problematic scenario
Change-Id: I318d32d8997b0acb4fb193fe46831bdc9a4dd083
parent f682b1a5

core/java/android/view/inputmethod/InputMethodManager.java

+11 −0
Original line number Diff line number Diff line
@@ -30,6 +30,7 @@ import android.content.Context; 
import android.content.pm.PackageManager;

import android.graphics.Rect;

import android.inputmethodservice.InputMethodService;

import android.os.Binder;

import android.os.Build;

import android.os.Bundle;

import android.os.Handler;
@@ -753,10 +754,20 @@ public final class InputMethodManager { 
            throw new IllegalStateException(e);

        }

        final InputMethodManager imm = new InputMethodManager(service, displayId, looper);

        // InputMethodManagerService#addClient() relies on Binder.getCalling{Pid, Uid}() to

        // associate PID/UID with each IME client. This means:

        //  A. if this method call will be handled as an IPC, there is no problem.

        //  B. if this method call will be handled as an in-proc method call, we need to

        //     ensure that Binder.getCalling{Pid, Uid}() return Process.my{Pid, Uid}()

        // Either ways we can always call Binder.{clear, restore}CallingIdentity() because

        // 1) doing so has no effect for A and 2) doing so is sufficient for B.

        final long identity = Binder.clearCallingIdentity();

        try {

            service.addClient(imm.mClient, imm.mIInputContext, displayId);

        } catch (RemoteException e) {

            e.rethrowFromSystemServer();

        } finally {

            Binder.restoreCallingIdentity(identity);

        }

        return imm;

    }

services/core/java/com/android/server/inputmethod/InputMethodManagerService.java

+7 −1
Original line number Diff line number Diff line
@@ -1770,6 +1770,12 @@ public class InputMethodManagerService extends IInputMethodManager.Stub 
    @Override

    public void addClient(IInputMethodClient client, IInputContext inputContext,

            int selfReportedDisplayId) {

        // Here there are two scenarios where this method is called:

        // A. IMM is being instantiated in a different process and this is an IPC from that process

        // B. IMM is being instantiated in the same process but Binder.clearCallingIdentity() is

        //    called in the caller side if necessary.

        // In either case the following UID/PID should be the ones where InputMethodManager is

        // actually running.

        final int callerUid = Binder.getCallingUid();

        final int callerPid = Binder.getCallingPid();

        synchronized (mMethodMap) {
@@ -1778,7 +1784,7 @@ public class InputMethodManagerService extends IInputMethodManager.Stub 
                if (state.uid == callerUid && state.pid == callerPid

                        && state.selfReportedDisplayId == selfReportedDisplayId) {

                    throw new SecurityException("uid=" + callerUid + "/pid=" + callerPid

                            + " is already registered");

                            + "/displayId=" + selfReportedDisplayId + " is already registered.");

                }

            }

            final ClientDeathRecipient deathRecipient = new ClientDeathRecipient(this, client);