0

Reland "TTS: Modify TTS service to work with XNNPack (part 2)"

This is a reland of commit 193134b93a

This change was originally reverted because it was breaking Google TTS.
The reason is because the chromiumos side had not been updated. This
change can be safely relanded (since we just started M119) as long as
the following change is also landed at the same time:

https://crrev.com/c/4826255

Original change's description:
> TTS: Modify TTS service to work with XNNPack (part 2)
>
> This change is the second half of fixing the TTS - XNNPack bug.
> This change does several things:
>
> 1. Calls GoogleTtsPreSandboxInit (added in an internal change),
> which explicitly initializes cpuinfo. This is done because cpuinfo
> is needed by some XNNPack code within libchrometts.so.
> 2. Gives the TTS service permission to call getcpu, which is used on
> arm64. Without this change, TTS doesn't work on arm64.
> 3. Removes the read permission to /proc/cpuinfo, which was initially
> added to address b:269146620 on x86_64. This was a temporary fix
> and is now unnecessary given the above two changes. See this design
> doc [1] for more details.
>
> [1] https://docs.google.com/document/d/1pU8hRzE0Z_eph-MeVYv_EWlbtrRhxXmHEwHbtPVoXhM/edit?usp=sharing
>
> (from fig client) # Run script to deploy Google TTS
> (from chrome-sdk) autoninja -C out_${SDK_BOARD}/Release chrome nacl_helper
> (from chrome-sdk) deploy_chrome --device=localhost:2224 --build-dir=out_${SDK_BOARD}/Release
> (on device) Turn on ChromeVox and ensure speech output.
>
> Bug: b:269146620,b:296458143
> Test: Manually on x86_64 and arm64.
> Change-Id: Ifc500da14a0f9207cac8e0fb177a8bbddc259489
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4503801
> Commit-Queue: Akihiro Ota <akihiroota@chromium.org>
> Reviewed-by: David Tseng <dtseng@chromium.org>
> Reviewed-by: Matthew Denton <mpdenton@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#1190179}

Bug: b:269146620,b:296458143
Change-Id: I0d46474fd4579891949b90910bf33c38286529c5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4849990
Reviewed-by: David Tseng <dtseng@chromium.org>
Reviewed-by: Matthew Denton <mpdenton@chromium.org>
Commit-Queue: Akihiro Ota <akihiroota@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1193746}
This commit is contained in:
Akihiro Ota
2023-09-07 20:11:52 +00:00
committed by Chromium LUCI CQ
parent 58d682dc81
commit 0c27605e43
5 changed files with 16 additions and 1 deletions

@ -50,6 +50,7 @@ generate_library_loader("libchrometts") {
functions = [
"GoogleTtsSetLogger",
"GoogleTtsPreSandboxInit",
"GoogleTtsInit",
"GoogleTtsShutdown",
"GoogleTtsInstallVoice",

@ -1 +1,4 @@
file://ui/accessibility/OWNERS
per-file tts_sandbox_hook.*=set noparent
per-file tts_sandbox_hook.*=file://sandbox/linux/OWNERS

@ -10,6 +10,8 @@
void GoogleTtsSetLogger(void (*logger_func)(int severity, const char* message));
void GoogleTtsPreSandboxInit();
bool GoogleTtsInit(const char* pipeline_path, const char* path_prefix);
void GoogleTtsShutdown();

@ -11,6 +11,7 @@
#include "base/files/file_util.h"
#include "base/logging.h"
#include "chromeos/services/tts/constants.h"
#include "library_loaders/libchrometts.h"
#include "sandbox/linux/syscall_broker/broker_command.h"
#include "sandbox/linux/syscall_broker/broker_file_permission.h"
@ -40,7 +41,6 @@ void AddReadOnlyFiles(std::vector<BrokerFilePermission>* permissions) {
// These files are required for some syscalls e.g. get_nprocs, sysinfo.
permissions->push_back(BrokerFilePermission::ReadOnly("/proc/stat"));
permissions->push_back(BrokerFilePermission::ReadOnly("/proc/meminfo"));
permissions->push_back(BrokerFilePermission::ReadOnly("/proc/cpuinfo"));
}
std::vector<BrokerFilePermission> GetTtsFilePermissions() {
@ -55,6 +55,14 @@ bool TtsPreSandboxHook(sandbox::policy::SandboxLinux::Options options) {
if (!dlopen(kLibchromettsPath, RTLD_LAZY))
LOG(ERROR) << "Unable to open libchrometts.so: " << dlerror();
LibChromeTtsLoader loader;
if (loader.Load(kLibchromettsPath)) {
loader.GoogleTtsPreSandboxInit();
} else {
LOG(ERROR) << "Unable to load libchrometts.so and perform pre-sandbox "
"initialization";
}
// Ensure this directory is created.
base::FilePath temp_data_dir(kTempDataDirectory);
base::CreateDirectoryAndGetError(temp_data_dir, nullptr);

@ -26,6 +26,7 @@ TtsProcessPolicy::~TtsProcessPolicy() {}
ResultExpr TtsProcessPolicy::EvaluateSyscall(int sysno) const {
switch (sysno) {
case __NR_getcpu:
case __NR_sysinfo:
return Allow();
default: