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

Commit 7fa65eef authored by Yohei Yukawa's avatar Yohei Yukawa
Browse files

Avoid sync IPCs from TSMS to SpellCheckerService 

Currently, TextServicesManagerServices uses an AIDL interface called
ISpellCheckerService when binding to a spell-checking service.
However, this interface uses synchronous (blocking) binder calls
rather than asynchronous (oneway) calls.  As a result, there are
situations where the system process has made a blocking binder call
into untrusted application code from its main looper thread.

As general policy, the system process must never allow its looper
threads to block on application code.

This CL addresses the above issue by converting ISpellCheckerService
into oneway interface, which instead takes a result receiver
ISpellCheckerServiceCallback so that spell-checking services can
return results asynchronously.

Note that the above protocol issue was also the root cause of
Bug 5471520.  Hence we can also logically revert the previous CL [1]
for Bug 5471520.

 [1]: Iedf2c2cdd8d4834545d06d72ade3ce211b104b1d
      4e713f14

Test: Ran `adb shell dumpsys textservices` to check the
      "Spell Checker Bind Groups:" section in the following three
      steps.
      1. Before apps start requesting spell checker sessions.
      2. While apps are owning active spell checker sessions.
      3. After all the apps that owned spell checker sessions are
         gone.
      Made sure that spell checker service is not running when
      there is not spell checker bind group.
Bug: 7254002
Change-Id: I92e7aa40dc9ea14f67d355f0bfa15325b775d27b
parent fd62c58e

Android.mk

+1 −0
Original line number Diff line number Diff line
@@ -370,6 +370,7 @@ LOCAL_SRC_FILES += \ 
	core/java/com/android/internal/statusbar/IStatusBar.aidl \

	core/java/com/android/internal/statusbar/IStatusBarService.aidl \

	core/java/com/android/internal/textservice/ISpellCheckerService.aidl \

	core/java/com/android/internal/textservice/ISpellCheckerServiceCallback.aidl \

	core/java/com/android/internal/textservice/ISpellCheckerSession.aidl \

	core/java/com/android/internal/textservice/ISpellCheckerSessionListener.aidl \

	core/java/com/android/internal/textservice/ITextServicesManager.aidl \

core/java/android/service/textservice/SpellCheckerService.java

+32 −8
Original line number Diff line number Diff line
@@ -17,6 +17,7 @@ 
package android.service.textservice;



import com.android.internal.textservice.ISpellCheckerService;

import com.android.internal.textservice.ISpellCheckerServiceCallback;

import com.android.internal.textservice.ISpellCheckerSession;

import com.android.internal.textservice.ISpellCheckerSessionListener;
@@ -311,16 +312,39 @@ public abstract class SpellCheckerService extends Service { 
            mInternalServiceRef = new WeakReference<SpellCheckerService>(service);

        }



        /**

         * Called from the system when an application is requesting a new spell checker session.

         *

         * <p>Note: This is an internal protocol used by the system to establish spell checker

         * sessions, which is not guaranteed to be stable and is subject to change.</p>

         *

         * @param locale locale to be returned from {@link Session#getLocale()}

         * @param listener IPC channel object to be used to implement

         *                 {@link Session#onGetSuggestionsMultiple(TextInfo[], int, boolean)} and

         *                 {@link Session#onGetSuggestions(TextInfo, int)}

         * @param bundle bundle to be returned from {@link Session#getBundle()}

         * @param callback IPC channel to return the result to the caller in an asynchronous manner

         */

        @Override

        public ISpellCheckerSession getISpellCheckerSession(

                String locale, ISpellCheckerSessionListener listener, Bundle bundle) {

        public void getISpellCheckerSession(

                String locale, ISpellCheckerSessionListener listener, Bundle bundle,

                ISpellCheckerServiceCallback callback) {

            final SpellCheckerService service = mInternalServiceRef.get();

            if (service == null) return null;

            final InternalISpellCheckerSession internalSession;

            if (service == null) {

                // If the owner SpellCheckerService object was already destroyed and got GC-ed,

                // the weak-reference returns null and we should just ignore this request.

                internalSession = null;

            } else {

                final Session session = service.createSession();

            final InternalISpellCheckerSession internalSession =

                internalSession =

                        new InternalISpellCheckerSession(locale, listener, bundle, session);

                session.onCreate();

            return internalSession;

            }

            try {

                callback.onSessionCreated(internalSession);

            } catch (RemoteException e) {

            }

        }

    }

core/java/com/android/internal/textservice/ISpellCheckerService.aidl

+20 −4
Original line number Diff line number Diff line
@@ -16,16 +16,32 @@ 


package com.android.internal.textservice;



import com.android.internal.textservice.ISpellCheckerServiceCallback;

import com.android.internal.textservice.ISpellCheckerSession;

import com.android.internal.textservice.ISpellCheckerSessionListener;



import android.os.Bundle;



/**

 * Public interface to the global spell checker.

 * IPC channels from TextServicesManagerService to SpellCheckerService.

 * @hide

 */

interface ISpellCheckerService {

    ISpellCheckerSession getISpellCheckerSession(

            String locale, ISpellCheckerSessionListener listener, in Bundle bundle);

oneway interface ISpellCheckerService {

    /**

     * Called from the system when an application is requesting a new spell checker session.

     *

     * <p>Note: This is an internal protocol used by the system to establish spell checker sessions,

     * which is not guaranteed to be stable and is subject to change.</p>

     *

     * @param locale locale to be returned from

     *               {@link android.service.textservice.SpellCheckerService.Session#getLocale()}

     * @param listener IPC channel object to be used to implement

     *                 {@link android.service.textservice.SpellCheckerService.Session#onGetSuggestionsMultiple(TextInfo[], int, boolean)} and

     *                 {@link android.service.textservice.SpellCheckerService.Session#onGetSuggestions(TextInfo, int)}

     * @param bundle bundle to be returned from {@link android.service.textservice.SpellCheckerService.Session#getBundle()}

     * @param callback IPC channel to return the result to the caller in an asynchronous manner

     */

    void getISpellCheckerSession(

            String locale, ISpellCheckerSessionListener listener, in Bundle bundle,

            ISpellCheckerServiceCallback callback);

}

core/java/com/android/internal/textservice/ISpellCheckerServiceCallback.aidl

0 → 100644
+35 −0
Original line number Diff line number Diff line 
/*

 * Copyright (C) 2017 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.internal.textservice;



import com.android.internal.textservice.ISpellCheckerSession;

import com.android.internal.textservice.ISpellCheckerSessionListener;



import android.os.Bundle;



/**

 * IPC channels from SpellCheckerService to TextServicesManagerService.

 * @hide

 */

oneway interface ISpellCheckerServiceCallback {

    // TODO: Currently SpellCheckerSession just ignores null newSession and continues waiting for

    // the next onSessionCreated with non-null newSession, which is supposed to never happen if

    // the system is working normally. We should at least free up resources in SpellCheckerSession.

    // Note: This method is called from non-system processes, in theory we cannot assume that

    // this method is always be called only once with non-null value.

    void onSessionCreated(ISpellCheckerSession newSession);

}

core/java/com/android/internal/textservice/ITextServicesSessionListener.aidl

+1 −1
Original line number Diff line number Diff line
@@ -21,7 +21,7 @@ import com.android.internal.textservice.ISpellCheckerSession; 
import android.view.textservice.SpellCheckerInfo;



/**

 * Interface to the text service session.

 * (Per-session) IPC channels from TextServicesManagerService to spell checker client applications.

 * @hide

 */

interface ITextServicesSessionListener {