Revert "nn: use cryptohome daemon storage location for cache files"
This reverts commit a69a9e160ff4db6cd8971d30d023aae1379a5c9a.
Reason for revert: Has broken relesae builds: https://logs.chromium.org/logs/chromeos/buildbucket/cr-buildbucket.appspot.com/8848535058247719152/+/steps/BuildPackages/0/stdout
Original change's description:
> nn: use cryptohome daemon storage location for cache files
>
> This CL forces any request that sets up compilation caching to store
> the files in the cryptohome daemon storage location of the ml_service
> process. This is done due to the docs indicating that cache files should
> be stored in a secure location. See:
> https://source.android.com/devices/neural-networks/compilation-caching#security
>
> We can't do this in configuration since the daemon storage is per-user
> and the username hash needs to be queried at runtime.
>
> BUG=b:178762478
> TEST=FEATURES=test emerge-volteer aosp-frameworks-ml-nn
>
> Change-Id: I09d5d4be01a3435cedda4e822a4f423f8723e5ce
> Reviewed-on: https://chromium-review.googlesource.com/c/aosp/platform/frameworks/ml/+/2817759
> Tested-by: Jim Pollock <jmpollock@chromium.org>
> Auto-Submit: Jim Pollock <jmpollock@chromium.org>
> Commit-Queue: Jim Pollock <jmpollock@chromium.org>
> Reviewed-by: Stuart Langley <slangley@chromium.org>
> Reviewed-by: Michael Pishchagin <mblsha@google.com>
Bug: b:178762478
Change-Id: Iac9913c54332c5fb5f1205d7b6ed9c58825b866c
Reviewed-on: https://chromium-review.googlesource.com/c/aosp/platform/frameworks/ml/+/2859271
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Reviewed-by: David Riley <davidriley@chromium.org>
Commit-Queue: Jim Pollock <jmpollock@chromium.org>
Auto-Submit: Jim Pollock <jmpollock@chromium.org>
diff --git a/nn/BUILD.gn b/nn/BUILD.gn
index 406d2c3..be3d506 100644
--- a/nn/BUILD.gn
+++ b/nn/BUILD.gn
@@ -73,9 +73,7 @@
pkg_deps = [
"libnnapi-support",
"openssl",
- "libbrillo",
"libchrome",
- "libsession_manager-client",
]
}
@@ -112,12 +110,9 @@
include_dirs = [
"common/include",
"driver/sample",
- "runtime",
"runtime/include",
]
sources = [
- "chromeos/compilation_caching.cpp",
- "chromeos/daemon_store.cpp",
"chromeos/versioned_drivers.cpp",
"runtime/BurstBuilder.cpp",
"runtime/Callbacks.cpp",
@@ -406,7 +401,6 @@
executable("chromeos_testrunner") {
include_dirs = [
- "chromeos",
"common/random",
]
configs += [
@@ -420,10 +414,8 @@
deps = [
":nn-common",
":nn_testrunner",
- ":runtime",
]
sources = [
- "chromeos/tests/common/daemon_store_test.cc",
"chromeos/tests/common/includes_test.cc",
"chromeos/tests/common/logger_enum_test.cc",
"chromeos/tests/common/random_test.cc",
diff --git a/nn/chromeos/compilation_caching.cpp b/nn/chromeos/compilation_caching.cpp
deleted file mode 100644
index 7ffb857..0000000
--- a/nn/chromeos/compilation_caching.cpp
+++ /dev/null
@@ -1,43 +0,0 @@
-// Copyright 2021 The Chromium OS Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-#include "CompilationBuilder.h"
-#include "NeuralNetworks.h"
-#include "Tracing.h"
-#include "daemon_store.h"
-
-using namespace android::nn;
-using namespace android::nn::hal;
-
-// The Android NNAPI documentation recommends store compilation
-// cache files in a secure location. See:
-// https://source.android.com/devices/neural-networks/compilation-caching#security
-//
-// In ChromeOS, this is put in the 'cryptohome' folder of the user, which
-// needs to be queried at runtime since it contains a hashed value of the
-// username. Ideally, this would simply be done at the client level and
-// then passed into ANeuralNetworksCompilation_setCaching, however in cases
-// like SODA, it has no knowledge that it is running in ChromeOS and so
-// getting that information up to the caller would require extending the
-// NNAPI specification to offer an interface to it.
-//
-// Instead, we take the requested cache dir location and append it to the
-// calculated cryptohome location 'under the hood'.
-int ANeuralNetworksCompilation_setCaching(
- ANeuralNetworksCompilation* compilation, const char* cache_dir,
- const uint8_t* token) {
- NNTRACE_RT(NNTRACE_PHASE_COMPILATION,
- "ANeuralNetworksCompilation_setCaching");
-
- if (!compilation || !cache_dir || !token) {
- LOG(ERROR) << "ANeuralNetworksCompilation_setCaching passed a nullptr";
- return ANEURALNETWORKS_UNEXPECTED_NULL;
- }
-
- // Append the requested cache dir onto the daemon storage location.
- std::string cache_store_dir = DaemonStore::Get().GetDaemonStoreDir(cache_dir);
-
- CompilationBuilder* c = reinterpret_cast<CompilationBuilder*>(compilation);
- return c->setCaching(cache_store_dir.c_str(), token);
-}
diff --git a/nn/chromeos/daemon_store.cpp b/nn/chromeos/daemon_store.cpp
deleted file mode 100644
index e2eb7c4..0000000
--- a/nn/chromeos/daemon_store.cpp
+++ /dev/null
@@ -1,109 +0,0 @@
-// Copyright 2021 The Chromium OS Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-#include "daemon_store.h"
-
-#include <base/files/file_util.h>
-#include <session_manager/dbus-proxies.h>
-
-using org::chromium::SessionManagerInterfaceProxy;
-
-namespace {
-constexpr char dbus_path_[] = "/run/dbus/system_bus_socket";
-constexpr char nnapi_subdir_[] = "nnapi_compilations";
-
-static std::unique_ptr<SessionManagerInterfaceProxy> SetUpDBus(
- scoped_refptr<dbus::Bus> bus) {
- if (!bus) {
- dbus::Bus::Options options;
- options.bus_type = dbus::Bus::SYSTEM;
-
- bus = new dbus::Bus(options);
- CHECK(bus->Connect());
- }
- return std::make_unique<SessionManagerInterfaceProxy>(bus);
-}
-
-class UserHash : public UserHashInterface {
- public:
- virtual std::string GetCurrentUserHash() const override {
- if (!base::PathExists(base::FilePath(dbus_path_))) {
- LOG(ERROR) << "Cannot query DBUS.";
- return "";
- }
-
- scoped_refptr<dbus::Bus> bus;
- auto sessionManagerProxy = SetUpDBus(bus);
-
- brillo::ErrorPtr error;
- std::string username;
- std::string hashed_username;
- if (!sessionManagerProxy->RetrievePrimarySession(
- &username, &hashed_username, &error)) {
- LOG(ERROR) << "Error in DBus call RetrievePrimarySession: "
- << error->GetMessage();
- return "";
- }
-
- return hashed_username;
- }
-};
-
-} // namespace
-
-DaemonStore& DaemonStore::Get() {
- static base::NoDestructor<DaemonStore> instance;
- return *instance.get();
-}
-
-DaemonStore::DaemonStore() : user_hasher_(std::make_unique<UserHash>()) {}
-
-void DaemonStore::SetUserHasherForTesting(
- std::unique_ptr<UserHashInterface> user_hasher) {
- user_hasher_ = std::move(user_hasher);
-}
-
-void DaemonStore::SetDaemonStoreBaseForTesting(const std::string& new_base) {
- daemon_store_base_ = new_base;
-}
-
-std::string DaemonStore::GetDaemonStoreBaseForTesting() {
- return daemon_store_base_;
-}
-
-std::string DaemonStore::GetDaemonStoreDir(
- const std::string& cache_subdir) const {
- std::string hashed_username = user_hasher_->GetCurrentUserHash();
- if (hashed_username.empty()) {
- LOG(ERROR) << "No active user session.";
- return "";
- }
-
- base::FilePath caching_dir =
- base::FilePath(daemon_store_base_).Append(hashed_username);
- if (!base::DirectoryExists(caching_dir)) {
- LOG(ERROR) << "User daemon-store directory (" << caching_dir
- << ") doesn't exist.";
- return "";
- }
-
- caching_dir = caching_dir.Append(nnapi_subdir_);
- if (!cache_subdir.empty()) {
- // Remove any leading slashes
- if (cache_subdir[0] == '/')
- caching_dir = caching_dir.Append(cache_subdir.substr(1));
- else
- caching_dir = caching_dir.Append(cache_subdir);
- }
-
- // Create the subdirectories
- base::File::Error error;
- if (!base::CreateDirectoryAndGetError(caching_dir, &error)) {
- LOG(ERROR) << "Could not create NNAPI caching dir: " << caching_dir
- << ", error: " << error;
- return "";
- }
-
- return caching_dir.StripTrailingSeparators().value();
-}
diff --git a/nn/chromeos/daemon_store.h b/nn/chromeos/daemon_store.h
deleted file mode 100644
index 8cb4086..0000000
--- a/nn/chromeos/daemon_store.h
+++ /dev/null
@@ -1,64 +0,0 @@
-// Copyright 2021 The Chromium OS Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-#ifndef ML_NN_CHROMEOS_DAEMON_STORE_H_
-#define ML_NN_CHROMEOS_DAEMON_STORE_H_
-
-#include <base/no_destructor.h>
-
-#include <string>
-
-// This interface is used to get the hashed version of the
-// current user to determine the daemon storage location.
-// Using an interface allows us to mock this for tests, since
-// the DBus call won't be available when testing.
-// See
-// https://chromium.googlesource.com/chromiumos/docs/+/HEAD/sandboxing.md#securely-mounting-cryptohome-daemon-store-folders
-class UserHashInterface {
- public:
- virtual std::string GetCurrentUserHash() const = 0;
- virtual ~UserHashInterface() = default;
-
- protected:
- UserHashInterface() = default;
-};
-
-// Singleton class used to get the daemon store location
-// used by compilation caching to store cache files. NNAPI
-// guidance indicates these files should be stored in
-// a secure location.
-class DaemonStore {
- public:
- static DaemonStore& Get();
- DaemonStore(const DaemonStore&) = delete;
-
- // Returns a string to the daemon store directory for
- // the current user with `cacheSubDir` appended. Returns an empty string if an
- // error occurs. Ideally we would return a base::FilePath, but we can't
- // include this as it has macros that clash with the android defined ones.
- std::string GetDaemonStoreDir(const std::string& cache_subdir) const;
-
- // NOTE: THESE METHODS ARE FOR TESTING ONLY.
- // Allow the interface to get the user hash to be changed.
- void SetUserHasherForTesting(std::unique_ptr<UserHashInterface> user_hasher);
- // Allow the base directory for the daemon storage location to be changed.
- void SetDaemonStoreBaseForTesting(const std::string& new_base);
- std::string GetDaemonStoreBaseForTesting();
-
- private:
- DaemonStore();
- friend class base::NoDestructor<DaemonStore>;
-
- std::unique_ptr<UserHashInterface> user_hasher_;
- std::string daemon_store_base_ = "/run/daemon-store/ml_service";
-};
-
-class MockUserHashForTesting : public UserHashInterface {
- public:
- virtual std::string GetCurrentUserHash() const override {
- return "mock-user-hash";
- }
-};
-
-#endif // ML_NN_CHROMEOS_DAEMON_STORE_H_
diff --git a/nn/chromeos/testrunner.cpp b/nn/chromeos/testrunner.cpp
index 706b639..d60015a 100644
--- a/nn/chromeos/testrunner.cpp
+++ b/nn/chromeos/testrunner.cpp
@@ -2,16 +2,13 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
-#include <stdlib.h>
-
-#include <filesystem>
#include <memory>
+#include <stdlib.h>
#include <gmock/gmock.h>
#include <gtest/gtest.h>
#include "android-base/logging.h"
-#include "daemon_store.h"
int main(int argc, char** argv) {
// Our testrunners are invoked with platform_test, which doesn't
@@ -22,26 +19,9 @@
setenv("ANDROID_LOG_TAGS", "*:f", 0);
#endif
- // For compilation caching, we expect that the directory
- // /run/daemon-store/ml_service will exist, but in tests it will
- // not, and DBus won't be available to query the current username
- // hash. So we set up a fake cryptohome area here and mock the
- // username hasher to simulate that environment for testing.
- DaemonStore& ds = DaemonStore::Get();
- ds.SetUserHasherForTesting(std::make_unique<MockUserHashForTesting>());
- std::string base("/tmp/run/daemon-store/ml_service");
- ds.SetDaemonStoreBaseForTesting(base);
- std::string cacheDir = base + "/mock-user-hash/nnapi_compilations";
- std::filesystem::create_directories(cacheDir);
-
testing::InitGoogleTest(&argc, argv);
android::base::InitLogging(argv, android::base::StderrLogger);
testing::GTEST_FLAG(throw_on_failure) = true;
testing::InitGoogleMock(&argc, argv);
-
- int test_result = RUN_ALL_TESTS();
-
- // Clean up the fake cryptohome area
- std::filesystem::remove_all(base);
- return test_result;
+ return RUN_ALL_TESTS();
}
diff --git a/nn/chromeos/tests/BUILD.gn b/nn/chromeos/tests/BUILD.gn
index 0c12eb1..cf01d7a 100644
--- a/nn/chromeos/tests/BUILD.gn
+++ b/nn/chromeos/tests/BUILD.gn
@@ -34,7 +34,6 @@
"${vts_src}/1.0/vts/functional/include",
"${nn_src}/common",
"${nn_src}/common/include",
- "${nn_src}/runtime",
"${nn_src}/runtime/include",
"${nn_src}/tools/test_generator/test_harness/include",
"${sysroot}/usr/include/aosp",
@@ -62,8 +61,6 @@
sources = [
"${vts_src}/1.0/vts/functional/Callbacks.cpp",
"${vts_src}/1.0/vts/functional/Utils.cpp",
- "${nn_src}/chromeos/compilation_caching.cpp",
- "${nn_src}/chromeos/daemon_store.cpp",
"${nn_src}/chromeos/testrunner.cpp",
"${nn_src}/chromeos/versioned_drivers.cpp",
"${nn_src}/common/ExecutionBurstController.cpp",
diff --git a/nn/chromeos/tests/common/daemon_store_test.cc b/nn/chromeos/tests/common/daemon_store_test.cc
deleted file mode 100644
index 0c398e4..0000000
--- a/nn/chromeos/tests/common/daemon_store_test.cc
+++ /dev/null
@@ -1,57 +0,0 @@
-// Copyright 2021 The Chromium OS Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-#include "daemon_store.h"
-
-#include <android-base/logging.h>
-#include <gtest/gtest.h>
-
-namespace {
-class DaemonStoreTest : public ::testing::Test {
- protected:
- std::string original_base_path_;
-
- virtual void SetUp() {
- original_base_path_ = DaemonStore::Get().GetDaemonStoreBaseForTesting();
- }
- virtual void TearDown() {
- DaemonStore::Get().SetUserHasherForTesting(
- std::make_unique<MockUserHashForTesting>());
- DaemonStore::Get().SetDaemonStoreBaseForTesting(original_base_path_);
- }
-};
-
-class EmptyUserHash : public UserHashInterface {
- public:
- virtual std::string GetCurrentUserHash() const override { return ""; }
-};
-} // namespace
-
-TEST_F(DaemonStoreTest, TestEmptyUserHash) {
- auto& ds = DaemonStore::Get();
- ds.SetUserHasherForTesting(std::make_unique<EmptyUserHash>());
-
- EXPECT_EQ("", ds.GetDaemonStoreDir(""));
- EXPECT_EQ("", ds.GetDaemonStoreDir("/some_test"));
-}
-
-TEST_F(DaemonStoreTest, TestNonExistentDir) {
- auto& ds = DaemonStore::Get();
- ds.SetDaemonStoreBaseForTesting("/does/not/exist");
-
- EXPECT_EQ("", ds.GetDaemonStoreDir(""));
- EXPECT_EQ("", ds.GetDaemonStoreDir("/some_test"));
-}
-
-TEST_F(DaemonStoreTest, TestGetUserHash) {
- auto& ds = DaemonStore::Get();
- std::string cache_dir(
- "/tmp/run/daemon-store/ml_service/mock-user-hash/nnapi_compilations");
-
- EXPECT_EQ(cache_dir + "/some_test", ds.GetDaemonStoreDir("some_test"));
- EXPECT_EQ(cache_dir + "/some_test", ds.GetDaemonStoreDir("/some_test"));
- EXPECT_EQ(cache_dir + "/some_test", ds.GetDaemonStoreDir("some_test/"));
- EXPECT_EQ(cache_dir + "/some/test", ds.GetDaemonStoreDir("some/test"));
- EXPECT_EQ(cache_dir + "/some/test", ds.GetDaemonStoreDir("/some/test"));
- EXPECT_EQ(cache_dir, ds.GetDaemonStoreDir(""));
-}
diff --git a/nn/runtime/NeuralNetworks.cpp b/nn/runtime/NeuralNetworks.cpp
index ff465e3..5d3dae4 100644
--- a/nn/runtime/NeuralNetworks.cpp
+++ b/nn/runtime/NeuralNetworks.cpp
@@ -1189,9 +1189,6 @@
return c->setPreference(preference);
}
-// A custom implementation that uses ChromeOS deamon storage is in
-// ml/nn/chromeos/compilation_caching.cpp
-#if !defined(NNAPI_CHROMEOS)
int ANeuralNetworksCompilation_setCaching(ANeuralNetworksCompilation* compilation,
const char* cacheDir, const uint8_t* token) {
NNTRACE_RT(NNTRACE_PHASE_COMPILATION, "ANeuralNetworksCompilation_setCaching");
@@ -1202,7 +1199,6 @@
CompilationBuilder* c = reinterpret_cast<CompilationBuilder*>(compilation);
return c->setCaching(cacheDir, token);
}
-#endif
int ANeuralNetworksCompilation_finish(ANeuralNetworksCompilation* compilation) {
NNTRACE_RT(NNTRACE_PHASE_COMPILATION, "ANeuralNetworksCompilation_finish");