Commit 24e35fb2 authored by Leo Zhang's avatar Leo Zhang Committed by Commit Bot

[IME] Fix CFI violation for IME service.

This will fix an init crash when shared library passes the MainEntry to
Chrome Utility process, which is a CFI violation.
This requires corresponding changes from decoder side to take effect.

TEST=local build Chrome with use_cfi
BUG=b:172527471

Change-Id: I6d9584533491be3e1fc489bc592075a49e90c5e4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2537516
Commit-Queue: Leo Zhang <googleo@chromium.org>
Reviewed-by: default avatarMatthew Denton <mpdenton@chromium.org>
Cr-Commit-Position: refs/heads/master@{#827560}
parent c35c9d13
...@@ -8,7 +8,6 @@ ...@@ -8,7 +8,6 @@
#include "base/files/file_path.h" #include "base/files/file_path.h"
#include "base/files/file_util.h" #include "base/files/file_util.h"
#include "chromeos/services/ime/constants.h" #include "chromeos/services/ime/constants.h"
#include "chromeos/services/ime/ime_decoder.h"
#include "chromeos/services/ime/public/cpp/buildflags.h" #include "chromeos/services/ime/public/cpp/buildflags.h"
namespace chromeos { namespace chromeos {
...@@ -66,12 +65,11 @@ DecoderEngine::DecoderEngine(ImeCrosPlatform* platform) : platform_(platform) { ...@@ -66,12 +65,11 @@ DecoderEngine::DecoderEngine(ImeCrosPlatform* platform) : platform_(platform) {
DecoderEngine::~DecoderEngine() {} DecoderEngine::~DecoderEngine() {}
bool DecoderEngine::TryLoadDecoder() { bool DecoderEngine::TryLoadDecoder() {
if (engine_main_entry_)
return true;
auto* decoder = ImeDecoder::GetInstance(); auto* decoder = ImeDecoder::GetInstance();
if (decoder->GetStatus() == ImeDecoder::Status::kSuccess) { if (decoder->GetStatus() == ImeDecoder::Status::kSuccess &&
engine_main_entry_ = decoder->CreateMainEntry(platform_); decoder->GetEntryPoints().isReady) {
decoder_entry_points_ = decoder->GetEntryPoints();
decoder_entry_points_->initOnce(platform_);
return true; return true;
} }
return false; return false;
...@@ -86,7 +84,7 @@ bool DecoderEngine::BindRequest( ...@@ -86,7 +84,7 @@ bool DecoderEngine::BindRequest(
// Activates an IME engine via the shared library. Passing a // Activates an IME engine via the shared library. Passing a
// |ClientDelegate| for engine instance created by the shared library to // |ClientDelegate| for engine instance created by the shared library to
// make safe calls on the client. // make safe calls on the client.
if (engine_main_entry_->ActivateIme( if (decoder_entry_points_->activateIme(
ime_spec.c_str(), ime_spec.c_str(),
new ClientDelegate(ime_spec, std::move(remote)))) { new ClientDelegate(ime_spec, std::move(remote)))) {
decoder_channel_receivers_.Add(this, std::move(receiver)); decoder_channel_receivers_.Add(this, std::move(receiver));
...@@ -102,8 +100,10 @@ bool DecoderEngine::BindRequest( ...@@ -102,8 +100,10 @@ bool DecoderEngine::BindRequest(
} }
bool DecoderEngine::IsImeSupportedByDecoder(const std::string& ime_spec) { bool DecoderEngine::IsImeSupportedByDecoder(const std::string& ime_spec) {
return engine_main_entry_ && if (decoder_entry_points_) {
engine_main_entry_->IsImeSupported(ime_spec.c_str()); return decoder_entry_points_->support(ime_spec.c_str());
}
return false;
} }
void DecoderEngine::ProcessMessage(const std::vector<uint8_t>& message, void DecoderEngine::ProcessMessage(const std::vector<uint8_t>& message,
...@@ -112,8 +112,9 @@ void DecoderEngine::ProcessMessage(const std::vector<uint8_t>& message, ...@@ -112,8 +112,9 @@ void DecoderEngine::ProcessMessage(const std::vector<uint8_t>& message,
std::vector<uint8_t> result; std::vector<uint8_t> result;
// Handle message via corresponding functions of loaded decoder. // Handle message via corresponding functions of loaded decoder.
if (engine_main_entry_) if (decoder_entry_points_) {
engine_main_entry_->Process(message.data(), message.size()); decoder_entry_points_->process(message.data(), message.size());
}
std::move(callback).Run(result); std::move(callback).Run(result);
} }
......
...@@ -5,7 +5,9 @@ ...@@ -5,7 +5,9 @@
#ifndef CHROMEOS_SERVICES_IME_DECODER_DECODER_ENGINE_H_ #ifndef CHROMEOS_SERVICES_IME_DECODER_DECODER_ENGINE_H_
#define CHROMEOS_SERVICES_IME_DECODER_DECODER_ENGINE_H_ #define CHROMEOS_SERVICES_IME_DECODER_DECODER_ENGINE_H_
#include "base/optional.h"
#include "base/scoped_native_library.h" #include "base/scoped_native_library.h"
#include "chromeos/services/ime/ime_decoder.h"
#include "chromeos/services/ime/input_engine.h" #include "chromeos/services/ime/input_engine.h"
#include "chromeos/services/ime/public/cpp/shared_lib/interfaces.h" #include "chromeos/services/ime/public/cpp/shared_lib/interfaces.h"
#include "chromeos/services/ime/public/mojom/input_engine.mojom.h" #include "chromeos/services/ime/public/mojom/input_engine.mojom.h"
...@@ -48,13 +50,10 @@ class DecoderEngine : public InputEngine { ...@@ -48,13 +50,10 @@ class DecoderEngine : public InputEngine {
// Returns whether the decoder shared library supports this ime_spec. // Returns whether the decoder shared library supports this ime_spec.
bool IsImeSupportedByDecoder(const std::string& ime_spec); bool IsImeSupportedByDecoder(const std::string& ime_spec);
// Shared library handle of the implementation for input logic with decoders.
base::ScopedNativeLibrary library_;
ImeEngineMainEntry* engine_main_entry_ = nullptr;
ImeCrosPlatform* platform_ = nullptr; ImeCrosPlatform* platform_ = nullptr;
base::Optional<ImeDecoder::EntryPoints> decoder_entry_points_;
mojo::ReceiverSet<mojom::InputChannel> decoder_channel_receivers_; mojo::ReceiverSet<mojom::InputChannel> decoder_channel_receivers_;
DISALLOW_COPY_AND_ASSIGN(DecoderEngine); DISALLOW_COPY_AND_ASSIGN(DecoderEngine);
......
...@@ -57,9 +57,6 @@ class SystemEngine : public InputEngine { ...@@ -57,9 +57,6 @@ class SystemEngine : public InputEngine {
void OnReply(const std::vector<uint8_t>& message, void OnReply(const std::vector<uint8_t>& message,
mojo::Remote<mojom::InputChannel>& remote); mojo::Remote<mojom::InputChannel>& remote);
// Shared library handle of the implementation for input logic with decoders.
base::ScopedNativeLibrary library_;
ImeEngineMainEntry* engine_main_entry_ = nullptr; ImeEngineMainEntry* engine_main_entry_ = nullptr;
ImeCrosPlatform* platform_ = nullptr; ImeCrosPlatform* platform_ = nullptr;
......
...@@ -45,6 +45,13 @@ void ImeLoggerBridge(int severity, const char* message) { ...@@ -45,6 +45,13 @@ void ImeLoggerBridge(int severity, const char* message) {
break; break;
} }
} }
// Check whether the crucial members of an EntryPoints are loaded.
bool isEntryPointsLoaded(ImeDecoder::EntryPoints entry) {
return (entry.initOnce && entry.support && entry.activateIme &&
entry.process && entry.closeDecoder);
}
} // namespace } // namespace
ImeDecoder::ImeDecoder() : status_(Status::kUninitialized) { ImeDecoder::ImeDecoder() : status_(Status::kUninitialized) {
...@@ -65,21 +72,35 @@ ImeDecoder::ImeDecoder() : status_(Status::kUninitialized) { ...@@ -65,21 +72,35 @@ ImeDecoder::ImeDecoder() : status_(Status::kUninitialized) {
return; return;
} }
// TODO(b/172527471): Remove it when decoder DSO is uprevved.
createMainEntry_ = reinterpret_cast<ImeMainEntryCreateFn>( createMainEntry_ = reinterpret_cast<ImeMainEntryCreateFn>(
library.GetFunctionPointer(IME_MAIN_ENTRY_CREATE_FN_NAME)); library.GetFunctionPointer(IME_MAIN_ENTRY_CREATE_FN_NAME));
if (!createMainEntry_) {
// TODO(b/172527471): Create a macro to fetch function pointers.
entry_points_.initOnce = reinterpret_cast<ImeDecoderInitOnceFn>(
library.GetFunctionPointer("ImeDecoderInitOnce"));
entry_points_.support = reinterpret_cast<ImeDecoderSupportsFn>(
library.GetFunctionPointer("ImeDecoderSupports"));
entry_points_.activateIme = reinterpret_cast<ImeDecoderActivateImeFn>(
library.GetFunctionPointer("ImeDecoderActivateIme"));
entry_points_.process = reinterpret_cast<ImeDecoderProcessFn>(
library.GetFunctionPointer("ImeDecoderProcess"));
entry_points_.closeDecoder = reinterpret_cast<ImeDecoderCloseFn>(
library.GetFunctionPointer("ImeDecoderClose"));
if (!isEntryPointsLoaded(entry_points_)) {
status_ = Status::kFunctionMissing; status_ = Status::kFunctionMissing;
return; return;
} }
entry_points_.isReady = true;
// Optional function pointer.
ImeEngineLoggerSetterFn loggerSetter = ImeEngineLoggerSetterFn loggerSetter =
reinterpret_cast<ImeEngineLoggerSetterFn>( reinterpret_cast<ImeEngineLoggerSetterFn>(
library.GetFunctionPointer("SetImeEngineLogger")); library.GetFunctionPointer("SetImeEngineLogger"));
if (loggerSetter) { if (loggerSetter) {
loggerSetter(ImeLoggerBridge); loggerSetter(ImeLoggerBridge);
} else { } else {
// Not a blocking issue yet. LOG(WARNING) << "Failed to set a Chrome Logger for decoder DSO.";
LOG(ERROR) << "Failed to load SetImeEngineLogger function.";
} }
library_ = std::move(library); library_ = std::move(library);
...@@ -97,10 +118,16 @@ ImeDecoder::Status ImeDecoder::GetStatus() const { ...@@ -97,10 +118,16 @@ ImeDecoder::Status ImeDecoder::GetStatus() const {
return status_; return status_;
} }
// TODO(b/172527471): Remove it when decoder DSO is uprevved.
ImeEngineMainEntry* ImeDecoder::CreateMainEntry(ImeCrosPlatform* platform) { ImeEngineMainEntry* ImeDecoder::CreateMainEntry(ImeCrosPlatform* platform) {
DCHECK(status_ == Status::kSuccess); DCHECK(createMainEntry_);
return createMainEntry_(platform); return createMainEntry_(platform);
} }
ImeDecoder::EntryPoints ImeDecoder::GetEntryPoints() {
DCHECK(status_ == Status::kSuccess);
return entry_points_;
}
} // namespace ime } // namespace ime
} // namespace chromeos } // namespace chromeos
...@@ -28,6 +28,19 @@ class ImeDecoder { ...@@ -28,6 +28,19 @@ class ImeDecoder {
kFunctionMissing = 4, kFunctionMissing = 4,
}; };
// This contains the function pointers to the entry points for the loaded
// decoder shared library.
struct EntryPoints {
ImeDecoderInitOnceFn initOnce;
ImeDecoderSupportsFn support;
ImeDecoderActivateImeFn activateIme;
ImeDecoderProcessFn process;
ImeDecoderCloseFn closeDecoder;
// Whether the EntryPoints is ready to use.
bool isReady;
};
// Gets the singleton ImeDecoder. // Gets the singleton ImeDecoder.
static ImeDecoder* GetInstance(); static ImeDecoder* GetInstance();
...@@ -36,8 +49,12 @@ class ImeDecoder { ...@@ -36,8 +49,12 @@ class ImeDecoder {
Status GetStatus() const; Status GetStatus() const;
// Returns an instance of ImeEngineMainEntry from the IME shared library. // Returns an instance of ImeEngineMainEntry from the IME shared library.
// TODO(b/172527471): Remove it when decoder DSO is uprevved.
ImeEngineMainEntry* CreateMainEntry(ImeCrosPlatform* platform); ImeEngineMainEntry* CreateMainEntry(ImeCrosPlatform* platform);
// Returns entry points of the loaded decoder shared library.
EntryPoints GetEntryPoints();
private: private:
friend class base::NoDestructor<ImeDecoder>; friend class base::NoDestructor<ImeDecoder>;
...@@ -49,8 +66,13 @@ class ImeDecoder { ...@@ -49,8 +66,13 @@ class ImeDecoder {
// Result of IME decoder DSO initialization. // Result of IME decoder DSO initialization.
base::Optional<base::ScopedNativeLibrary> library_; base::Optional<base::ScopedNativeLibrary> library_;
// Function pointors from decoder DSO.
// TODO(b/172527471): Remove it when decoder DSO is uprevved.
ImeMainEntryCreateFn createMainEntry_; ImeMainEntryCreateFn createMainEntry_;
EntryPoints entry_points_;
DISALLOW_COPY_AND_ASSIGN(ImeDecoder); DISALLOW_COPY_AND_ASSIGN(ImeDecoder);
}; };
......
...@@ -209,6 +209,7 @@ class ImeClientDelegate { ...@@ -209,6 +209,7 @@ class ImeClientDelegate {
// This class is implemented in the shared library and processes messages from // This class is implemented in the shared library and processes messages from
// clients of the IME service. The shared library will exposes its create // clients of the IME service. The shared library will exposes its create
// function to the IME service. // function to the IME service.
// DEPRECATED: Will be removed soon.
class ImeEngineMainEntry { class ImeEngineMainEntry {
protected: protected:
virtual ~ImeEngineMainEntry() = default; virtual ~ImeEngineMainEntry() = default;
...@@ -252,6 +253,12 @@ typedef ImeEngineMainEntry* (*ImeMainEntryCreateFn)(ImeCrosPlatform*); ...@@ -252,6 +253,12 @@ typedef ImeEngineMainEntry* (*ImeMainEntryCreateFn)(ImeCrosPlatform*);
// For use when bridging logs logged in IME shared library to Chrome logging. // For use when bridging logs logged in IME shared library to Chrome logging.
typedef void (*ImeEngineLoggerSetterFn)(ChromeLoggerFunc); typedef void (*ImeEngineLoggerSetterFn)(ChromeLoggerFunc);
typedef void (*ImeDecoderInitOnceFn)(ImeCrosPlatform*);
typedef bool (*ImeDecoderSupportsFn)(const char*);
typedef bool (*ImeDecoderActivateImeFn)(const char*, ImeClientDelegate*);
typedef void (*ImeDecoderProcessFn)(const uint8_t*, size_t);
typedef void (*ImeDecoderCloseFn)();
// Defined name of ImeMainEntryCreateFn exported from shared library. // Defined name of ImeMainEntryCreateFn exported from shared library.
#define IME_MAIN_ENTRY_CREATE_FN_NAME "CreateImeMainEntry" #define IME_MAIN_ENTRY_CREATE_FN_NAME "CreateImeMainEntry"
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment