diff options
48 files changed, 1996 insertions, 694 deletions
diff --git a/cmds/installd/InstalldNativeService.cpp b/cmds/installd/InstalldNativeService.cpp index c55fc6af83..a176df9789 100644 --- a/cmds/installd/InstalldNativeService.cpp +++ b/cmds/installd/InstalldNativeService.cpp @@ -2354,7 +2354,7 @@ binder::Status InstalldNativeService::copySystemProfile(const std::string& syste // TODO: Consider returning error codes. binder::Status InstalldNativeService::mergeProfiles(int32_t uid, const std::string& packageName, - const std::string& profileName, bool* _aidl_return) { + const std::string& profileName, int* _aidl_return) { ENFORCE_UID(AID_SYSTEM); CHECK_ARGUMENT_PACKAGE_NAME(packageName); std::lock_guard<std::recursive_mutex> lock(mLock); @@ -2654,7 +2654,8 @@ binder::Status InstalldNativeService::moveAb(const std::string& apkPath, } binder::Status InstalldNativeService::deleteOdex(const std::string& apkPath, - const std::string& instructionSet, const std::optional<std::string>& outputPath) { + const std::string& instructionSet, const std::optional<std::string>& outputPath, + int64_t* _aidl_return) { ENFORCE_UID(AID_SYSTEM); CHECK_ARGUMENT_PATH(apkPath); CHECK_ARGUMENT_PATH(outputPath); @@ -2664,8 +2665,8 @@ binder::Status InstalldNativeService::deleteOdex(const std::string& apkPath, const char* instruction_set = instructionSet.c_str(); const char* oat_dir = outputPath ? outputPath->c_str() : nullptr; - bool res = delete_odex(apk_path, instruction_set, oat_dir); - return res ? ok() : error(); + *_aidl_return = delete_odex(apk_path, instruction_set, oat_dir); + return *_aidl_return == -1 ? error() : ok(); } // This kernel feature is experimental. diff --git a/cmds/installd/InstalldNativeService.h b/cmds/installd/InstalldNativeService.h index 9819327840..3127be6fd5 100644 --- a/cmds/installd/InstalldNativeService.h +++ b/cmds/installd/InstalldNativeService.h @@ -122,7 +122,7 @@ public: binder::Status rmdex(const std::string& codePath, const std::string& instructionSet); binder::Status mergeProfiles(int32_t uid, const std::string& packageName, - const std::string& profileName, bool* _aidl_return); + const std::string& profileName, int* _aidl_return); binder::Status dumpProfiles(int32_t uid, const std::string& packageName, const std::string& profileName, const std::string& codePath, bool* _aidl_return); binder::Status copySystemProfile(const std::string& systemProfile, @@ -147,7 +147,7 @@ public: binder::Status moveAb(const std::string& apkPath, const std::string& instructionSet, const std::string& outputPath); binder::Status deleteOdex(const std::string& apkPath, const std::string& instructionSet, - const std::optional<std::string>& outputPath); + const std::optional<std::string>& outputPath, int64_t* _aidl_return); binder::Status installApkVerity(const std::string& filePath, android::base::unique_fd verityInput, int32_t contentSize); binder::Status assertFsverityRootHashMatches(const std::string& filePath, diff --git a/cmds/installd/binder/android/os/IInstalld.aidl b/cmds/installd/binder/android/os/IInstalld.aidl index 4ac70a4857..816e50826d 100644 --- a/cmds/installd/binder/android/os/IInstalld.aidl +++ b/cmds/installd/binder/android/os/IInstalld.aidl @@ -71,7 +71,7 @@ interface IInstalld { void rmdex(@utf8InCpp String codePath, @utf8InCpp String instructionSet); - boolean mergeProfiles(int uid, @utf8InCpp String packageName, @utf8InCpp String profileName); + int mergeProfiles(int uid, @utf8InCpp String packageName, @utf8InCpp String profileName); boolean dumpProfiles(int uid, @utf8InCpp String packageName, @utf8InCpp String profileName, @utf8InCpp String codePath); boolean copySystemProfile(@utf8InCpp String systemProfile, int uid, @@ -93,7 +93,7 @@ interface IInstalld { @utf8InCpp String toBase); void moveAb(@utf8InCpp String apkPath, @utf8InCpp String instructionSet, @utf8InCpp String outputPath); - void deleteOdex(@utf8InCpp String apkPath, @utf8InCpp String instructionSet, + long deleteOdex(@utf8InCpp String apkPath, @utf8InCpp String instructionSet, @nullable @utf8InCpp String outputPath); void installApkVerity(@utf8InCpp String filePath, in FileDescriptor verityInput, int contentSize); diff --git a/cmds/installd/dexopt.cpp b/cmds/installd/dexopt.cpp index 204953cd07..15f0c5b75c 100644 --- a/cmds/installd/dexopt.cpp +++ b/cmds/installd/dexopt.cpp @@ -53,6 +53,7 @@ #include "execv_helper.h" #include "globals.h" #include "installd_deps.h" +#include "installd_constants.h" #include "otapreopt_utils.h" #include "run_dex2oat.h" #include "unique_file.h" @@ -292,8 +293,8 @@ static void SetDex2OatScheduling(bool set_to_bg) { } } -static unique_fd create_profile(uid_t uid, const std::string& profile, int32_t flags) { - unique_fd fd(TEMP_FAILURE_RETRY(open(profile.c_str(), flags, 0600))); +static unique_fd create_profile(uid_t uid, const std::string& profile, int32_t flags, mode_t mode) { + unique_fd fd(TEMP_FAILURE_RETRY(open(profile.c_str(), flags, mode))); if (fd.get() < 0) { if (errno != EEXIST) { PLOG(ERROR) << "Failed to create profile " << profile; @@ -310,7 +311,7 @@ static unique_fd create_profile(uid_t uid, const std::string& profile, int32_t f return fd; } -static unique_fd open_profile(uid_t uid, const std::string& profile, int32_t flags) { +static unique_fd open_profile(uid_t uid, const std::string& profile, int32_t flags, mode_t mode) { // Do not follow symlinks when opening a profile: // - primary profiles should not contain symlinks in their paths // - secondary dex paths should have been already resolved and validated @@ -320,7 +321,7 @@ static unique_fd open_profile(uid_t uid, const std::string& profile, int32_t fla // Reference profiles and snapshots are created on the fly; so they might not exist beforehand. unique_fd fd; if ((flags & O_CREAT) != 0) { - fd = create_profile(uid, profile, flags); + fd = create_profile(uid, profile, flags, mode); } else { fd.reset(TEMP_FAILURE_RETRY(open(profile.c_str(), flags))); } @@ -336,6 +337,16 @@ static unique_fd open_profile(uid_t uid, const std::string& profile, int32_t fla PLOG(ERROR) << "Failed to open profile " << profile; } return invalid_unique_fd(); + } else { + // If we just create the file we need to set its mode because on Android + // open has a mask that only allows owner access. + if ((flags & O_CREAT) != 0) { + if (fchmod(fd.get(), mode) != 0) { + PLOG(ERROR) << "Could not set mode " << std::hex << mode << std::dec + << " on profile" << profile; + // Not a terminal failure. + } + } } return fd; @@ -345,20 +356,29 @@ static unique_fd open_current_profile(uid_t uid, userid_t user, const std::strin const std::string& location, bool is_secondary_dex) { std::string profile = create_current_profile_path(user, package_name, location, is_secondary_dex); - return open_profile(uid, profile, O_RDONLY); + return open_profile(uid, profile, O_RDONLY, /*mode=*/ 0); } static unique_fd open_reference_profile(uid_t uid, const std::string& package_name, const std::string& location, bool read_write, bool is_secondary_dex) { std::string profile = create_reference_profile_path(package_name, location, is_secondary_dex); - return open_profile(uid, profile, read_write ? (O_CREAT | O_RDWR) : O_RDONLY); + return open_profile( + uid, + profile, + read_write ? (O_CREAT | O_RDWR) : O_RDONLY, + S_IRUSR | S_IWUSR | S_IRGRP); // so that ART can also read it when apps run. } static UniqueFile open_reference_profile_as_unique_file(uid_t uid, const std::string& package_name, const std::string& location, bool read_write, bool is_secondary_dex) { std::string profile_path = create_reference_profile_path(package_name, location, is_secondary_dex); - unique_fd ufd = open_profile(uid, profile_path, read_write ? (O_CREAT | O_RDWR) : O_RDONLY); + unique_fd ufd = open_profile( + uid, + profile_path, + read_write ? (O_CREAT | O_RDWR) : O_RDONLY, + S_IRUSR | S_IWUSR | S_IRGRP); // so that ART can also read it when apps run. + return UniqueFile(ufd.release(), profile_path, [](const std::string& path) { clear_profile(path); }); @@ -367,7 +387,7 @@ static UniqueFile open_reference_profile_as_unique_file(uid_t uid, const std::st static unique_fd open_spnashot_profile(uid_t uid, const std::string& package_name, const std::string& location) { std::string profile = create_snapshot_profile_path(package_name, location); - return open_profile(uid, profile, O_CREAT | O_RDWR | O_TRUNC); + return open_profile(uid, profile, O_CREAT | O_RDWR | O_TRUNC, S_IRUSR | S_IWUSR); } static void open_profile_files(uid_t uid, const std::string& package_name, @@ -397,11 +417,12 @@ static void open_profile_files(uid_t uid, const std::string& package_name, static constexpr int PROFMAN_BIN_RETURN_CODE_SUCCESS = 0; static constexpr int PROFMAN_BIN_RETURN_CODE_COMPILE = 1; -static constexpr int PROFMAN_BIN_RETURN_CODE_SKIP_COMPILATION = 2; +static constexpr int PROFMAN_BIN_RETURN_CODE_SKIP_COMPILATION_NOT_ENOUGH_DELTA = 2; static constexpr int PROFMAN_BIN_RETURN_CODE_BAD_PROFILES = 3; static constexpr int PROFMAN_BIN_RETURN_CODE_ERROR_IO = 4; static constexpr int PROFMAN_BIN_RETURN_CODE_ERROR_LOCKING = 5; static constexpr int PROFMAN_BIN_RETURN_CODE_ERROR_DIFFERENT_VERSIONS = 6; +static constexpr int PROFMAN_BIN_RETURN_CODE_SKIP_COMPILATION_EMPTY_PROFILES = 7; class RunProfman : public ExecVHelper { public: @@ -536,15 +557,7 @@ class RunProfman : public ExecVHelper { std::vector<unique_fd> apk_fds_; }; - - -// Decides if profile guided compilation is needed or not based on existing profiles. -// The location is the package name for primary apks or the dex path for secondary dex files. -// Returns true if there is enough information in the current profiles that makes it -// worth to recompile the given location. -// If the return value is true all the current profiles would have been merged into -// the reference profiles accessible with open_reference_profile(). -static bool analyze_profiles(uid_t uid, const std::string& package_name, +static int analyze_profiles(uid_t uid, const std::string& package_name, const std::string& location, bool is_secondary_dex) { std::vector<unique_fd> profiles_fd; unique_fd reference_profile_fd; @@ -553,7 +566,7 @@ static bool analyze_profiles(uid_t uid, const std::string& package_name, if (profiles_fd.empty() || (reference_profile_fd.get() < 0)) { // Skip profile guided compilation because no profiles were found. // Or if the reference profile info couldn't be opened. - return false; + return PROFILES_ANALYSIS_DONT_OPTIMIZE_EMPTY_PROFILES; } RunProfman profman_merge; @@ -575,6 +588,7 @@ static bool analyze_profiles(uid_t uid, const std::string& package_name, /* parent */ int return_code = wait_child(pid); bool need_to_compile = false; + bool empty_profiles = false; bool should_clear_current_profiles = false; bool should_clear_reference_profile = false; if (!WIFEXITED(return_code)) { @@ -587,11 +601,17 @@ static bool analyze_profiles(uid_t uid, const std::string& package_name, should_clear_current_profiles = true; should_clear_reference_profile = false; break; - case PROFMAN_BIN_RETURN_CODE_SKIP_COMPILATION: + case PROFMAN_BIN_RETURN_CODE_SKIP_COMPILATION_NOT_ENOUGH_DELTA: need_to_compile = false; should_clear_current_profiles = false; should_clear_reference_profile = false; break; + case PROFMAN_BIN_RETURN_CODE_SKIP_COMPILATION_EMPTY_PROFILES: + need_to_compile = false; + empty_profiles = true; + should_clear_current_profiles = false; + should_clear_reference_profile = false; + break; case PROFMAN_BIN_RETURN_CODE_BAD_PROFILES: LOG(WARNING) << "Bad profiles for location " << location; need_to_compile = false; @@ -634,16 +654,29 @@ static bool analyze_profiles(uid_t uid, const std::string& package_name, if (should_clear_reference_profile) { clear_reference_profile(package_name, location, is_secondary_dex); } - return need_to_compile; + int result = 0; + if (need_to_compile) { + result = PROFILES_ANALYSIS_OPTIMIZE; + } else if (empty_profiles) { + result = PROFILES_ANALYSIS_DONT_OPTIMIZE_EMPTY_PROFILES; + } else { + result = PROFILES_ANALYSIS_DONT_OPTIMIZE_SMALL_DELTA; + } + return result; } // Decides if profile guided compilation is needed or not based on existing profiles. -// The analysis is done for the primary apks of the given package. -// Returns true if there is enough information in the current profiles that makes it -// worth to recompile the package. -// If the return value is true all the current profiles would have been merged into -// the reference profiles accessible with open_reference_profile(). -bool analyze_primary_profiles(uid_t uid, const std::string& package_name, +// The analysis is done for a single profile name (which corresponds to a single code path). +// +// Returns PROFILES_ANALYSIS_OPTIMIZE if there is enough information in the current profiles +// that makes it worth to recompile the package. +// If the return value is PROFILES_ANALYSIS_OPTIMIZE all the current profiles would have been +// merged into the reference profiles accessible with open_reference_profile(). +// +// Return PROFILES_ANALYSIS_DONT_OPTIMIZE_SMALL_DELTA if the package should not optimize. +// As a special case returns PROFILES_ANALYSIS_DONT_OPTIMIZE_EMPTY_PROFILES if all profiles are +// empty. +int analyze_primary_profiles(uid_t uid, const std::string& package_name, const std::string& profile_name) { return analyze_profiles(uid, package_name, profile_name, /*is_secondary_dex*/false); } @@ -1147,7 +1180,7 @@ class RunDexoptAnalyzer : public ExecVHelper { int zip_fd, const std::string& instruction_set, const std::string& compiler_filter, - bool profile_was_updated, + int profile_analysis_result, bool downgrade, const char* class_loader_context, const std::string& class_loader_context_fds) { @@ -1163,7 +1196,8 @@ class RunDexoptAnalyzer : public ExecVHelper { std::string zip_fd_arg = "--zip-fd=" + std::to_string(zip_fd); std::string isa_arg = "--isa=" + instruction_set; std::string compiler_filter_arg = "--compiler-filter=" + compiler_filter; - const char* assume_profile_changed = "--assume-profile-changed"; + std::string profile_analysis_arg = "--profile-analysis-result=" + + std::to_string(profile_analysis_result); const char* downgrade_flag = "--downgrade"; std::string class_loader_context_arg = "--class-loader-context="; if (class_loader_context != nullptr) { @@ -1185,9 +1219,8 @@ class RunDexoptAnalyzer : public ExecVHelper { AddArg(vdex_fd_arg); } AddArg(zip_fd_arg); - if (profile_was_updated) { - AddArg(assume_profile_changed); - } + AddArg(profile_analysis_arg); + if (downgrade) { AddArg(downgrade_flag); } @@ -1559,7 +1592,7 @@ static bool process_secondary_dex_dexopt(const std::string& dex_path, const char } // Analyze profiles. - bool profile_was_updated = analyze_profiles(uid, pkgname, dex_path, + int profile_analysis_result = analyze_profiles(uid, pkgname, dex_path, /*is_secondary_dex*/true); // Run dexoptanalyzer to get dexopt_needed code. This is not expected to return. @@ -1570,7 +1603,8 @@ static bool process_secondary_dex_dexopt(const std::string& dex_path, const char oat_file_fd.get(), zip_fd.get(), instruction_set, - compiler_filter, profile_was_updated, + compiler_filter, + profile_analysis_result, downgrade, class_loader_context, join_fds(context_zip_fds)); @@ -2218,38 +2252,52 @@ bool move_ab(const char* apk_path, const char* instruction_set, const char* oat_ return success; } -bool delete_odex(const char* apk_path, const char* instruction_set, const char* oat_dir) { +int64_t delete_odex(const char* apk_path, const char* instruction_set, const char* oat_dir) { // Delete the oat/odex file. char out_path[PKG_PATH_MAX]; if (!create_oat_out_path(apk_path, instruction_set, oat_dir, /*is_secondary_dex*/false, out_path)) { - return false; + LOG(ERROR) << "Cannot create apk path for " << apk_path; + return -1; } // In case of a permission failure report the issue. Otherwise just print a warning. - auto unlink_and_check = [](const char* path) -> bool { - int result = unlink(path); - if (result != 0) { - if (errno == EACCES || errno == EPERM) { + auto unlink_and_check = [](const char* path) -> int64_t { + struct stat file_stat; + if (stat(path, &file_stat) != 0) { + if (errno != ENOENT) { + PLOG(ERROR) << "Could not stat " << path; + return -1; + } + return 0; + } + + if (unlink(path) != 0) { + if (errno != ENOENT) { PLOG(ERROR) << "Could not unlink " << path; - return false; + return -1; } - PLOG(WARNING) << "Could not unlink " << path; } - return true; + return static_cast<int64_t>(file_stat.st_size); }; // Delete the oat/odex file. - bool return_value_oat = unlink_and_check(out_path); + int64_t return_value_oat = unlink_and_check(out_path); // Derive and delete the app image. - bool return_value_art = unlink_and_check(create_image_filename(out_path).c_str()); + int64_t return_value_art = unlink_and_check(create_image_filename(out_path).c_str()); // Derive and delete the vdex file. - bool return_value_vdex = unlink_and_check(create_vdex_filename(out_path).c_str()); + int64_t return_value_vdex = unlink_and_check(create_vdex_filename(out_path).c_str()); + + // Report result + if (return_value_oat == -1 + || return_value_art == -1 + || return_value_vdex == -1) { + return -1; + } - // Report success. - return return_value_oat && return_value_art && return_value_vdex; + return return_value_oat + return_value_art + return_value_vdex; } static bool is_absolute_path(const std::string& path) { @@ -2484,7 +2532,7 @@ static bool create_boot_image_profile_snapshot(const std::string& package_name, for (size_t i = 0; i < profiles.size(); ) { std::vector<unique_fd> profiles_fd; for (size_t k = 0; k < kAggregationBatchSize && i < profiles.size(); k++, i++) { - unique_fd fd = open_profile(AID_SYSTEM, profiles[i], O_RDONLY); + unique_fd fd = open_profile(AID_SYSTEM, profiles[i], O_RDONLY, /*mode=*/ 0); if (fd.get() >= 0) { profiles_fd.push_back(std::move(fd)); } diff --git a/cmds/installd/dexopt.h b/cmds/installd/dexopt.h index d35953c058..5a637b1ce7 100644 --- a/cmds/installd/dexopt.h +++ b/cmds/installd/dexopt.h @@ -54,15 +54,20 @@ bool clear_primary_current_profile(const std::string& pkgname, const std::string // Clear all current profiles identified by the given profile name (all users). bool clear_primary_current_profiles(const std::string& pkgname, const std::string& profile_name); -// Decide if profile guided compilation is needed or not based on existing profiles. +// Decides if profile guided compilation is needed or not based on existing profiles. // The analysis is done for a single profile name (which corresponds to a single code path). -// Returns true if there is enough information in the current profiles that makes it -// worth to recompile the package. -// If the return value is true all the current profiles would have been merged into -// the reference profiles accessible with open_reference_profile(). -bool analyze_primary_profiles(uid_t uid, - const std::string& pkgname, - const std::string& profile_name); +// +// Returns PROFILES_ANALYSIS_OPTIMIZE if there is enough information in the current profiles +// that makes it worth to recompile the package. +// If the return value is PROFILES_ANALYSIS_OPTIMIZE all the current profiles would have been +// merged into the reference profiles accessible with open_reference_profile(). +// +// Return PROFILES_ANALYSIS_DONT_OPTIMIZE_SMALL_DELTA if the package should not optimize. +// As a special case returns PROFILES_ANALYSIS_DONT_OPTIMIZE_EMPTY_PROFILES if all profiles are +// empty. +int analyze_primary_profiles(uid_t uid, + const std::string& pkgname, + const std::string& profile_name); // Create a snapshot of the profile information for the given package profile. // If appId is -1, the method creates the profile snapshot for the boot image. @@ -104,7 +109,8 @@ bool prepare_app_profile(const std::string& package_name, const std::string& code_path, const std::optional<std::string>& dex_metadata); -bool delete_odex(const char* apk_path, const char* instruction_set, const char* output_path); +// Returns the total bytes that were freed, or -1 in case of errors. +int64_t delete_odex(const char* apk_path, const char* instruction_set, const char* output_path); bool reconcile_secondary_dex_file(const std::string& dex_path, const std::string& pkgname, int uid, const std::vector<std::string>& isas, diff --git a/cmds/installd/installd_constants.h b/cmds/installd/installd_constants.h index b5ee481645..00d8441607 100644 --- a/cmds/installd/installd_constants.h +++ b/cmds/installd/installd_constants.h @@ -77,6 +77,12 @@ constexpr int DEXOPT_MASK = constexpr int FLAG_STORAGE_DE = 1 << 0; constexpr int FLAG_STORAGE_CE = 1 << 1; +// TODO: import them from dexoptanalyzer.h +// NOTE: keep in sync with Installer.java +constexpr int PROFILES_ANALYSIS_OPTIMIZE = 1; +constexpr int PROFILES_ANALYSIS_DONT_OPTIMIZE_SMALL_DELTA = 2; +constexpr int PROFILES_ANALYSIS_DONT_OPTIMIZE_EMPTY_PROFILES = 3; + #define ARRAY_SIZE(a) (sizeof(a) / sizeof(*(a))) } // namespace installd diff --git a/cmds/installd/run_dex2oat.cpp b/cmds/installd/run_dex2oat.cpp index a27fd103a4..e847626a14 100644 --- a/cmds/installd/run_dex2oat.cpp +++ b/cmds/installd/run_dex2oat.cpp @@ -324,6 +324,12 @@ void RunDex2Oat::PrepareCompilerRuntimeAndPerfConfigFlags(bool post_bootcomplete AddRuntimeArg(MapPropertyToArg("dalvik.vm.dex2oat-Xms", "-Xms%s")); AddRuntimeArg(MapPropertyToArg("dalvik.vm.dex2oat-Xmx", "-Xmx%s")); + + // Enable compiling dex files in isolation on low ram devices. + // It takes longer but reduces the memory footprint. + if (GetBoolProperty("ro.config.low_ram", false)) { + AddArg("--compile-individually"); + } } void RunDex2Oat::Exec(int exit_code) { diff --git a/cmds/installd/tests/installd_dexopt_test.cpp b/cmds/installd/tests/installd_dexopt_test.cpp index e27202597c..7e7e513568 100644 --- a/cmds/installd/tests/installd_dexopt_test.cpp +++ b/cmds/installd/tests/installd_dexopt_test.cpp @@ -38,6 +38,7 @@ #include "binder_test_utils.h" #include "dexopt.h" #include "InstalldNativeService.h" +#include "installd_constants.h" #include "globals.h" #include "tests/test_utils.h" #include "utils.h" @@ -517,7 +518,8 @@ protected: // Check the access to the compiler output. // - speed-profile artifacts are not world-wide readable. // - files are owned by the system uid. - std::string odex = GetPrimaryDexArtifact(oat_dir, apk_path_, "odex"); + std::string odex = GetPrimaryDexArtifact(oat_dir, apk_path_, + oat_dir == nullptr ? "dex" : "odex"); std::string vdex = GetPrimaryDexArtifact(oat_dir, apk_path_, "vdex"); std::string art = GetPrimaryDexArtifact(oat_dir, apk_path_, "art"); @@ -545,7 +547,7 @@ protected: } } return android_data_dir + DALVIK_CACHE + '/' + kRuntimeIsa + "/" + path - + "@classes.dex"; + + "@classes." + type; } else { std::string::size_type name_end = dex_path.rfind('.'); std::string::size_type name_start = dex_path.rfind('/'); @@ -553,6 +555,53 @@ protected: dex_path.substr(name_start + 1, name_end - name_start) + type; } } + + int64_t GetSize(const std::string& path) { + struct stat file_stat; + if (stat(path.c_str(), &file_stat) == 0) { + return static_cast<int64_t>(file_stat.st_size); + } + PLOG(ERROR) << "Cannot stat path: " << path; + return -1; + } + + void TestDeleteOdex(bool in_dalvik_cache) { + const char* oat_dir = in_dalvik_cache ? nullptr : app_oat_dir_.c_str(); + CompilePrimaryDexOk( + "speed-profile", + DEXOPT_BOOTCOMPLETE | DEXOPT_PROFILE_GUIDED | DEXOPT_PUBLIC + | DEXOPT_GENERATE_APP_IMAGE, + oat_dir, + kTestAppGid, + DEX2OAT_FROM_SCRATCH, + /*binder_result=*/nullptr, + empty_dm_file_.c_str()); + + + int64_t odex_size = GetSize(GetPrimaryDexArtifact(oat_dir, apk_path_, + in_dalvik_cache ? "dex" : "odex")); + int64_t vdex_size = GetSize(GetPrimaryDexArtifact(oat_dir, apk_path_, "vdex")); + int64_t art_size = GetSize(GetPrimaryDexArtifact(oat_dir, apk_path_, "art")); + + LOG(ERROR) << "test odex " << odex_size; + LOG(ERROR) << "test vdex_size " << vdex_size; + LOG(ERROR) << "test art_size " << art_size; + int64_t expected_bytes_freed = odex_size + vdex_size + art_size; + + int64_t bytes_freed; + binder::Status result = service_->deleteOdex( + apk_path_, + kRuntimeIsa, + in_dalvik_cache ? std::nullopt : std::make_optional<std::string>(app_oat_dir_.c_str()), + &bytes_freed); + ASSERT_TRUE(result.isOk()) << result.toString8().c_str(); + + ASSERT_GE(odex_size, 0); + ASSERT_GE(vdex_size, 0); + ASSERT_GE(art_size, 0); + + ASSERT_EQ(expected_bytes_freed, bytes_freed); + } }; @@ -701,6 +750,16 @@ TEST_F(DexoptTest, DexoptPrimaryBackgroundOk) { empty_dm_file_.c_str()); } +TEST_F(DexoptTest, DeleteDexoptArtifactsData) { + LOG(INFO) << "DeleteDexoptArtifactsData"; + TestDeleteOdex(/*in_dalvik_cache=*/ false); +} + +TEST_F(DexoptTest, DeleteDexoptArtifactsDalvikCache) { + LOG(INFO) << "DeleteDexoptArtifactsDalvikCache"; + TestDeleteOdex(/*in_dalvik_cache=*/ true); +} + TEST_F(DexoptTest, ResolveStartupConstStrings) { LOG(INFO) << "DexoptDex2oatResolveStartupStrings"; const std::string property = "persist.device_config.runtime.dex2oat_resolve_startup_strings"; @@ -919,7 +978,7 @@ class ProfileTest : public DexoptTest { return; } - // Check that the snapshot was created witht he expected acess flags. + // Check that the snapshot was created with the expected access flags. CheckFileAccess(snap_profile_, kSystemUid, kSystemGid, 0600 | S_IFREG); // The snapshot should be equivalent to the merge of profiles. @@ -951,19 +1010,19 @@ class ProfileTest : public DexoptTest { void mergePackageProfiles(const std::string& package_name, const std::string& code_path, - bool expected_result) { - bool result; + int expected_result) { + int result; ASSERT_BINDER_SUCCESS(service_->mergeProfiles( kTestAppUid, package_name, code_path, &result)); ASSERT_EQ(expected_result, result); - if (!expected_result) { - // Do not check the files if we expect to fail. + // There's nothing to check if the files are empty. + if (result == PROFILES_ANALYSIS_DONT_OPTIMIZE_EMPTY_PROFILES) { return; } - // Check that the snapshot was created witht he expected acess flags. - CheckFileAccess(ref_profile_, kTestAppUid, kTestAppUid, 0600 | S_IFREG); + // Check that the snapshot was created with the expected access flags. + CheckFileAccess(ref_profile_, kTestAppUid, kTestAppUid, 0640 | S_IFREG); // The snapshot should be equivalent to the merge of profiles. std::string ref_profile_content = ref_profile_ + ".expected"; @@ -1077,7 +1136,7 @@ TEST_F(ProfileTest, ProfileMergeOk) { LOG(INFO) << "ProfileMergeOk"; SetupProfiles(/*setup_ref*/ true); - mergePackageProfiles(package_name_, "primary.prof", /*expected_result*/ true); + mergePackageProfiles(package_name_, "primary.prof", PROFILES_ANALYSIS_OPTIMIZE); } // The reference profile is created on the fly. We need to be able to @@ -1086,14 +1145,15 @@ TEST_F(ProfileTest, ProfileMergeOkNoReference) { LOG(INFO) << "ProfileMergeOkNoReference"; SetupProfiles(/*setup_ref*/ false); - mergePackageProfiles(package_name_, "primary.prof", /*expected_result*/ true); + mergePackageProfiles(package_name_, "primary.prof", PROFILES_ANALYSIS_OPTIMIZE); } TEST_F(ProfileTest, ProfileMergeFailWrongPackage) { LOG(INFO) << "ProfileMergeFailWrongPackage"; SetupProfiles(/*setup_ref*/ true); - mergePackageProfiles("not.there", "primary.prof", /*expected_result*/ false); + mergePackageProfiles("not.there", "primary.prof", + PROFILES_ANALYSIS_DONT_OPTIMIZE_EMPTY_PROFILES); } TEST_F(ProfileTest, ProfileDirOk) { diff --git a/libs/binder/Android.bp b/libs/binder/Android.bp index b9c8d2028b..d8f8e558a2 100644 --- a/libs/binder/Android.bp +++ b/libs/binder/Android.bp @@ -146,6 +146,14 @@ cc_library { darwin: { enabled: false, }, + host: { + static_libs: [ + "libbase", + ], + srcs: [ + "UtilsHost.cpp", + ], + }, }, aidl: { @@ -257,6 +265,32 @@ aidl_interface { }, } +// TODO(b/184872979): remove once the Rust API is created. +cc_library { + name: "libbinder_rpc_unstable", + srcs: ["libbinder_rpc_unstable.cpp"], + defaults: ["libbinder_ndk_host_user"], + shared_libs: [ + "libbase", + "libbinder", + "libbinder_ndk", + "libutils", + ], + + // enumerate stable entry points, for apex use + stubs: { + symbol_file: "libbinder_rpc_unstable.map.txt", + }, + + // This library is intentionally limited to these targets, and it will be removed later. + // Do not expand the visibility. + visibility: [ + "//packages/modules/Virtualization/authfs:__subpackages__", + "//packages/modules/Virtualization/compos", + "//packages/modules/Virtualization/microdroid", + ], +} + // libbinder historically contained additional interfaces that provided specific // functionality in the platform but have nothing to do with binder itself. These // are moved out of libbinder in order to avoid the overhead of their vtables. diff --git a/libs/binder/Binder.cpp b/libs/binder/Binder.cpp index 415b44e683..02321cdf97 100644 --- a/libs/binder/Binder.cpp +++ b/libs/binder/Binder.cpp @@ -17,6 +17,7 @@ #include <binder/Binder.h> #include <atomic> +#include <set> #include <android-base/unique_fd.h> #include <binder/BpBinder.h> @@ -150,7 +151,8 @@ status_t IBinder::getDebugPid(pid_t* out) { return OK; } -status_t IBinder::setRpcClientDebug(android::base::unique_fd socketFd, uint32_t maxRpcThreads) { +status_t IBinder::setRpcClientDebug(android::base::unique_fd socketFd, + const sp<IBinder>& keepAliveBinder) { if constexpr (!kEnableRpcDevServers) { ALOGW("setRpcClientDebug disallowed because RPC is not enabled"); return INVALID_OPERATION; @@ -158,7 +160,7 @@ status_t IBinder::setRpcClientDebug(android::base::unique_fd socketFd, uint32_t BBinder* local = this->localBinder(); if (local != nullptr) { - return local->BBinder::setRpcClientDebug(std::move(socketFd), maxRpcThreads); + return local->BBinder::setRpcClientDebug(std::move(socketFd), keepAliveBinder); } BpBinder* proxy = this->remoteBinder(); @@ -173,12 +175,44 @@ status_t IBinder::setRpcClientDebug(android::base::unique_fd socketFd, uint32_t status = data.writeFileDescriptor(socketFd.release(), true /* own */); if (status != OK) return status; } - if (status = data.writeUint32(maxRpcThreads); status != OK) return status; + if (status = data.writeStrongBinder(keepAliveBinder); status != OK) return status; return transact(SET_RPC_CLIENT_TRANSACTION, data, &reply); } // --------------------------------------------------------------------------- +class BBinder::RpcServerLink : public IBinder::DeathRecipient { +public: + // On binder died, calls RpcServer::shutdown on @a rpcServer, and removes itself from @a binder. + RpcServerLink(const sp<RpcServer>& rpcServer, const sp<IBinder>& keepAliveBinder, + const wp<BBinder>& binder) + : mRpcServer(rpcServer), mKeepAliveBinder(keepAliveBinder), mBinder(binder) {} + void binderDied(const wp<IBinder>&) override { + LOG_RPC_DETAIL("RpcServerLink: binder died, shutting down RpcServer"); + if (mRpcServer == nullptr) { + ALOGW("RpcServerLink: Unable to shut down RpcServer because it does not exist."); + } else { + ALOGW_IF(!mRpcServer->shutdown(), + "RpcServerLink: RpcServer did not shut down properly. Not started?"); + } + mRpcServer.clear(); + + auto promoted = mBinder.promote(); + if (promoted == nullptr) { + ALOGW("RpcServerLink: Unable to remove link from parent binder object because parent " + "binder object is gone."); + } else { + promoted->removeRpcServerLink(sp<RpcServerLink>::fromExisting(this)); + } + mBinder.clear(); + } + +private: + sp<RpcServer> mRpcServer; + sp<IBinder> mKeepAliveBinder; // hold to avoid automatically unlinking + wp<BBinder> mBinder; +}; + class BBinder::Extras { public: @@ -191,7 +225,7 @@ public: // for below objects Mutex mLock; - sp<RpcServer> mRpcServer; + std::set<sp<RpcServerLink>> mRpcServerLinks; BpBinder::ObjectManager mObjects; }; @@ -320,9 +354,9 @@ bool BBinder::isRequestingSid() void BBinder::setRequestingSid(bool requestingSid) { - ALOGW_IF(mParceled, - "setRequestingSid() should not be called after a binder object " - "is parceled/sent to another process"); + LOG_ALWAYS_FATAL_IF(mParceled, + "setRequestingSid() should not be called after a binder object " + "is parceled/sent to another process"); Extras* e = mExtras.load(std::memory_order_acquire); @@ -346,9 +380,9 @@ sp<IBinder> BBinder::getExtension() { } void BBinder::setMinSchedulerPolicy(int policy, int priority) { - ALOGW_IF(mParceled, - "setMinSchedulerPolicy() should not be called after a binder object " - "is parceled/sent to another process"); + LOG_ALWAYS_FATAL_IF(mParceled, + "setMinSchedulerPolicy() should not be called after a binder object " + "is parceled/sent to another process"); switch (policy) { case SCHED_NORMAL: @@ -397,9 +431,9 @@ bool BBinder::isInheritRt() { } void BBinder::setInheritRt(bool inheritRt) { - ALOGW_IF(mParceled, - "setInheritRt() should not be called after a binder object " - "is parceled/sent to another process"); + LOG_ALWAYS_FATAL_IF(mParceled, + "setInheritRt() should not be called after a binder object " + "is parceled/sent to another process"); Extras* e = mExtras.load(std::memory_order_acquire); @@ -420,9 +454,9 @@ pid_t BBinder::getDebugPid() { } void BBinder::setExtension(const sp<IBinder>& extension) { - ALOGW_IF(mParceled, - "setExtension() should not be called after a binder object " - "is parceled/sent to another process"); + LOG_ALWAYS_FATAL_IF(mParceled, + "setExtension() should not be called after a binder object " + "is parceled/sent to another process"); Extras* e = getOrCreateExtras(); e->mExtension = extension; @@ -449,36 +483,37 @@ status_t BBinder::setRpcClientDebug(const Parcel& data) { status_t status; bool hasSocketFd; android::base::unique_fd clientFd; - uint32_t maxRpcThreads; if (status = data.readBool(&hasSocketFd); status != OK) return status; if (hasSocketFd) { if (status = data.readUniqueFileDescriptor(&clientFd); status != OK) return status; } - if (status = data.readUint32(&maxRpcThreads); status != OK) return status; + sp<IBinder> keepAliveBinder; + if (status = data.readNullableStrongBinder(&keepAliveBinder); status != OK) return status; - return setRpcClientDebug(std::move(clientFd), maxRpcThreads); + return setRpcClientDebug(std::move(clientFd), keepAliveBinder); } -status_t BBinder::setRpcClientDebug(android::base::unique_fd socketFd, uint32_t maxRpcThreads) { +status_t BBinder::setRpcClientDebug(android::base::unique_fd socketFd, + const sp<IBinder>& keepAliveBinder) { if constexpr (!kEnableRpcDevServers) { ALOGW("%s: disallowed because RPC is not enabled", __PRETTY_FUNCTION__); return INVALID_OPERATION; } const int socketFdForPrint = socketFd.get(); - LOG_RPC_DETAIL("%s(%d, %" PRIu32 ")", __PRETTY_FUNCTION__, socketFdForPrint, maxRpcThreads); + LOG_RPC_DETAIL("%s(fd=%d)", __PRETTY_FUNCTION__, socketFdForPrint); if (!socketFd.ok()) { ALOGE("%s: No socket FD provided.", __PRETTY_FUNCTION__); return BAD_VALUE; } - if (maxRpcThreads <= 0) { - ALOGE("%s: RPC is useless with %" PRIu32 " threads.", __PRETTY_FUNCTION__, maxRpcThreads); - return BAD_VALUE; + + if (keepAliveBinder == nullptr) { + ALOGE("%s: No keepAliveBinder provided.", __PRETTY_FUNCTION__); + return UNEXPECTED_NULL; } - // TODO(b/182914638): RPC and binder should share the same thread pool count. size_t binderThreadPoolMaxCount = ProcessState::self()->getThreadPoolMaxThreadCount(); if (binderThreadPoolMaxCount <= 1) { ALOGE("%s: ProcessState thread pool max count is %zu. RPC is disabled for this service " @@ -487,24 +522,38 @@ status_t BBinder::setRpcClientDebug(android::base::unique_fd socketFd, uint32_t return INVALID_OPERATION; } + // Weak ref to avoid circular dependency: + // BBinder -> RpcServerLink ----> RpcServer -X-> BBinder + // `-X-> BBinder + auto weakThis = wp<BBinder>::fromExisting(this); + Extras* e = getOrCreateExtras(); AutoMutex _l(e->mLock); - if (e->mRpcServer != nullptr) { - ALOGE("%s: Already have RPC client", __PRETTY_FUNCTION__); - return ALREADY_EXISTS; + auto rpcServer = RpcServer::make(); + LOG_ALWAYS_FATAL_IF(rpcServer == nullptr, "RpcServer::make returns null"); + rpcServer->iUnderstandThisCodeIsExperimentalAndIWillNotUseItInProduction(); + auto link = sp<RpcServerLink>::make(rpcServer, keepAliveBinder, weakThis); + if (auto status = keepAliveBinder->linkToDeath(link, nullptr, 0); status != OK) { + ALOGE("%s: keepAliveBinder->linkToDeath returns %s", __PRETTY_FUNCTION__, + statusToString(status).c_str()); + return status; } - e->mRpcServer = RpcServer::make(); - LOG_ALWAYS_FATAL_IF(e->mRpcServer == nullptr, "RpcServer::make returns null"); - e->mRpcServer->iUnderstandThisCodeIsExperimentalAndIWillNotUseItInProduction(); - // Weak ref to avoid circular dependency: BBinder -> RpcServer -X-> BBinder - e->mRpcServer->setRootObjectWeak(wp<BBinder>::fromExisting(this)); - e->mRpcServer->setupExternalServer(std::move(socketFd)); - e->mRpcServer->start(); - LOG_RPC_DETAIL("%s(%d, %" PRIu32 ") successful", __PRETTY_FUNCTION__, socketFdForPrint, - maxRpcThreads); + rpcServer->setRootObjectWeak(weakThis); + rpcServer->setupExternalServer(std::move(socketFd)); + rpcServer->setMaxThreads(binderThreadPoolMaxCount); + rpcServer->start(); + e->mRpcServerLinks.emplace(link); + LOG_RPC_DETAIL("%s(fd=%d) successful", __PRETTY_FUNCTION__, socketFdForPrint); return OK; } +void BBinder::removeRpcServerLink(const sp<RpcServerLink>& link) { + Extras* e = mExtras.load(std::memory_order_acquire); + if (!e) return; + AutoMutex _l(e->mLock); + (void)e->mRpcServerLinks.erase(link); +} + BBinder::~BBinder() { Extras* e = mExtras.load(std::memory_order_relaxed); diff --git a/libs/binder/Parcel.cpp b/libs/binder/Parcel.cpp index 232a70c894..ebba375a79 100644 --- a/libs/binder/Parcel.cpp +++ b/libs/binder/Parcel.cpp @@ -206,6 +206,7 @@ status_t Parcel::flattenBinder(const sp<IBinder>& binder) { status_t status = writeInt32(1); // non-null if (status != OK) return status; RpcAddress address = RpcAddress::zero(); + // TODO(b/167966510): need to undo this if the Parcel is not sent status = mSession->state()->onBinderLeaving(mSession, binder, &address); if (status != OK) return status; status = address.writeToParcel(this); diff --git a/libs/binder/RpcAddress.cpp b/libs/binder/RpcAddress.cpp index 5c3232045e..98dee9a039 100644 --- a/libs/binder/RpcAddress.cpp +++ b/libs/binder/RpcAddress.cpp @@ -29,7 +29,7 @@ RpcAddress RpcAddress::zero() { } bool RpcAddress::isZero() const { - RpcWireAddress ZERO{0}; + RpcWireAddress ZERO{.options = 0}; return memcmp(mRawAddr.get(), &ZERO, sizeof(RpcWireAddress)) == 0; } @@ -51,13 +51,34 @@ static void ReadRandomBytes(uint8_t* buf, size_t len) { close(fd); } -RpcAddress RpcAddress::unique() { +RpcAddress RpcAddress::random(bool forServer) { + // The remainder of this header acts as reserved space for different kinds + // of binder objects. + uint64_t options = RPC_WIRE_ADDRESS_OPTION_CREATED; + + // servers and clients allocate addresses independently, so this bit can + // tell you where an address originates + if (forServer) options |= RPC_WIRE_ADDRESS_OPTION_FOR_SERVER; + RpcAddress ret; - ReadRandomBytes((uint8_t*)ret.mRawAddr.get(), sizeof(RpcWireAddress)); + RpcWireAddress* raw = ret.mRawAddr.get(); + + raw->options = options; + ReadRandomBytes(raw->address, sizeof(raw->address)); + LOG_RPC_DETAIL("Creating new address: %s", ret.toString().c_str()); return ret; } +bool RpcAddress::isForServer() const { + return mRawAddr.get()->options & RPC_WIRE_ADDRESS_OPTION_FOR_SERVER; +} + +bool RpcAddress::isRecognizedType() const { + uint64_t allKnownOptions = RPC_WIRE_ADDRESS_OPTION_CREATED | RPC_WIRE_ADDRESS_OPTION_FOR_SERVER; + return (mRawAddr.get()->options & ~allKnownOptions) == 0; +} + RpcAddress RpcAddress::fromRawEmbedded(const RpcWireAddress* raw) { RpcAddress addr; memcpy(addr.mRawAddr.get(), raw, sizeof(RpcWireAddress)); diff --git a/libs/binder/RpcServer.cpp b/libs/binder/RpcServer.cpp index 2d2eed2671..a8f3fa8f6f 100644 --- a/libs/binder/RpcServer.cpp +++ b/libs/binder/RpcServer.cpp @@ -270,14 +270,25 @@ void RpcServer::establishConnection(sp<RpcServer>&& server, base::unique_fd clie return; } - if (header.sessionId == RPC_SESSION_ID_NEW) { + RpcAddress sessionId = RpcAddress::fromRawEmbedded(&header.sessionId); + + if (sessionId.isZero()) { if (reverse) { ALOGE("Cannot create a new session with a reverse connection, would leak"); return; } - LOG_ALWAYS_FATAL_IF(server->mSessionIdCounter >= INT32_MAX, "Out of session IDs"); - server->mSessionIdCounter++; + RpcAddress sessionId = RpcAddress::zero(); + size_t tries = 0; + do { + // don't block if there is some entropy issue + if (tries++ > 5) { + ALOGE("Cannot find new address: %s", sessionId.toString().c_str()); + return; + } + + sessionId = RpcAddress::random(true /*forServer*/); + } while (server->mSessions.end() != server->mSessions.find(sessionId)); session = RpcSession::make(); session->setMaxThreads(server->mMaxThreads); @@ -285,35 +296,38 @@ void RpcServer::establishConnection(sp<RpcServer>&& server, base::unique_fd clie sp<RpcServer::EventListener>::fromExisting( static_cast<RpcServer::EventListener*>( server.get())), - server->mSessionIdCounter)) { + sessionId)) { ALOGE("Failed to attach server to session"); return; } - server->mSessions[server->mSessionIdCounter] = session; + server->mSessions[sessionId] = session; } else { - auto it = server->mSessions.find(header.sessionId); + auto it = server->mSessions.find(sessionId); if (it == server->mSessions.end()) { - ALOGE("Cannot add thread, no record of session with ID %d", header.sessionId); + ALOGE("Cannot add thread, no record of session with ID %s", + sessionId.toString().c_str()); return; } session = it->second; } if (reverse) { - LOG_ALWAYS_FATAL_IF(!session->addClientConnection(std::move(clientFd)), + LOG_ALWAYS_FATAL_IF(!session->addOutgoingConnection(std::move(clientFd), true), "server state must already be initialized"); return; } detachGuard.Disable(); - session->preJoin(std::move(thisThread)); + session->preJoinThreadOwnership(std::move(thisThread)); } + auto setupResult = session->preJoinSetup(std::move(clientFd)); + // avoid strong cycle server = nullptr; - RpcSession::join(std::move(session), std::move(clientFd)); + RpcSession::join(std::move(session), std::move(setupResult)); } bool RpcServer::setupSocketServer(const RpcSocketAddress& addr) { @@ -348,19 +362,21 @@ bool RpcServer::setupSocketServer(const RpcSocketAddress& addr) { return true; } -void RpcServer::onSessionLockedAllServerThreadsEnded(const sp<RpcSession>& session) { +void RpcServer::onSessionLockedAllIncomingThreadsEnded(const sp<RpcSession>& session) { auto id = session->mId; LOG_ALWAYS_FATAL_IF(id == std::nullopt, "Server sessions must be initialized with ID"); - LOG_RPC_DETAIL("Dropping session %d", *id); + LOG_RPC_DETAIL("Dropping session with address %s", id->toString().c_str()); std::lock_guard<std::mutex> _l(mLock); auto it = mSessions.find(*id); - LOG_ALWAYS_FATAL_IF(it == mSessions.end(), "Bad state, unknown session id %d", *id); - LOG_ALWAYS_FATAL_IF(it->second != session, "Bad state, session has id mismatch %d", *id); + LOG_ALWAYS_FATAL_IF(it == mSessions.end(), "Bad state, unknown session id %s", + id->toString().c_str()); + LOG_ALWAYS_FATAL_IF(it->second != session, "Bad state, session has id mismatch %s", + id->toString().c_str()); (void)mSessions.erase(it); } -void RpcServer::onSessionServerThreadEnded() { +void RpcServer::onSessionIncomingThreadEnded() { mShutdownCv.notify_all(); } diff --git a/libs/binder/RpcSession.cpp b/libs/binder/RpcSession.cpp index 62118ffddc..4f55eef2d1 100644 --- a/libs/binder/RpcSession.cpp +++ b/libs/binder/RpcSession.cpp @@ -51,7 +51,7 @@ RpcSession::~RpcSession() { LOG_RPC_DETAIL("RpcSession destroyed %p", this); std::lock_guard<std::mutex> _l(mMutex); - LOG_ALWAYS_FATAL_IF(mServerConnections.size() != 0, + LOG_ALWAYS_FATAL_IF(mIncomingConnections.size() != 0, "Should not be able to destroy a session with servers in use."); } @@ -61,10 +61,10 @@ sp<RpcSession> RpcSession::make() { void RpcSession::setMaxThreads(size_t threads) { std::lock_guard<std::mutex> _l(mMutex); - LOG_ALWAYS_FATAL_IF(!mClientConnections.empty() || !mServerConnections.empty(), + LOG_ALWAYS_FATAL_IF(!mOutgoingConnections.empty() || !mIncomingConnections.empty(), "Must set max threads before setting up connections, but has %zu client(s) " "and %zu server(s)", - mClientConnections.size(), mServerConnections.size()); + mOutgoingConnections.size(), mIncomingConnections.size()); mMaxThreads = threads; } @@ -100,17 +100,23 @@ bool RpcSession::addNullDebuggingClient() { return false; } - return addClientConnection(std::move(serverFd)); + return addOutgoingConnection(std::move(serverFd), false); } sp<IBinder> RpcSession::getRootObject() { - ExclusiveConnection connection(sp<RpcSession>::fromExisting(this), ConnectionUse::CLIENT); - return state()->getRootObject(connection.fd(), sp<RpcSession>::fromExisting(this)); + ExclusiveConnection connection; + status_t status = ExclusiveConnection::find(sp<RpcSession>::fromExisting(this), + ConnectionUse::CLIENT, &connection); + if (status != OK) return nullptr; + return state()->getRootObject(connection.get(), sp<RpcSession>::fromExisting(this)); } status_t RpcSession::getRemoteMaxThreads(size_t* maxThreads) { - ExclusiveConnection connection(sp<RpcSession>::fromExisting(this), ConnectionUse::CLIENT); - return state()->getMaxThreads(connection.fd(), sp<RpcSession>::fromExisting(this), maxThreads); + ExclusiveConnection connection; + status_t status = ExclusiveConnection::find(sp<RpcSession>::fromExisting(this), + ConnectionUse::CLIENT, &connection); + if (status != OK) return status; + return state()->getMaxThreads(connection.get(), sp<RpcSession>::fromExisting(this), maxThreads); } bool RpcSession::shutdownAndWait(bool wait) { @@ -133,17 +139,23 @@ bool RpcSession::shutdownAndWait(bool wait) { status_t RpcSession::transact(const sp<IBinder>& binder, uint32_t code, const Parcel& data, Parcel* reply, uint32_t flags) { - ExclusiveConnection connection(sp<RpcSession>::fromExisting(this), - (flags & IBinder::FLAG_ONEWAY) ? ConnectionUse::CLIENT_ASYNC - : ConnectionUse::CLIENT); - return state()->transact(connection.fd(), binder, code, data, + ExclusiveConnection connection; + status_t status = + ExclusiveConnection::find(sp<RpcSession>::fromExisting(this), + (flags & IBinder::FLAG_ONEWAY) ? ConnectionUse::CLIENT_ASYNC + : ConnectionUse::CLIENT, + &connection); + if (status != OK) return status; + return state()->transact(connection.get(), binder, code, data, sp<RpcSession>::fromExisting(this), reply, flags); } status_t RpcSession::sendDecStrong(const RpcAddress& address) { - ExclusiveConnection connection(sp<RpcSession>::fromExisting(this), - ConnectionUse::CLIENT_REFCOUNT); - return state()->sendDecStrong(connection.fd(), sp<RpcSession>::fromExisting(this), address); + ExclusiveConnection connection; + status_t status = ExclusiveConnection::find(sp<RpcSession>::fromExisting(this), + ConnectionUse::CLIENT_REFCOUNT, &connection); + if (status != OK) return status; + return state()->sendDecStrong(connection.get(), sp<RpcSession>::fromExisting(this), address); } std::unique_ptr<RpcSession::FdTrigger> RpcSession::FdTrigger::make() { @@ -206,25 +218,27 @@ status_t RpcSession::readId() { LOG_ALWAYS_FATAL_IF(mForServer != nullptr, "Can only update ID for client."); } - int32_t id; + ExclusiveConnection connection; + status_t status = ExclusiveConnection::find(sp<RpcSession>::fromExisting(this), + ConnectionUse::CLIENT, &connection); + if (status != OK) return status; - ExclusiveConnection connection(sp<RpcSession>::fromExisting(this), ConnectionUse::CLIENT); - status_t status = - state()->getSessionId(connection.fd(), sp<RpcSession>::fromExisting(this), &id); + mId = RpcAddress::zero(); + status = state()->getSessionId(connection.get(), sp<RpcSession>::fromExisting(this), + &mId.value()); if (status != OK) return status; - LOG_RPC_DETAIL("RpcSession %p has id %d", this, id); - mId = id; + LOG_RPC_DETAIL("RpcSession %p has id %s", this, mId->toString().c_str()); return OK; } -void RpcSession::WaitForShutdownListener::onSessionLockedAllServerThreadsEnded( +void RpcSession::WaitForShutdownListener::onSessionLockedAllIncomingThreadsEnded( const sp<RpcSession>& session) { (void)session; mShutdown = true; } -void RpcSession::WaitForShutdownListener::onSessionServerThreadEnded() { +void RpcSession::WaitForShutdownListener::onSessionIncomingThreadEnded() { mCv.notify_all(); } @@ -236,7 +250,7 @@ void RpcSession::WaitForShutdownListener::waitForShutdown(std::unique_lock<std:: } } -void RpcSession::preJoin(std::thread thread) { +void RpcSession::preJoinThreadOwnership(std::thread thread) { LOG_ALWAYS_FATAL_IF(thread.get_id() != std::this_thread::get_id(), "Must own this thread"); { @@ -245,23 +259,38 @@ void RpcSession::preJoin(std::thread thread) { } } -void RpcSession::join(sp<RpcSession>&& session, unique_fd client) { +RpcSession::PreJoinSetupResult RpcSession::preJoinSetup(base::unique_fd fd) { // must be registered to allow arbitrary client code executing commands to // be able to do nested calls (we can't only read from it) - sp<RpcConnection> connection = session->assignServerToThisThread(std::move(client)); + sp<RpcConnection> connection = assignIncomingConnectionToThisThread(std::move(fd)); - while (true) { - status_t error = session->state()->getAndExecuteCommand(connection->fd, session, - RpcState::CommandType::ANY); + status_t status = mState->readConnectionInit(connection, sp<RpcSession>::fromExisting(this)); - if (error != OK) { - LOG_RPC_DETAIL("Binder connection thread closing w/ status %s", - statusToString(error).c_str()); - break; + return PreJoinSetupResult{ + .connection = std::move(connection), + .status = status, + }; +} + +void RpcSession::join(sp<RpcSession>&& session, PreJoinSetupResult&& setupResult) { + sp<RpcConnection>& connection = setupResult.connection; + + if (setupResult.status == OK) { + while (true) { + status_t status = session->state()->getAndExecuteCommand(connection, session, + RpcState::CommandType::ANY); + if (status != OK) { + LOG_RPC_DETAIL("Binder connection thread closing w/ status %s", + statusToString(status).c_str()); + break; + } } + } else { + ALOGE("Connection failed to init, closing with status %s", + statusToString(setupResult.status).c_str()); } - LOG_ALWAYS_FATAL_IF(!session->removeServerConnection(connection), + LOG_ALWAYS_FATAL_IF(!session->removeIncomingConnection(connection), "bad state: connection object guaranteed to be in list"); sp<RpcSession::EventListener> listener; @@ -278,25 +307,30 @@ void RpcSession::join(sp<RpcSession>&& session, unique_fd client) { session = nullptr; if (listener != nullptr) { - listener->onSessionServerThreadEnded(); + listener->onSessionIncomingThreadEnded(); } } -wp<RpcServer> RpcSession::server() { - return mForServer; +sp<RpcServer> RpcSession::server() { + RpcServer* unsafeServer = mForServer.unsafe_get(); + sp<RpcServer> server = mForServer.promote(); + + LOG_ALWAYS_FATAL_IF((unsafeServer == nullptr) != (server == nullptr), + "wp<> is to avoid strong cycle only"); + return server; } bool RpcSession::setupSocketClient(const RpcSocketAddress& addr) { { std::lock_guard<std::mutex> _l(mMutex); - LOG_ALWAYS_FATAL_IF(mClientConnections.size() != 0, + LOG_ALWAYS_FATAL_IF(mOutgoingConnections.size() != 0, "Must only setup session once, but already has %zu clients", - mClientConnections.size()); + mOutgoingConnections.size()); } - if (!setupOneSocketConnection(addr, RPC_SESSION_ID_NEW, false /*reverse*/)) return false; + if (!setupOneSocketConnection(addr, RpcAddress::zero(), false /*reverse*/)) return false; - // TODO(b/185167543): we should add additional sessions dynamically + // TODO(b/189955605): we should add additional sessions dynamically // instead of all at once. // TODO(b/186470974): first risk of blocking size_t numThreadsAvailable; @@ -314,11 +348,11 @@ bool RpcSession::setupSocketClient(const RpcSocketAddress& addr) { // we've already setup one client for (size_t i = 0; i + 1 < numThreadsAvailable; i++) { - // TODO(b/185167543): shutdown existing connections? + // TODO(b/189955605): shutdown existing connections? if (!setupOneSocketConnection(addr, mId.value(), false /*reverse*/)) return false; } - // TODO(b/185167543): we should add additional sessions dynamically + // TODO(b/189955605): we should add additional sessions dynamically // instead of all at once - the other side should be responsible for setting // up additional connections. We need to create at least one (unless 0 are // requested to be set) in order to allow the other side to reliably make @@ -331,7 +365,8 @@ bool RpcSession::setupSocketClient(const RpcSocketAddress& addr) { return true; } -bool RpcSession::setupOneSocketConnection(const RpcSocketAddress& addr, int32_t id, bool reverse) { +bool RpcSession::setupOneSocketConnection(const RpcSocketAddress& addr, const RpcAddress& id, + bool reverse) { for (size_t tries = 0; tries < 5; tries++) { if (tries > 0) usleep(10000); @@ -355,9 +390,9 @@ bool RpcSession::setupOneSocketConnection(const RpcSocketAddress& addr, int32_t return false; } - RpcConnectionHeader header{ - .sessionId = id, - }; + RpcConnectionHeader header{.options = 0}; + memcpy(&header.sessionId, &id.viewRawEmbedded(), sizeof(RpcWireAddress)); + if (reverse) header.options |= RPC_CONNECTION_OPTION_REVERSE; if (sizeof(header) != TEMP_FAILURE_RETRY(write(serverFd.get(), &header, sizeof(header)))) { @@ -381,20 +416,23 @@ bool RpcSession::setupOneSocketConnection(const RpcSocketAddress& addr, int32_t unique_fd fd = std::move(serverFd); // NOLINTNEXTLINE(performance-unnecessary-copy-initialization) sp<RpcSession> session = thiz; - session->preJoin(std::move(thread)); - ownershipTransferred = true; - joinCv.notify_one(); + session->preJoinThreadOwnership(std::move(thread)); + + // only continue once we have a response or the connection fails + auto setupResult = session->preJoinSetup(std::move(fd)); + ownershipTransferred = true; threadLock.unlock(); + joinCv.notify_one(); // do not use & vars below - RpcSession::join(std::move(session), std::move(fd)); + RpcSession::join(std::move(session), std::move(setupResult)); }); joinCv.wait(lock, [&] { return ownershipTransferred; }); LOG_ALWAYS_FATAL_IF(!ownershipTransferred); return true; } else { - return addClientConnection(std::move(serverFd)); + return addOutgoingConnection(std::move(serverFd), true); } } @@ -402,25 +440,39 @@ bool RpcSession::setupOneSocketConnection(const RpcSocketAddress& addr, int32_t return false; } -bool RpcSession::addClientConnection(unique_fd fd) { - std::lock_guard<std::mutex> _l(mMutex); +bool RpcSession::addOutgoingConnection(unique_fd fd, bool init) { + sp<RpcConnection> connection = sp<RpcConnection>::make(); + { + std::lock_guard<std::mutex> _l(mMutex); - // first client connection added, but setForServer not called, so - // initializaing for a client. - if (mShutdownTrigger == nullptr) { - mShutdownTrigger = FdTrigger::make(); - mEventListener = mShutdownListener = sp<WaitForShutdownListener>::make(); - if (mShutdownTrigger == nullptr) return false; + // first client connection added, but setForServer not called, so + // initializaing for a client. + if (mShutdownTrigger == nullptr) { + mShutdownTrigger = FdTrigger::make(); + mEventListener = mShutdownListener = sp<WaitForShutdownListener>::make(); + if (mShutdownTrigger == nullptr) return false; + } + + connection->fd = std::move(fd); + connection->exclusiveTid = gettid(); + mOutgoingConnections.push_back(connection); } - sp<RpcConnection> session = sp<RpcConnection>::make(); - session->fd = std::move(fd); - mClientConnections.push_back(session); - return true; + status_t status = OK; + if (init) { + mState->sendConnectionInit(connection, sp<RpcSession>::fromExisting(this)); + } + + { + std::lock_guard<std::mutex> _l(mMutex); + connection->exclusiveTid = std::nullopt; + } + + return status == OK; } bool RpcSession::setForServer(const wp<RpcServer>& server, const wp<EventListener>& eventListener, - int32_t sessionId) { + const RpcAddress& sessionId) { LOG_ALWAYS_FATAL_IF(mForServer != nullptr); LOG_ALWAYS_FATAL_IF(server == nullptr); LOG_ALWAYS_FATAL_IF(mEventListener != nullptr); @@ -436,25 +488,26 @@ bool RpcSession::setForServer(const wp<RpcServer>& server, const wp<EventListene return true; } -sp<RpcSession::RpcConnection> RpcSession::assignServerToThisThread(unique_fd fd) { +sp<RpcSession::RpcConnection> RpcSession::assignIncomingConnectionToThisThread(unique_fd fd) { std::lock_guard<std::mutex> _l(mMutex); sp<RpcConnection> session = sp<RpcConnection>::make(); session->fd = std::move(fd); session->exclusiveTid = gettid(); - mServerConnections.push_back(session); + mIncomingConnections.push_back(session); return session; } -bool RpcSession::removeServerConnection(const sp<RpcConnection>& connection) { +bool RpcSession::removeIncomingConnection(const sp<RpcConnection>& connection) { std::lock_guard<std::mutex> _l(mMutex); - if (auto it = std::find(mServerConnections.begin(), mServerConnections.end(), connection); - it != mServerConnections.end()) { - mServerConnections.erase(it); - if (mServerConnections.size() == 0) { + if (auto it = std::find(mIncomingConnections.begin(), mIncomingConnections.end(), connection); + it != mIncomingConnections.end()) { + mIncomingConnections.erase(it); + if (mIncomingConnections.size() == 0) { sp<EventListener> listener = mEventListener.promote(); if (listener) { - listener->onSessionLockedAllServerThreadsEnded(sp<RpcSession>::fromExisting(this)); + listener->onSessionLockedAllIncomingThreadsEnded( + sp<RpcSession>::fromExisting(this)); } } return true; @@ -462,13 +515,16 @@ bool RpcSession::removeServerConnection(const sp<RpcConnection>& connection) { return false; } -RpcSession::ExclusiveConnection::ExclusiveConnection(const sp<RpcSession>& session, - ConnectionUse use) - : mSession(session) { +status_t RpcSession::ExclusiveConnection::find(const sp<RpcSession>& session, ConnectionUse use, + ExclusiveConnection* connection) { + connection->mSession = session; + connection->mConnection = nullptr; + connection->mReentrant = false; + pid_t tid = gettid(); - std::unique_lock<std::mutex> _l(mSession->mMutex); + std::unique_lock<std::mutex> _l(session->mMutex); - mSession->mWaitingThreads++; + session->mWaitingThreads++; while (true) { sp<RpcConnection> exclusive; sp<RpcConnection> available; @@ -476,11 +532,11 @@ RpcSession::ExclusiveConnection::ExclusiveConnection(const sp<RpcSession>& sessi // CHECK FOR DEDICATED CLIENT SOCKET // // A server/looper should always use a dedicated connection if available - findConnection(tid, &exclusive, &available, mSession->mClientConnections, - mSession->mClientConnectionsOffset); + findConnection(tid, &exclusive, &available, session->mOutgoingConnections, + session->mOutgoingConnectionsOffset); // WARNING: this assumes a server cannot request its client to send - // a transaction, as mServerConnections is excluded below. + // a transaction, as mIncomingConnections is excluded below. // // Imagine we have more than one thread in play, and a single thread // sends a synchronous, then an asynchronous command. Imagine the @@ -490,42 +546,59 @@ RpcSession::ExclusiveConnection::ExclusiveConnection(const sp<RpcSession>& sessi // command. So, we move to considering the second available thread // for subsequent calls. if (use == ConnectionUse::CLIENT_ASYNC && (exclusive != nullptr || available != nullptr)) { - mSession->mClientConnectionsOffset = - (mSession->mClientConnectionsOffset + 1) % mSession->mClientConnections.size(); + session->mOutgoingConnectionsOffset = (session->mOutgoingConnectionsOffset + 1) % + session->mOutgoingConnections.size(); } - // USE SERVING SOCKET (for nested transaction) - // - // asynchronous calls cannot be nested + // USE SERVING SOCKET (e.g. nested transaction) if (use != ConnectionUse::CLIENT_ASYNC) { + sp<RpcConnection> exclusiveIncoming; // server connections are always assigned to a thread - findConnection(tid, &exclusive, nullptr /*available*/, mSession->mServerConnections, - 0 /* index hint */); + findConnection(tid, &exclusiveIncoming, nullptr /*available*/, + session->mIncomingConnections, 0 /* index hint */); + + // asynchronous calls cannot be nested, we currently allow ref count + // calls to be nested (so that you can use this without having extra + // threads). Note 'drainCommands' is used so that these ref counts can't + // build up. + if (exclusiveIncoming != nullptr) { + if (exclusiveIncoming->allowNested) { + // guaranteed to be processed as nested command + exclusive = exclusiveIncoming; + } else if (use == ConnectionUse::CLIENT_REFCOUNT && available == nullptr) { + // prefer available socket, but if we don't have one, don't + // wait for one + exclusive = exclusiveIncoming; + } + } } // if our thread is already using a connection, prioritize using that if (exclusive != nullptr) { - mConnection = exclusive; - mReentrant = true; + connection->mConnection = exclusive; + connection->mReentrant = true; break; } else if (available != nullptr) { - mConnection = available; - mConnection->exclusiveTid = tid; + connection->mConnection = available; + connection->mConnection->exclusiveTid = tid; break; } - // TODO(b/185167543): this should return an error, rather than crash a - // server - // in regular binder, this would usually be a deadlock :) - LOG_ALWAYS_FATAL_IF(mSession->mClientConnections.size() == 0, - "Session has no client connections. This is required for an RPC server " - "to make any non-nested (e.g. oneway or on another thread) calls."); + if (session->mOutgoingConnections.size() == 0) { + ALOGE("Session has no client connections. This is required for an RPC server to make " + "any non-nested (e.g. oneway or on another thread) calls. Use: %d. Server " + "connections: %zu", + static_cast<int>(use), session->mIncomingConnections.size()); + return WOULD_BLOCK; + } LOG_RPC_DETAIL("No available connections (have %zu clients and %zu servers). Waiting...", - mSession->mClientConnections.size(), mSession->mServerConnections.size()); - mSession->mAvailableConnectionCv.wait(_l); + session->mOutgoingConnections.size(), session->mIncomingConnections.size()); + session->mAvailableConnectionCv.wait(_l); } - mSession->mWaitingThreads--; + session->mWaitingThreads--; + + return OK; } void RpcSession::ExclusiveConnection::findConnection(pid_t tid, sp<RpcConnection>* exclusive, @@ -559,7 +632,7 @@ RpcSession::ExclusiveConnection::~ExclusiveConnection() { // reentrant use of a connection means something less deep in the call stack // is using this fd, and it retains the right to it. So, we don't give up // exclusive ownership, and no thread is freed. - if (!mReentrant) { + if (!mReentrant && mConnection != nullptr) { std::unique_lock<std::mutex> _l(mSession->mMutex); mConnection->exclusiveTid = std::nullopt; if (mSession->mWaitingThreads > 0) { diff --git a/libs/binder/RpcState.cpp b/libs/binder/RpcState.cpp index 3113841d27..fd2eff6870 100644 --- a/libs/binder/RpcState.cpp +++ b/libs/binder/RpcState.cpp @@ -83,21 +83,45 @@ status_t RpcState::onBinderLeaving(const sp<RpcSession>& session, const sp<IBind } LOG_ALWAYS_FATAL_IF(isRpc, "RPC binder must have known address at this point"); - auto&& [it, inserted] = mNodeForAddress.insert({RpcAddress::unique(), - BinderNode{ - .binder = binder, - .timesSent = 1, - .sentRef = binder, - }}); - // TODO(b/182939933): better organization could avoid needing this log - LOG_ALWAYS_FATAL_IF(!inserted); - - *outAddress = it->first; - return OK; + bool forServer = session->server() != nullptr; + + for (size_t tries = 0; tries < 5; tries++) { + auto&& [it, inserted] = mNodeForAddress.insert({RpcAddress::random(forServer), + BinderNode{ + .binder = binder, + .timesSent = 1, + .sentRef = binder, + }}); + if (inserted) { + *outAddress = it->first; + return OK; + } + + // well, we don't have visibility into the header here, but still + static_assert(sizeof(RpcWireAddress) == 40, "this log needs updating"); + ALOGW("2**256 is 1e77. If you see this log, you probably have some entropy issue, or maybe " + "you witness something incredible!"); + } + + ALOGE("Unable to create an address in order to send out %p", binder.get()); + return WOULD_BLOCK; } status_t RpcState::onBinderEntering(const sp<RpcSession>& session, const RpcAddress& address, sp<IBinder>* out) { + // ensure that: if we want to use addresses for something else in the future (for + // instance, allowing transitive binder sends), that we don't accidentally + // send those addresses to old server. Accidentally ignoring this in that + // case and considering the binder to be recognized could cause this + // process to accidentally proxy transactions for that binder. Of course, + // if we communicate with a binder, it could always be proxying + // information. However, we want to make sure that isn't done on accident + // by a client. + if (!address.isRecognizedType()) { + ALOGE("Address is of an unknown type, rejecting: %s", address.toString().c_str()); + return BAD_VALUE; + } + std::unique_lock<std::mutex> _l(mNodeMutex); if (mTerminated) return DEAD_OBJECT; @@ -117,6 +141,14 @@ status_t RpcState::onBinderEntering(const sp<RpcSession>& session, const RpcAddr return OK; } + // we don't know about this binder, so the other side of the connection + // should have created it. + if (address.isForServer() == !!session->server()) { + ALOGE("Server received unrecognized address which we should own the creation of %s.", + address.toString().c_str()); + return BAD_VALUE; + } + auto&& [it, inserted] = mNodeForAddress.insert({address, BinderNode{}}); LOG_ALWAYS_FATAL_IF(!inserted, "Failed to insert binder when creating proxy"); @@ -222,9 +254,11 @@ RpcState::CommandData::CommandData(size_t size) : mSize(size) { mData.reset(new (std::nothrow) uint8_t[size]); } -status_t RpcState::rpcSend(const base::unique_fd& fd, const sp<RpcSession>& session, - const char* what, const void* data, size_t size) { - LOG_RPC_DETAIL("Sending %s on fd %d: %s", what, fd.get(), hexString(data, size).c_str()); +status_t RpcState::rpcSend(const sp<RpcSession::RpcConnection>& connection, + const sp<RpcSession>& session, const char* what, const void* data, + size_t size) { + LOG_RPC_DETAIL("Sending %s on fd %d: %s", what, connection->fd.get(), + hexString(data, size).c_str()); if (size > std::numeric_limits<ssize_t>::max()) { ALOGE("Cannot send %s at size %zu (too big)", what, size); @@ -232,12 +266,12 @@ status_t RpcState::rpcSend(const base::unique_fd& fd, const sp<RpcSession>& sess return BAD_VALUE; } - ssize_t sent = TEMP_FAILURE_RETRY(send(fd.get(), data, size, MSG_NOSIGNAL)); + ssize_t sent = TEMP_FAILURE_RETRY(send(connection->fd.get(), data, size, MSG_NOSIGNAL)); if (sent < 0 || sent != static_cast<ssize_t>(size)) { int savedErrno = errno; LOG_RPC_DETAIL("Failed to send %s (sent %zd of %zu bytes) on fd %d, error: %s", what, sent, - size, fd.get(), strerror(savedErrno)); + size, connection->fd.get(), strerror(savedErrno)); (void)session->shutdownAndWait(false); return -savedErrno; @@ -246,32 +280,60 @@ status_t RpcState::rpcSend(const base::unique_fd& fd, const sp<RpcSession>& sess return OK; } -status_t RpcState::rpcRec(const base::unique_fd& fd, const sp<RpcSession>& session, - const char* what, void* data, size_t size) { +status_t RpcState::rpcRec(const sp<RpcSession::RpcConnection>& connection, + const sp<RpcSession>& session, const char* what, void* data, + size_t size) { if (size > std::numeric_limits<ssize_t>::max()) { ALOGE("Cannot rec %s at size %zu (too big)", what, size); (void)session->shutdownAndWait(false); return BAD_VALUE; } - if (status_t status = session->mShutdownTrigger->interruptableReadFully(fd.get(), data, size); + if (status_t status = + session->mShutdownTrigger->interruptableReadFully(connection->fd.get(), data, size); status != OK) { - LOG_RPC_DETAIL("Failed to read %s (%zu bytes) on fd %d, error: %s", what, size, fd.get(), - statusToString(status).c_str()); + LOG_RPC_DETAIL("Failed to read %s (%zu bytes) on fd %d, error: %s", what, size, + connection->fd.get(), statusToString(status).c_str()); return status; } - LOG_RPC_DETAIL("Received %s on fd %d: %s", what, fd.get(), hexString(data, size).c_str()); + LOG_RPC_DETAIL("Received %s on fd %d: %s", what, connection->fd.get(), + hexString(data, size).c_str()); + return OK; +} + +status_t RpcState::sendConnectionInit(const sp<RpcSession::RpcConnection>& connection, + const sp<RpcSession>& session) { + RpcOutgoingConnectionInit init{ + .msg = RPC_CONNECTION_INIT_OKAY, + }; + return rpcSend(connection, session, "connection init", &init, sizeof(init)); +} + +status_t RpcState::readConnectionInit(const sp<RpcSession::RpcConnection>& connection, + const sp<RpcSession>& session) { + RpcOutgoingConnectionInit init; + if (status_t status = rpcRec(connection, session, "connection init", &init, sizeof(init)); + status != OK) + return status; + + static_assert(sizeof(init.msg) == sizeof(RPC_CONNECTION_INIT_OKAY)); + if (0 != strncmp(init.msg, RPC_CONNECTION_INIT_OKAY, sizeof(init.msg))) { + ALOGE("Connection init message unrecognized %.*s", static_cast<int>(sizeof(init.msg)), + init.msg); + return BAD_VALUE; + } return OK; } -sp<IBinder> RpcState::getRootObject(const base::unique_fd& fd, const sp<RpcSession>& session) { +sp<IBinder> RpcState::getRootObject(const sp<RpcSession::RpcConnection>& connection, + const sp<RpcSession>& session) { Parcel data; data.markForRpc(session); Parcel reply; - status_t status = transactAddress(fd, RpcAddress::zero(), RPC_SPECIAL_TRANSACT_GET_ROOT, data, - session, &reply, 0); + status_t status = transactAddress(connection, RpcAddress::zero(), RPC_SPECIAL_TRANSACT_GET_ROOT, + data, session, &reply, 0); if (status != OK) { ALOGE("Error getting root object: %s", statusToString(status).c_str()); return nullptr; @@ -280,14 +342,15 @@ sp<IBinder> RpcState::getRootObject(const base::unique_fd& fd, const sp<RpcSessi return reply.readStrongBinder(); } -status_t RpcState::getMaxThreads(const base::unique_fd& fd, const sp<RpcSession>& session, - size_t* maxThreadsOut) { +status_t RpcState::getMaxThreads(const sp<RpcSession::RpcConnection>& connection, + const sp<RpcSession>& session, size_t* maxThreadsOut) { Parcel data; data.markForRpc(session); Parcel reply; - status_t status = transactAddress(fd, RpcAddress::zero(), RPC_SPECIAL_TRANSACT_GET_MAX_THREADS, - data, session, &reply, 0); + status_t status = + transactAddress(connection, RpcAddress::zero(), RPC_SPECIAL_TRANSACT_GET_MAX_THREADS, + data, session, &reply, 0); if (status != OK) { ALOGE("Error getting max threads: %s", statusToString(status).c_str()); return status; @@ -305,30 +368,26 @@ status_t RpcState::getMaxThreads(const base::unique_fd& fd, const sp<RpcSession> return OK; } -status_t RpcState::getSessionId(const base::unique_fd& fd, const sp<RpcSession>& session, - int32_t* sessionIdOut) { +status_t RpcState::getSessionId(const sp<RpcSession::RpcConnection>& connection, + const sp<RpcSession>& session, RpcAddress* sessionIdOut) { Parcel data; data.markForRpc(session); Parcel reply; - status_t status = transactAddress(fd, RpcAddress::zero(), RPC_SPECIAL_TRANSACT_GET_SESSION_ID, - data, session, &reply, 0); + status_t status = + transactAddress(connection, RpcAddress::zero(), RPC_SPECIAL_TRANSACT_GET_SESSION_ID, + data, session, &reply, 0); if (status != OK) { ALOGE("Error getting session ID: %s", statusToString(status).c_str()); return status; } - int32_t sessionId; - status = reply.readInt32(&sessionId); - if (status != OK) return status; - - *sessionIdOut = sessionId; - return OK; + return sessionIdOut->readFromParcel(reply); } -status_t RpcState::transact(const base::unique_fd& fd, const sp<IBinder>& binder, uint32_t code, - const Parcel& data, const sp<RpcSession>& session, Parcel* reply, - uint32_t flags) { +status_t RpcState::transact(const sp<RpcSession::RpcConnection>& connection, + const sp<IBinder>& binder, uint32_t code, const Parcel& data, + const sp<RpcSession>& session, Parcel* reply, uint32_t flags) { if (!data.isForRpc()) { ALOGE("Refusing to send RPC with parcel not crafted for RPC"); return BAD_TYPE; @@ -342,12 +401,12 @@ status_t RpcState::transact(const base::unique_fd& fd, const sp<IBinder>& binder RpcAddress address = RpcAddress::zero(); if (status_t status = onBinderLeaving(session, binder, &address); status != OK) return status; - return transactAddress(fd, address, code, data, session, reply, flags); + return transactAddress(connection, address, code, data, session, reply, flags); } -status_t RpcState::transactAddress(const base::unique_fd& fd, const RpcAddress& address, - uint32_t code, const Parcel& data, const sp<RpcSession>& session, - Parcel* reply, uint32_t flags) { +status_t RpcState::transactAddress(const sp<RpcSession::RpcConnection>& connection, + const RpcAddress& address, uint32_t code, const Parcel& data, + const sp<RpcSession>& session, Parcel* reply, uint32_t flags) { LOG_ALWAYS_FATAL_IF(!data.isForRpc()); LOG_ALWAYS_FATAL_IF(data.objectsCount() != 0); @@ -397,23 +456,25 @@ status_t RpcState::transactAddress(const base::unique_fd& fd, const RpcAddress& memcpy(transactionData.data() + sizeof(RpcWireHeader) + sizeof(RpcWireTransaction), data.data(), data.dataSize()); - if (status_t status = - rpcSend(fd, session, "transaction", transactionData.data(), transactionData.size()); + if (status_t status = rpcSend(connection, session, "transaction", transactionData.data(), + transactionData.size()); status != OK) + // TODO(b/167966510): need to undo onBinderLeaving - we know the + // refcount isn't successfully transferred. return status; if (flags & IBinder::FLAG_ONEWAY) { - LOG_RPC_DETAIL("Oneway command, so no longer waiting on %d", fd.get()); + LOG_RPC_DETAIL("Oneway command, so no longer waiting on %d", connection->fd.get()); // Do not wait on result. // However, too many oneway calls may cause refcounts to build up and fill up the socket, // so process those. - return drainCommands(fd, session, CommandType::CONTROL_ONLY); + return drainCommands(connection, session, CommandType::CONTROL_ONLY); } LOG_ALWAYS_FATAL_IF(reply == nullptr, "Reply parcel must be used for synchronous transaction."); - return waitForReply(fd, session, reply); + return waitForReply(connection, session, reply); } static void cleanup_reply_data(Parcel* p, const uint8_t* data, size_t dataSize, @@ -425,17 +486,18 @@ static void cleanup_reply_data(Parcel* p, const uint8_t* data, size_t dataSize, LOG_ALWAYS_FATAL_IF(objectsCount, 0); } -status_t RpcState::waitForReply(const base::unique_fd& fd, const sp<RpcSession>& session, - Parcel* reply) { +status_t RpcState::waitForReply(const sp<RpcSession::RpcConnection>& connection, + const sp<RpcSession>& session, Parcel* reply) { RpcWireHeader command; while (true) { - if (status_t status = rpcRec(fd, session, "command header", &command, sizeof(command)); + if (status_t status = + rpcRec(connection, session, "command header", &command, sizeof(command)); status != OK) return status; if (command.command == RPC_COMMAND_REPLY) break; - if (status_t status = processServerCommand(fd, session, command, CommandType::ANY); + if (status_t status = processCommand(connection, session, command, CommandType::ANY); status != OK) return status; } @@ -443,7 +505,7 @@ status_t RpcState::waitForReply(const base::unique_fd& fd, const sp<RpcSession>& CommandData data(command.bodySize); if (!data.valid()) return NO_MEMORY; - if (status_t status = rpcRec(fd, session, "reply body", data.data(), command.bodySize); + if (status_t status = rpcRec(connection, session, "reply body", data.data(), command.bodySize); status != OK) return status; @@ -465,8 +527,8 @@ status_t RpcState::waitForReply(const base::unique_fd& fd, const sp<RpcSession>& return OK; } -status_t RpcState::sendDecStrong(const base::unique_fd& fd, const sp<RpcSession>& session, - const RpcAddress& addr) { +status_t RpcState::sendDecStrong(const sp<RpcSession::RpcConnection>& connection, + const sp<RpcSession>& session, const RpcAddress& addr) { { std::lock_guard<std::mutex> _l(mNodeMutex); if (mTerminated) return DEAD_OBJECT; // avoid fatal only, otherwise races @@ -485,39 +547,42 @@ status_t RpcState::sendDecStrong(const base::unique_fd& fd, const sp<RpcSession> .command = RPC_COMMAND_DEC_STRONG, .bodySize = sizeof(RpcWireAddress), }; - if (status_t status = rpcSend(fd, session, "dec ref header", &cmd, sizeof(cmd)); status != OK) + if (status_t status = rpcSend(connection, session, "dec ref header", &cmd, sizeof(cmd)); + status != OK) return status; - if (status_t status = rpcSend(fd, session, "dec ref body", &addr.viewRawEmbedded(), + if (status_t status = rpcSend(connection, session, "dec ref body", &addr.viewRawEmbedded(), sizeof(RpcWireAddress)); status != OK) return status; return OK; } -status_t RpcState::getAndExecuteCommand(const base::unique_fd& fd, const sp<RpcSession>& session, - CommandType type) { - LOG_RPC_DETAIL("getAndExecuteCommand on fd %d", fd.get()); +status_t RpcState::getAndExecuteCommand(const sp<RpcSession::RpcConnection>& connection, + const sp<RpcSession>& session, CommandType type) { + LOG_RPC_DETAIL("getAndExecuteCommand on fd %d", connection->fd.get()); RpcWireHeader command; - if (status_t status = rpcRec(fd, session, "command header", &command, sizeof(command)); + if (status_t status = rpcRec(connection, session, "command header", &command, sizeof(command)); status != OK) return status; - return processServerCommand(fd, session, command, type); + return processCommand(connection, session, command, type); } -status_t RpcState::drainCommands(const base::unique_fd& fd, const sp<RpcSession>& session, - CommandType type) { +status_t RpcState::drainCommands(const sp<RpcSession::RpcConnection>& connection, + const sp<RpcSession>& session, CommandType type) { uint8_t buf; - while (0 < TEMP_FAILURE_RETRY(recv(fd.get(), &buf, sizeof(buf), MSG_PEEK | MSG_DONTWAIT))) { - status_t status = getAndExecuteCommand(fd, session, type); + while (0 < TEMP_FAILURE_RETRY( + recv(connection->fd.get(), &buf, sizeof(buf), MSG_PEEK | MSG_DONTWAIT))) { + status_t status = getAndExecuteCommand(connection, session, type); if (status != OK) return status; } return OK; } -status_t RpcState::processServerCommand(const base::unique_fd& fd, const sp<RpcSession>& session, - const RpcWireHeader& command, CommandType type) { +status_t RpcState::processCommand(const sp<RpcSession::RpcConnection>& connection, + const sp<RpcSession>& session, const RpcWireHeader& command, + CommandType type) { IPCThreadState* kernelBinderState = IPCThreadState::selfOrNull(); IPCThreadState::SpGuard spGuard{ .address = __builtin_frame_address(0), @@ -536,9 +601,9 @@ status_t RpcState::processServerCommand(const base::unique_fd& fd, const sp<RpcS switch (command.command) { case RPC_COMMAND_TRANSACT: if (type != CommandType::ANY) return BAD_TYPE; - return processTransact(fd, session, command); + return processTransact(connection, session, command); case RPC_COMMAND_DEC_STRONG: - return processDecStrong(fd, session, command); + return processDecStrong(connection, session, command); } // We should always know the version of the opposing side, and since the @@ -550,20 +615,20 @@ status_t RpcState::processServerCommand(const base::unique_fd& fd, const sp<RpcS (void)session->shutdownAndWait(false); return DEAD_OBJECT; } -status_t RpcState::processTransact(const base::unique_fd& fd, const sp<RpcSession>& session, - const RpcWireHeader& command) { +status_t RpcState::processTransact(const sp<RpcSession::RpcConnection>& connection, + const sp<RpcSession>& session, const RpcWireHeader& command) { LOG_ALWAYS_FATAL_IF(command.command != RPC_COMMAND_TRANSACT, "command: %d", command.command); CommandData transactionData(command.bodySize); if (!transactionData.valid()) { return NO_MEMORY; } - if (status_t status = rpcRec(fd, session, "transaction body", transactionData.data(), + if (status_t status = rpcRec(connection, session, "transaction body", transactionData.data(), transactionData.size()); status != OK) return status; - return processTransactInternal(fd, session, std::move(transactionData), nullptr /*targetRef*/); + return processTransactInternal(connection, session, std::move(transactionData)); } static void do_nothing_to_transact_data(Parcel* p, const uint8_t* data, size_t dataSize, @@ -575,8 +640,15 @@ static void do_nothing_to_transact_data(Parcel* p, const uint8_t* data, size_t d (void)objectsCount; } -status_t RpcState::processTransactInternal(const base::unique_fd& fd, const sp<RpcSession>& session, - CommandData transactionData, sp<IBinder>&& targetRef) { +status_t RpcState::processTransactInternal(const sp<RpcSession::RpcConnection>& connection, + const sp<RpcSession>& session, + CommandData transactionData) { + // for 'recursive' calls to this, we have already read and processed the + // binder from the transaction data and taken reference counts into account, + // so it is cached here. + sp<IBinder> targetRef; +processTransactInternalTailCall: + if (transactionData.size() < sizeof(RpcWireTransaction)) { ALOGE("Expecting %zu but got %zu bytes for RpcWireTransaction. Terminating!", sizeof(RpcWireTransaction), transactionData.size()); @@ -588,6 +660,7 @@ status_t RpcState::processTransactInternal(const base::unique_fd& fd, const sp<R // TODO(b/182939933): heap allocation just for lookup in mNodeForAddress, // maybe add an RpcAddress 'view' if the type remains 'heavy' auto addr = RpcAddress::fromRawEmbedded(&transaction->address); + bool oneway = transaction->flags & IBinder::FLAG_ONEWAY; status_t replyStatus = OK; sp<IBinder> target; @@ -616,7 +689,7 @@ status_t RpcState::processTransactInternal(const base::unique_fd& fd, const sp<R addr.toString().c_str()); (void)session->shutdownAndWait(false); replyStatus = BAD_VALUE; - } else if (transaction->flags & IBinder::FLAG_ONEWAY) { + } else if (oneway) { std::unique_lock<std::mutex> _l(mNodeMutex); auto it = mNodeForAddress.find(addr); if (it->second.binder.promote() != target) { @@ -673,7 +746,12 @@ status_t RpcState::processTransactInternal(const base::unique_fd& fd, const sp<R data.markForRpc(session); if (target) { + bool origAllowNested = connection->allowNested; + connection->allowNested = !oneway; + replyStatus = target->transact(transaction->code, data, &reply, transaction->flags); + + connection->allowNested = origAllowNested; } else { LOG_RPC_DETAIL("Got special transaction %u", transaction->code); @@ -684,13 +762,13 @@ status_t RpcState::processTransactInternal(const base::unique_fd& fd, const sp<R } case RPC_SPECIAL_TRANSACT_GET_SESSION_ID: { // for client connections, this should always report the value - // originally returned from the server - int32_t id = session->mId.value(); - replyStatus = reply.writeInt32(id); + // originally returned from the server, so this is asserting + // that it exists + replyStatus = session->mId.value().writeToParcel(&reply); break; } default: { - sp<RpcServer> server = session->server().promote(); + sp<RpcServer> server = session->server(); if (server) { switch (transaction->code) { case RPC_SPECIAL_TRANSACT_GET_ROOT: { @@ -709,7 +787,7 @@ status_t RpcState::processTransactInternal(const base::unique_fd& fd, const sp<R } } - if (transaction->flags & IBinder::FLAG_ONEWAY) { + if (oneway) { if (replyStatus != OK) { ALOGW("Oneway call failed with error: %d", replyStatus); } @@ -749,13 +827,12 @@ status_t RpcState::processTransactInternal(const base::unique_fd& fd, const sp<R // - gotta go fast auto& todo = const_cast<BinderNode::AsyncTodo&>(it->second.asyncTodo.top()); - CommandData nextData = std::move(todo.data); - sp<IBinder> nextRef = std::move(todo.ref); + // reset up arguments + transactionData = std::move(todo.data); + targetRef = std::move(todo.ref); it->second.asyncTodo.pop(); - _l.unlock(); - return processTransactInternal(fd, session, std::move(nextData), - std::move(nextRef)); + goto processTransactInternalTailCall; } } return OK; @@ -783,11 +860,11 @@ status_t RpcState::processTransactInternal(const base::unique_fd& fd, const sp<R memcpy(replyData.data() + sizeof(RpcWireHeader) + sizeof(RpcWireReply), reply.data(), reply.dataSize()); - return rpcSend(fd, session, "reply", replyData.data(), replyData.size()); + return rpcSend(connection, session, "reply", replyData.data(), replyData.size()); } -status_t RpcState::processDecStrong(const base::unique_fd& fd, const sp<RpcSession>& session, - const RpcWireHeader& command) { +status_t RpcState::processDecStrong(const sp<RpcSession::RpcConnection>& connection, + const sp<RpcSession>& session, const RpcWireHeader& command) { LOG_ALWAYS_FATAL_IF(command.command != RPC_COMMAND_DEC_STRONG, "command: %d", command.command); CommandData commandData(command.bodySize); @@ -795,7 +872,7 @@ status_t RpcState::processDecStrong(const base::unique_fd& fd, const sp<RpcSessi return NO_MEMORY; } if (status_t status = - rpcRec(fd, session, "dec ref body", commandData.data(), commandData.size()); + rpcRec(connection, session, "dec ref body", commandData.data(), commandData.size()); status != OK) return status; diff --git a/libs/binder/RpcState.h b/libs/binder/RpcState.h index 13c31154eb..529dee534c 100644 --- a/libs/binder/RpcState.h +++ b/libs/binder/RpcState.h @@ -51,31 +51,37 @@ public: RpcState(); ~RpcState(); + status_t sendConnectionInit(const sp<RpcSession::RpcConnection>& connection, + const sp<RpcSession>& session); + status_t readConnectionInit(const sp<RpcSession::RpcConnection>& connection, + const sp<RpcSession>& session); + // TODO(b/182940634): combine some special transactions into one "getServerInfo" call? - sp<IBinder> getRootObject(const base::unique_fd& fd, const sp<RpcSession>& session); - status_t getMaxThreads(const base::unique_fd& fd, const sp<RpcSession>& session, - size_t* maxThreadsOut); - status_t getSessionId(const base::unique_fd& fd, const sp<RpcSession>& session, - int32_t* sessionIdOut); - - [[nodiscard]] status_t transact(const base::unique_fd& fd, const sp<IBinder>& address, - uint32_t code, const Parcel& data, + sp<IBinder> getRootObject(const sp<RpcSession::RpcConnection>& connection, + const sp<RpcSession>& session); + status_t getMaxThreads(const sp<RpcSession::RpcConnection>& connection, + const sp<RpcSession>& session, size_t* maxThreadsOut); + status_t getSessionId(const sp<RpcSession::RpcConnection>& connection, + const sp<RpcSession>& session, RpcAddress* sessionIdOut); + + [[nodiscard]] status_t transact(const sp<RpcSession::RpcConnection>& connection, + const sp<IBinder>& address, uint32_t code, const Parcel& data, const sp<RpcSession>& session, Parcel* reply, uint32_t flags); - [[nodiscard]] status_t transactAddress(const base::unique_fd& fd, const RpcAddress& address, - uint32_t code, const Parcel& data, - const sp<RpcSession>& session, Parcel* reply, - uint32_t flags); - [[nodiscard]] status_t sendDecStrong(const base::unique_fd& fd, const sp<RpcSession>& session, - const RpcAddress& address); + [[nodiscard]] status_t transactAddress(const sp<RpcSession::RpcConnection>& connection, + const RpcAddress& address, uint32_t code, + const Parcel& data, const sp<RpcSession>& session, + Parcel* reply, uint32_t flags); + [[nodiscard]] status_t sendDecStrong(const sp<RpcSession::RpcConnection>& connection, + const sp<RpcSession>& session, const RpcAddress& address); enum class CommandType { ANY, CONTROL_ONLY, }; - [[nodiscard]] status_t getAndExecuteCommand(const base::unique_fd& fd, + [[nodiscard]] status_t getAndExecuteCommand(const sp<RpcSession::RpcConnection>& connection, const sp<RpcSession>& session, CommandType type); - [[nodiscard]] status_t drainCommands(const base::unique_fd& fd, const sp<RpcSession>& session, - CommandType type); + [[nodiscard]] status_t drainCommands(const sp<RpcSession::RpcConnection>& connection, + const sp<RpcSession>& session, CommandType type); /** * Called by Parcel for outgoing binders. This implies one refcount of @@ -130,23 +136,25 @@ private: size_t mSize; }; - [[nodiscard]] status_t rpcSend(const base::unique_fd& fd, const sp<RpcSession>& session, - const char* what, const void* data, size_t size); - [[nodiscard]] status_t rpcRec(const base::unique_fd& fd, const sp<RpcSession>& session, - const char* what, void* data, size_t size); - - [[nodiscard]] status_t waitForReply(const base::unique_fd& fd, const sp<RpcSession>& session, - Parcel* reply); - [[nodiscard]] status_t processServerCommand(const base::unique_fd& fd, - const sp<RpcSession>& session, - const RpcWireHeader& command, CommandType type); - [[nodiscard]] status_t processTransact(const base::unique_fd& fd, const sp<RpcSession>& session, + [[nodiscard]] status_t rpcSend(const sp<RpcSession::RpcConnection>& connection, + const sp<RpcSession>& session, const char* what, + const void* data, size_t size); + [[nodiscard]] status_t rpcRec(const sp<RpcSession::RpcConnection>& connection, + const sp<RpcSession>& session, const char* what, void* data, + size_t size); + + [[nodiscard]] status_t waitForReply(const sp<RpcSession::RpcConnection>& connection, + const sp<RpcSession>& session, Parcel* reply); + [[nodiscard]] status_t processCommand(const sp<RpcSession::RpcConnection>& connection, + const sp<RpcSession>& session, + const RpcWireHeader& command, CommandType type); + [[nodiscard]] status_t processTransact(const sp<RpcSession::RpcConnection>& connection, + const sp<RpcSession>& session, const RpcWireHeader& command); - [[nodiscard]] status_t processTransactInternal(const base::unique_fd& fd, + [[nodiscard]] status_t processTransactInternal(const sp<RpcSession::RpcConnection>& connection, const sp<RpcSession>& session, - CommandData transactionData, - sp<IBinder>&& targetRef); - [[nodiscard]] status_t processDecStrong(const base::unique_fd& fd, + CommandData transactionData); + [[nodiscard]] status_t processDecStrong(const sp<RpcSession::RpcConnection>& connection, const sp<RpcSession>& session, const RpcWireHeader& command); diff --git a/libs/binder/RpcWireFormat.h b/libs/binder/RpcWireFormat.h index 649c1eeb8e..2016483138 100644 --- a/libs/binder/RpcWireFormat.h +++ b/libs/binder/RpcWireFormat.h @@ -20,16 +20,38 @@ namespace android { #pragma clang diagnostic push #pragma clang diagnostic error "-Wpadded" -constexpr int32_t RPC_SESSION_ID_NEW = -1; - enum : uint8_t { RPC_CONNECTION_OPTION_REVERSE = 0x1, }; +constexpr uint64_t RPC_WIRE_ADDRESS_OPTION_CREATED = 1 << 0; // distinguish from '0' address +constexpr uint64_t RPC_WIRE_ADDRESS_OPTION_FOR_SERVER = 1 << 1; + +struct RpcWireAddress { + uint64_t options; + uint8_t address[32]; +}; + +/** + * This is sent to an RpcServer in order to request a new connection is created, + * either as part of a new session or an existing session + */ struct RpcConnectionHeader { - int32_t sessionId; + RpcWireAddress sessionId; uint8_t options; - uint8_t reserved[3]; + uint8_t reserved[7]; +}; + +#define RPC_CONNECTION_INIT_OKAY "cci" + +/** + * Whenever a client connection is setup, this is sent as the initial + * transaction. The main use of this is in order to control the timing for when + * a reverse connection is setup. + */ +struct RpcOutgoingConnectionInit { + char msg[4]; + uint8_t reserved[4]; }; enum : uint32_t { @@ -73,10 +95,6 @@ struct RpcWireHeader { uint32_t reserved[2]; }; -struct RpcWireAddress { - uint8_t address[32]; -}; - struct RpcWireTransaction { RpcWireAddress address; uint32_t code; diff --git a/libs/binder/TEST_MAPPING b/libs/binder/TEST_MAPPING index b58d919d33..59f0ba6d26 100644 --- a/libs/binder/TEST_MAPPING +++ b/libs/binder/TEST_MAPPING @@ -31,6 +31,9 @@ "name": "binderStabilityTest" }, { + "name": "binderUtilsTest" + }, + { "name": "libbinder_ndk_unit_test" }, { diff --git a/libs/binder/UtilsHost.cpp b/libs/binder/UtilsHost.cpp new file mode 100644 index 0000000000..e524dabf7b --- /dev/null +++ b/libs/binder/UtilsHost.cpp @@ -0,0 +1,177 @@ +/* + * Copyright (C) 2021 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. + */ + +#include "UtilsHost.h" + +#include <poll.h> +#include <string.h> +#include <sys/types.h> +#include <sys/wait.h> + +#include <sstream> + +#include <log/log.h> + +namespace android { + +CommandResult::~CommandResult() { + if (!pid.has_value()) return; + if (*pid == 0) { + ALOGW("%s: PID is unexpectedly 0, won't kill it", __PRETTY_FUNCTION__); + return; + } + + ALOGE_IF(kill(*pid, SIGKILL) != 0, "kill(%d): %s", *pid, strerror(errno)); + + while (pid.has_value()) { + int status; + LOG_HOST("%s: Waiting for PID %d to exit.", __PRETTY_FUNCTION__, *pid); + int waitres = waitpid(*pid, &status, 0); + if (waitres == -1) { + ALOGE("%s: waitpid(%d): %s", __PRETTY_FUNCTION__, *pid, strerror(errno)); + break; + } + if (WIFEXITED(status)) { + LOG_HOST("%s: PID %d exited.", __PRETTY_FUNCTION__, *pid); + pid.reset(); + } else if (WIFSIGNALED(status)) { + LOG_HOST("%s: PID %d terminated by signal %d.", __PRETTY_FUNCTION__, *pid, + WTERMSIG(status)); + pid.reset(); + } else if (WIFSTOPPED(status)) { + ALOGW("%s: pid %d stopped", __PRETTY_FUNCTION__, *pid); + } else if (WIFCONTINUED(status)) { + ALOGW("%s: pid %d continued", __PRETTY_FUNCTION__, *pid); + } + } +} + +std::ostream& operator<<(std::ostream& os, const CommandResult& res) { + if (res.exitCode) os << "code=" << *res.exitCode; + if (res.signal) os << "signal=" << *res.signal; + if (res.pid) os << ", pid=" << *res.pid; + return os << ", stdout=" << res.stdout << ", stderr=" << res.stderr; +} + +std::string CommandResult::toString() const { + std::stringstream ss; + ss << (*this); + return ss.str(); +} + +android::base::Result<CommandResult> execute(std::vector<std::string> argStringVec, + const std::function<bool(const CommandResult&)>& end) { + // turn vector<string> into null-terminated char* vector. + std::vector<char*> argv; + argv.reserve(argStringVec.size() + 1); + for (auto& arg : argStringVec) argv.push_back(arg.data()); + argv.push_back(nullptr); + + CommandResult ret; + android::base::unique_fd outWrite; + if (!android::base::Pipe(&ret.outPipe, &outWrite)) + return android::base::ErrnoError() << "pipe() for outPipe"; + android::base::unique_fd errWrite; + if (!android::base::Pipe(&ret.errPipe, &errWrite)) + return android::base::ErrnoError() << "pipe() for errPipe"; + + int pid = fork(); + if (pid == -1) return android::base::ErrnoError() << "fork()"; + if (pid == 0) { + // child + ret.outPipe.reset(); + ret.errPipe.reset(); + + int res = TEMP_FAILURE_RETRY(dup2(outWrite.get(), STDOUT_FILENO)); + LOG_ALWAYS_FATAL_IF(-1 == res, "dup2(outPipe): %s", strerror(errno)); + outWrite.reset(); + + res = TEMP_FAILURE_RETRY(dup2(errWrite.get(), STDERR_FILENO)); + LOG_ALWAYS_FATAL_IF(-1 == res, "dup2(errPipe): %s", strerror(errno)); + errWrite.reset(); + + execvp(argv[0], argv.data()); + LOG_ALWAYS_FATAL("execvp() returns"); + } + // parent + outWrite.reset(); + errWrite.reset(); + ret.pid = pid; + + auto handlePoll = [](android::base::unique_fd* fd, const pollfd& pfd, std::string* s) { + if (!fd->ok()) return true; + if (pfd.revents & POLLIN) { + char buf[1024]; + ssize_t n = TEMP_FAILURE_RETRY(read(fd->get(), buf, sizeof(buf))); + if (n < 0) return false; + if (n > 0) *s += std::string_view(buf, n); + } + if (pfd.revents & POLLHUP) { + fd->reset(); + } + return true; + }; + + // Drain both stdout and stderr. Check end() regularly until both are closed. + while (ret.outPipe.ok() || ret.errPipe.ok()) { + pollfd fds[2]; + pollfd *outPollFd = nullptr, *errPollFd = nullptr; + memset(fds, 0, sizeof(fds)); + nfds_t nfds = 0; + if (ret.outPipe.ok()) { + outPollFd = &fds[nfds++]; + *outPollFd = {.fd = ret.outPipe.get(), .events = POLLIN}; + } + if (ret.errPipe.ok()) { + errPollFd = &fds[nfds++]; + *errPollFd = {.fd = ret.errPipe.get(), .events = POLLIN}; + } + int pollRet = poll(fds, nfds, 1000 /* ms timeout */); + if (pollRet == -1) return android::base::ErrnoError() << "poll()"; + + if (!handlePoll(&ret.outPipe, *outPollFd, &ret.stdout)) + return android::base::ErrnoError() << "read(stdout)"; + if (!handlePoll(&ret.errPipe, *errPollFd, &ret.stderr)) + return android::base::ErrnoError() << "read(stderr)"; + + if (end && end(ret)) return ret; + } + + // If both stdout and stderr are closed by the subprocess, it may or may not be terminated. + while (ret.pid.has_value()) { + int status; + auto exitPid = waitpid(pid, &status, 0); + if (exitPid == -1) return android::base::ErrnoError() << "waitpid(" << pid << ")"; + if (exitPid == pid) { + if (WIFEXITED(status)) { + ret.pid = std::nullopt; + ret.exitCode = WEXITSTATUS(status); + } else if (WIFSIGNALED(status)) { + ret.pid = std::nullopt; + ret.signal = WTERMSIG(status); + } else if (WIFSTOPPED(status)) { + ALOGW("%s: pid %d stopped", __PRETTY_FUNCTION__, *ret.pid); + } else if (WIFCONTINUED(status)) { + ALOGW("%s: pid %d continued", __PRETTY_FUNCTION__, *ret.pid); + } + } + // ret is not changed unless the process is terminated (where pid == nullopt). Hence there + // is no need to check the predicate `end(ret)`. + } + + return ret; +} +} // namespace android diff --git a/libs/binder/UtilsHost.h b/libs/binder/UtilsHost.h new file mode 100644 index 0000000000..0f29f60320 --- /dev/null +++ b/libs/binder/UtilsHost.h @@ -0,0 +1,99 @@ +/* + * Copyright (C) 2021 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. + */ + +#pragma once + +#include <optional> +#include <ostream> +#include <string> +#include <variant> +#include <vector> + +#include <android-base/macros.h> +#include <android-base/result.h> +#include <android-base/unique_fd.h> + +/** + * Log a lot more information about host-device binder communication, when debugging issues. + */ +#define SHOULD_LOG_HOST false + +#if SHOULD_LOG_HOST +#define LOG_HOST(...) ALOGI(__VA_ARGS__) +#else +#define LOG_HOST(...) ALOGV(__VA_ARGS__) // for type checking +#endif + +namespace android { + +struct CommandResult { + std::optional<int32_t> exitCode; + std::optional<int32_t> signal; + std::optional<pid_t> pid; + std::string stdout; + std::string stderr; + + android::base::unique_fd outPipe; + android::base::unique_fd errPipe; + + CommandResult() = default; + CommandResult(CommandResult&& other) noexcept { (*this) = std::move(other); } + CommandResult& operator=(CommandResult&& other) noexcept { + std::swap(exitCode, other.exitCode); + std::swap(signal, other.signal); + std::swap(pid, other.pid); + std::swap(stdout, other.stdout); + std::swap(stderr, other.stderr); + return *this; + } + ~CommandResult(); + [[nodiscard]] std::string toString() const; + + [[nodiscard]] bool stdoutEndsWithNewLine() const { + return !stdout.empty() && stdout.back() == '\n'; + } + +private: + DISALLOW_COPY_AND_ASSIGN(CommandResult); +}; + +std::ostream& operator<<(std::ostream& os, const CommandResult& res); + +// Execute a command using tokens specified in @a argStringVec. +// +// @a end is a predicate checked periodically when the command emits any output to stdout or +// stderr. When it is evaluated to true, the function returns immediately even though +// the child process has not been terminated. The function also assumes that, after @a end +// is evaluated to true, the child process does not emit any other messages. +// If this is not the case, caller to execute() must handle these I/O in the pipes in the returned +// CommandResult object. Otherwise the child program may hang on I/O. +// +// If @a end is nullptr, it is equivalent to a predicate that always returns false. In this +// case, execute() returns after the child process is terminated. +// +// If @a end is evaluated to true, and execute() returns with the child process running, +// the returned CommandResult has pid, outPipe, and errPipe set. In this case, the caller is +// responsible for holding the returned CommandResult. When the CommandResult object is destroyed, +// the child process is killed. +// +// On the other hand, execute() returns with the child process terminated, either exitCode or signal +// is set. +// +// If the parent process has encountered any errors for system calls, return ExecuteError with +// the proper errno set. +android::base::Result<CommandResult> execute(std::vector<std::string> argStringVec, + const std::function<bool(const CommandResult&)>& end); +} // namespace android diff --git a/libs/binder/include/binder/Binder.h b/libs/binder/include/binder/Binder.h index d162dda16c..472e546944 100644 --- a/libs/binder/include/binder/Binder.h +++ b/libs/binder/include/binder/Binder.h @@ -102,7 +102,7 @@ public: void setParceled(); [[nodiscard]] status_t setRpcClientDebug(android::base::unique_fd clientFd, - uint32_t maxRpcThreads); + const sp<IBinder>& keepAliveBinder); protected: virtual ~BBinder(); @@ -117,11 +117,13 @@ private: BBinder(const BBinder& o); BBinder& operator=(const BBinder& o); + class RpcServerLink; class Extras; Extras* getOrCreateExtras(); [[nodiscard]] status_t setRpcClientDebug(const Parcel& data); + void removeRpcServerLink(const sp<RpcServerLink>& link); std::atomic<Extras*> mExtras; diff --git a/libs/binder/include/binder/IBinder.h b/libs/binder/include/binder/IBinder.h index ce28d7c706..f9cdac7de9 100644 --- a/libs/binder/include/binder/IBinder.h +++ b/libs/binder/include/binder/IBinder.h @@ -157,22 +157,22 @@ public: * Set the RPC client fd to this binder service, for debugging. This is only available on * debuggable builds. * - * |maxRpcThreads| must be positive because RPC is useless without threads. - * * When this is called on a binder service, the service: * 1. sets up RPC server * 2. spawns 1 new thread that calls RpcServer::join() - * - join() spawns at most |maxRpcThreads| threads that accept() connections; see RpcServer + * - join() spawns some number of threads that accept() connections; see RpcServer * - * setRpcClientDebug() may only be called once. - * TODO(b/182914638): If allow to shut down the client, addRpcClient can be called repeatedly. + * setRpcClientDebug() may be called multiple times. Each call will add a new RpcServer + * and opens up a TCP port. * * Note: A thread is spawned for each accept()'ed fd, which may call into functions of the * interface freely. See RpcServer::join(). To avoid such race conditions, implement the service * functions with multithreading support. + * + * On death of @a keepAliveBinder, the RpcServer shuts down. */ [[nodiscard]] status_t setRpcClientDebug(android::base::unique_fd socketFd, - uint32_t maxRpcThreads); + const sp<IBinder>& keepAliveBinder); // NOLINTNEXTLINE(google-default-arguments) virtual status_t transact( uint32_t code, diff --git a/libs/binder/include/binder/RpcAddress.h b/libs/binder/include/binder/RpcAddress.h index 5a3f3a6afa..e428908c88 100644 --- a/libs/binder/include/binder/RpcAddress.h +++ b/libs/binder/include/binder/RpcAddress.h @@ -29,11 +29,7 @@ class Parcel; struct RpcWireAddress; /** - * This class represents an identifier of a binder object. - * - * The purpose of this class it to hide the ABI of an RpcWireAddress, and - * potentially allow us to change the size of it in the future (RpcWireAddress - * is PIMPL, essentially - although the type that is used here is not exposed). + * This class represents an identifier across an RPC boundary. */ class RpcAddress { public: @@ -46,9 +42,20 @@ public: bool isZero() const; /** - * Create a new address which is unique + * Create a new random address. + */ + static RpcAddress random(bool forServer); + + /** + * Whether this address was created with 'bool forServer' true + */ + bool isForServer() const; + + /** + * Whether this address is one that could be created with this version of + * libbinder. */ - static RpcAddress unique(); + bool isRecognizedType() const; /** * Creates a new address as a copy of an embedded object. diff --git a/libs/binder/include/binder/RpcServer.h b/libs/binder/include/binder/RpcServer.h index b88bf5091b..c8d2857347 100644 --- a/libs/binder/include/binder/RpcServer.h +++ b/libs/binder/include/binder/RpcServer.h @@ -17,6 +17,7 @@ #include <android-base/unique_fd.h> #include <binder/IBinder.h> +#include <binder/RpcAddress.h> #include <binder/RpcSession.h> #include <utils/Errors.h> #include <utils/RefBase.h> @@ -97,7 +98,7 @@ public: * * If this is not specified, this will be a single-threaded server. * - * TODO(b/185167543): these are currently created per client, but these + * TODO(b/167966510): these are currently created per client, but these * should be shared. */ void setMaxThreads(size_t threads); @@ -155,8 +156,8 @@ private: friend sp<RpcServer>; RpcServer(); - void onSessionLockedAllServerThreadsEnded(const sp<RpcSession>& session) override; - void onSessionServerThreadEnded() override; + void onSessionLockedAllIncomingThreadsEnded(const sp<RpcSession>& session) override; + void onSessionIncomingThreadEnded() override; static void establishConnection(sp<RpcServer>&& server, base::unique_fd clientFd); bool setupSocketServer(const RpcSocketAddress& address); @@ -171,8 +172,7 @@ private: std::map<std::thread::id, std::thread> mConnectingThreads; sp<IBinder> mRootObject; wp<IBinder> mRootObjectWeak; - std::map<int32_t, sp<RpcSession>> mSessions; - int32_t mSessionIdCounter = 0; + std::map<RpcAddress, sp<RpcSession>> mSessions; std::unique_ptr<RpcSession::FdTrigger> mShutdownTrigger; std::condition_variable mShutdownCv; }; diff --git a/libs/binder/include/binder/RpcSession.h b/libs/binder/include/binder/RpcSession.h index 7aa6d021e7..69c2a1a956 100644 --- a/libs/binder/include/binder/RpcSession.h +++ b/libs/binder/include/binder/RpcSession.h @@ -54,7 +54,7 @@ public: * If this is called, 'shutdown' on this session must also be called. * Otherwise, a threadpool will leak. * - * TODO(b/185167543): start these dynamically + * TODO(b/189955605): start these dynamically */ void setMaxThreads(size_t threads); size_t getMaxThreads(); @@ -118,7 +118,11 @@ public: ~RpcSession(); - wp<RpcServer> server(); + /** + * Server if this session is created as part of a server (symmetrical to + * client servers). Otherwise, nullptr. + */ + sp<RpcServer> server(); // internal only const std::unique_ptr<RpcState>& state() { return mState; } @@ -170,14 +174,14 @@ private: class EventListener : public virtual RefBase { public: - virtual void onSessionLockedAllServerThreadsEnded(const sp<RpcSession>& session) = 0; - virtual void onSessionServerThreadEnded() = 0; + virtual void onSessionLockedAllIncomingThreadsEnded(const sp<RpcSession>& session) = 0; + virtual void onSessionIncomingThreadEnded() = 0; }; class WaitForShutdownListener : public EventListener { public: - void onSessionLockedAllServerThreadsEnded(const sp<RpcSession>& session) override; - void onSessionServerThreadEnded() override; + void onSessionLockedAllIncomingThreadsEnded(const sp<RpcSession>& session) override; + void onSessionIncomingThreadEnded() override; void waitForShutdown(std::unique_lock<std::mutex>& lock); private: @@ -185,30 +189,46 @@ private: bool mShutdown = false; }; - status_t readId(); - - // transfer ownership of thread - void preJoin(std::thread thread); - // join on thread passed to preJoin - static void join(sp<RpcSession>&& session, base::unique_fd client); - struct RpcConnection : public RefBase { base::unique_fd fd; // whether this or another thread is currently using this fd to make // or receive transactions. std::optional<pid_t> exclusiveTid; + + bool allowNested = false; }; + status_t readId(); + + // A thread joining a server must always call these functions in order, and + // cleanup is only programmed once into join. These are in separate + // functions in order to allow for different locks to be taken during + // different parts of setup. + // + // transfer ownership of thread (usually done while a lock is taken on the + // structure which originally owns the thread) + void preJoinThreadOwnership(std::thread thread); + // pass FD to thread and read initial connection information + struct PreJoinSetupResult { + // Server connection object associated with this + sp<RpcConnection> connection; + // Status of setup + status_t status; + }; + PreJoinSetupResult preJoinSetup(base::unique_fd fd); + // join on thread passed to preJoinThreadOwnership + static void join(sp<RpcSession>&& session, PreJoinSetupResult&& result); + [[nodiscard]] bool setupSocketClient(const RpcSocketAddress& address); - [[nodiscard]] bool setupOneSocketConnection(const RpcSocketAddress& address, int32_t sessionId, - bool server); - [[nodiscard]] bool addClientConnection(base::unique_fd fd); + [[nodiscard]] bool setupOneSocketConnection(const RpcSocketAddress& address, + const RpcAddress& sessionId, bool server); + [[nodiscard]] bool addOutgoingConnection(base::unique_fd fd, bool init); [[nodiscard]] bool setForServer(const wp<RpcServer>& server, const wp<RpcSession::EventListener>& eventListener, - int32_t sessionId); - sp<RpcConnection> assignServerToThisThread(base::unique_fd fd); - [[nodiscard]] bool removeServerConnection(const sp<RpcConnection>& connection); + const RpcAddress& sessionId); + sp<RpcConnection> assignIncomingConnectionToThisThread(base::unique_fd fd); + [[nodiscard]] bool removeIncomingConnection(const sp<RpcConnection>& connection); enum class ConnectionUse { CLIENT, @@ -216,12 +236,14 @@ private: CLIENT_REFCOUNT, }; - // RAII object for session connection + // Object representing exclusive access to a connection. class ExclusiveConnection { public: - explicit ExclusiveConnection(const sp<RpcSession>& session, ConnectionUse use); + static status_t find(const sp<RpcSession>& session, ConnectionUse use, + ExclusiveConnection* connection); + ~ExclusiveConnection(); - const base::unique_fd& fd() { return mConnection->fd; } + const sp<RpcConnection>& get() { return mConnection; } private: static void findConnection(pid_t tid, sp<RpcConnection>* exclusive, @@ -238,13 +260,13 @@ private: bool mReentrant = false; }; - // On the other side of a session, for each of mClientConnections here, there should - // be one of mServerConnections on the other side (and vice versa). + // On the other side of a session, for each of mOutgoingConnections here, there should + // be one of mIncomingConnections on the other side (and vice versa). // // For the simplest session, a single server with one client, you would // have: - // - the server has a single 'mServerConnections' and a thread listening on this - // - the client has a single 'mClientConnections' and makes calls to this + // - the server has a single 'mIncomingConnections' and a thread listening on this + // - the client has a single 'mOutgoingConnections' and makes calls to this // - here, when the client makes a call, the server can call back into it // (nested calls), but outside of this, the client will only ever read // calls from the server when it makes a call itself. @@ -256,8 +278,7 @@ private: sp<WaitForShutdownListener> mShutdownListener; // used for client sessions wp<EventListener> mEventListener; // mForServer if server, mShutdownListener if client - // TODO(b/183988761): this shouldn't be guessable - std::optional<int32_t> mId; + std::optional<RpcAddress> mId; std::unique_ptr<FdTrigger> mShutdownTrigger; @@ -270,12 +291,9 @@ private: std::condition_variable mAvailableConnectionCv; // for mWaitingThreads size_t mWaitingThreads = 0; // hint index into clients, ++ when sending an async transaction - size_t mClientConnectionsOffset = 0; - std::vector<sp<RpcConnection>> mClientConnections; - std::vector<sp<RpcConnection>> mServerConnections; - - // TODO(b/185167543): allow sharing between different sessions in a - // process? (or combine with mServerConnections) + size_t mOutgoingConnectionsOffset = 0; + std::vector<sp<RpcConnection>> mOutgoingConnections; + std::vector<sp<RpcConnection>> mIncomingConnections; std::map<std::thread::id, std::thread> mThreads; }; diff --git a/libs/binder/libbinder_rpc_unstable.cpp b/libs/binder/libbinder_rpc_unstable.cpp new file mode 100644 index 0000000000..68ec669c31 --- /dev/null +++ b/libs/binder/libbinder_rpc_unstable.cpp @@ -0,0 +1,50 @@ +/* + * Copyright (C) 2021 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. + */ + +#include <android-base/logging.h> +#include <android/binder_libbinder.h> +#include <binder/RpcServer.h> +#include <binder/RpcSession.h> + +using android::RpcServer; +using android::RpcSession; + +extern "C" { + +bool RunRpcServer(AIBinder* service, unsigned int port) { + auto server = RpcServer::make(); + server->iUnderstandThisCodeIsExperimentalAndIWillNotUseItInProduction(); + if (!server->setupVsockServer(port)) { + LOG(ERROR) << "Failed to set up vsock server with port " << port; + return false; + } + server->setRootObject(AIBinder_toPlatformBinder(service)); + server->join(); + + // Shutdown any open sessions since server failed. + (void)server->shutdown(); + return true; +} + +AIBinder* RpcClient(unsigned int cid, unsigned int port) { + auto session = RpcSession::make(); + if (!session->setupVsockClient(cid, port)) { + LOG(ERROR) << "Failed to set up vsock client with CID " << cid << " and port " << port; + return nullptr; + } + return AIBinder_fromPlatformBinder(session->getRootObject()); +} +} diff --git a/libs/binder/libbinder_rpc_unstable.map.txt b/libs/binder/libbinder_rpc_unstable.map.txt new file mode 100644 index 0000000000..3921a4dde8 --- /dev/null +++ b/libs/binder/libbinder_rpc_unstable.map.txt @@ -0,0 +1,7 @@ +LIBBINDER_RPC_UNSTABLE_SHIM { # platform-only + global: + RunRpcServer; + RpcClient; + local: + *; +}; diff --git a/libs/binder/ndk/include_platform/android/binder_parcel_platform.h b/libs/binder/ndk/include_platform/android/binder_parcel_platform.h index 6372449716..b24094ef16 100644 --- a/libs/binder/ndk/include_platform/android/binder_parcel_platform.h +++ b/libs/binder/ndk/include_platform/android/binder_parcel_platform.h @@ -33,7 +33,6 @@ bool AParcel_getAllowFds(const AParcel*); #endif -#if !defined(__ANDROID_APEX__) /** * Data written to the parcel will be zero'd before being deleted or realloced. * @@ -44,6 +43,5 @@ bool AParcel_getAllowFds(const AParcel*); * \param parcel The parcel to clear associated data from. */ void AParcel_markSensitive(const AParcel* parcel); -#endif __END_DECLS diff --git a/libs/binder/ndk/libbinder_ndk.map.txt b/libs/binder/ndk/libbinder_ndk.map.txt index 7d4b82e4b6..685ebb5023 100644 --- a/libs/binder/ndk/libbinder_ndk.map.txt +++ b/libs/binder/ndk/libbinder_ndk.map.txt @@ -117,7 +117,7 @@ LIBBINDER_NDK31 { # introduced=31 ABinderProcess_setupPolling; # apex AIBinder_getCallingSid; # apex AIBinder_setRequestingSid; # apex - AParcel_markSensitive; # llndk + AParcel_markSensitive; # systemapi llndk AServiceManager_forEachDeclaredInstance; # apex llndk AServiceManager_forceLazyServicesPersist; # llndk AServiceManager_isDeclared; # apex llndk diff --git a/libs/binder/ndk/tests/Android.bp b/libs/binder/ndk/tests/Android.bp index ede4873dd6..488009f812 100644 --- a/libs/binder/ndk/tests/Android.bp +++ b/libs/binder/ndk/tests/Android.bp @@ -73,7 +73,10 @@ cc_test { "IBinderNdkUnitTest-cpp", "IBinderNdkUnitTest-ndk_platform", ], - test_suites: ["general-tests", "vts"], + test_suites: [ + "general-tests", + "vts", + ], require_root: true, } @@ -115,4 +118,12 @@ aidl_interface { "IBinderNdkUnitTest.aidl", "IEmpty.aidl", ], + backend: { + java: { + enabled: false, + }, + ndk: { + apps_enabled: false, + }, + }, } diff --git a/libs/binder/ndk/tests/IBinderNdkUnitTest.aidl b/libs/binder/ndk/tests/IBinderNdkUnitTest.aidl index ecbd6490e9..a626d39d91 100644 --- a/libs/binder/ndk/tests/IBinderNdkUnitTest.aidl +++ b/libs/binder/ndk/tests/IBinderNdkUnitTest.aidl @@ -21,6 +21,7 @@ import IEmpty; +@SensitiveData interface IBinderNdkUnitTest { int repeatInt(int a); diff --git a/libs/binder/rust/Android.bp b/libs/binder/rust/Android.bp index 7d655d8c3e..8d27eedb57 100644 --- a/libs/binder/rust/Android.bp +++ b/libs/binder/rust/Android.bp @@ -106,6 +106,21 @@ rust_bindgen { ], } +// TODO(b/184872979): remove once the Rust API is created. +rust_bindgen { + name: "libbinder_rpc_unstable_bindgen", + wrapper_src: "src/binder_rpc_unstable.hpp", + crate_name: "binder_rpc_unstable_bindgen", + source_stem: "bindings", + shared_libs: [ + "libutils", + ], + apex_available: [ + "com.android.compos", + "com.android.virt", + ], +} + rust_test { name: "libbinder_rs-internal_test", crate_name: "binder", diff --git a/libs/binder/rust/src/binder.rs b/libs/binder/rust/src/binder.rs index 695a83e414..2a09afc6dc 100644 --- a/libs/binder/rust/src/binder.rs +++ b/libs/binder/rust/src/binder.rs @@ -25,6 +25,7 @@ use std::borrow::Borrow; use std::cmp::Ordering; use std::ffi::{c_void, CStr, CString}; use std::fmt; +use std::fs::File; use std::marker::PhantomData; use std::ops::Deref; use std::os::raw::c_char; @@ -54,6 +55,14 @@ pub trait Interface: Send { fn as_binder(&self) -> SpIBinder { panic!("This object was not a Binder object and cannot be converted into an SpIBinder.") } + + /// Dump transaction handler for this Binder object. + /// + /// This handler is a no-op by default and should be implemented for each + /// Binder service struct that wishes to respond to dump transactions. + fn dump(&self, _file: &File, _args: &[&CStr]) -> Result<()> { + Ok(()) + } } /// Interface stability promise @@ -98,6 +107,10 @@ pub trait Remotable: Send + Sync { /// `reply` may be [`None`] if the sender does not expect a reply. fn on_transact(&self, code: TransactionCode, data: &Parcel, reply: &mut Parcel) -> Result<()>; + /// Handle a request to invoke the dump transaction on this + /// object. + fn on_dump(&self, file: &File, args: &[&CStr]) -> Result<()>; + /// Retrieve the class of this remote object. /// /// This method should always return the same InterfaceClass for the same @@ -218,7 +231,7 @@ impl InterfaceClass { if class.is_null() { panic!("Expected non-null class pointer from AIBinder_Class_define!"); } - sys::AIBinder_Class_setOnDump(class, None); + sys::AIBinder_Class_setOnDump(class, Some(I::on_dump)); sys::AIBinder_Class_setHandleShellCommand(class, None); class }; @@ -492,6 +505,16 @@ pub trait InterfaceClassMethods { /// returned by `on_create` for this class. This function takes ownership of /// the provided pointer and destroys it. unsafe extern "C" fn on_destroy(object: *mut c_void); + + /// Called to handle the `dump` transaction. + /// + /// # Safety + /// + /// Must be called with a non-null, valid pointer to a local `AIBinder` that + /// contains a `T` pointer in its user data. fd should be a non-owned file + /// descriptor, and args must be an array of null-terminated string + /// poiinters with length num_args. + unsafe extern "C" fn on_dump(binder: *mut sys::AIBinder, fd: i32, args: *mut *const c_char, num_args: u32) -> status_t; } /// Interface for transforming a generic SpIBinder into a specific remote @@ -778,6 +801,10 @@ macro_rules! declare_binder_interface { } } + fn on_dump(&self, file: &std::fs::File, args: &[&std::ffi::CStr]) -> $crate::Result<()> { + self.0.dump(file, args) + } + fn get_class() -> $crate::InterfaceClass { static CLASS_INIT: std::sync::Once = std::sync::Once::new(); static mut CLASS: Option<$crate::InterfaceClass> = None; diff --git a/libs/binder/rust/src/binder_rpc_unstable.hpp b/libs/binder/rust/src/binder_rpc_unstable.hpp new file mode 100644 index 0000000000..7932d0f29c --- /dev/null +++ b/libs/binder/rust/src/binder_rpc_unstable.hpp @@ -0,0 +1,26 @@ +/* + * Copyright (C) 2021 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. + */ + +#pragma once + +extern "C" { + +struct AIBinder; + +bool RunRpcServer(AIBinder* service, unsigned int port); +AIBinder* RpcClient(unsigned int cid, unsigned int port); + +} diff --git a/libs/binder/rust/src/lib.rs b/libs/binder/rust/src/lib.rs index 7c0584bade..cb330a65e5 100644 --- a/libs/binder/rust/src/lib.rs +++ b/libs/binder/rust/src/lib.rs @@ -119,6 +119,13 @@ pub use proxy::{get_interface, get_service, wait_for_interface, wait_for_service pub use proxy::{AssociateClass, DeathRecipient, Proxy, SpIBinder, WpIBinder}; pub use state::{ProcessState, ThreadState}; +/// Unstable, in-development API that only allowlisted clients are allowed to use. +pub mod unstable_api { + pub use crate::binder::AsNative; + pub use crate::proxy::unstable_api::new_spibinder; + pub use crate::sys::AIBinder; +} + /// The public API usable outside AIDL-generated interface crates. pub mod public_api { pub use super::parcel::ParcelFileDescriptor; diff --git a/libs/binder/rust/src/native.rs b/libs/binder/rust/src/native.rs index 3b3fd08cdc..5e324b3f5b 100644 --- a/libs/binder/rust/src/native.rs +++ b/libs/binder/rust/src/native.rs @@ -21,10 +21,13 @@ use crate::proxy::SpIBinder; use crate::sys; use std::convert::TryFrom; -use std::ffi::{c_void, CString}; +use std::ffi::{c_void, CStr, CString}; +use std::fs::File; use std::mem::ManuallyDrop; use std::ops::Deref; -use std::ptr; +use std::os::raw::c_char; +use std::os::unix::io::FromRawFd; +use std::slice; /// Rust wrapper around Binder remotable objects. /// @@ -273,7 +276,7 @@ impl<T: Remotable> InterfaceClassMethods for Binder<T> { /// Must be called with a valid pointer to a `T` object. After this call, /// the pointer will be invalid and should not be dereferenced. unsafe extern "C" fn on_destroy(object: *mut c_void) { - ptr::drop_in_place(object as *mut T) + Box::from_raw(object as *mut T); } /// Called whenever a new, local `AIBinder` object is needed of a specific @@ -290,6 +293,37 @@ impl<T: Remotable> InterfaceClassMethods for Binder<T> { // object created by Box. args } + + /// Called to handle the `dump` transaction. + /// + /// # Safety + /// + /// Must be called with a non-null, valid pointer to a local `AIBinder` that + /// contains a `T` pointer in its user data. fd should be a non-owned file + /// descriptor, and args must be an array of null-terminated string + /// poiinters with length num_args. + unsafe extern "C" fn on_dump(binder: *mut sys::AIBinder, fd: i32, args: *mut *const c_char, num_args: u32) -> status_t { + if fd < 0 { + return StatusCode::UNEXPECTED_NULL as status_t; + } + // We don't own this file, so we need to be careful not to drop it. + let file = ManuallyDrop::new(File::from_raw_fd(fd)); + + if args.is_null() { + return StatusCode::UNEXPECTED_NULL as status_t; + } + let args = slice::from_raw_parts(args, num_args as usize); + let args: Vec<_> = args.iter().map(|s| CStr::from_ptr(*s)).collect(); + + let object = sys::AIBinder_getUserData(binder); + let binder: &T = &*(object as *const T); + let res = binder.on_dump(&file, &args); + + match res { + Ok(()) => 0, + Err(e) => e as status_t, + } + } } impl<T: Remotable> Drop for Binder<T> { @@ -410,6 +444,10 @@ impl Remotable for () { Ok(()) } + fn on_dump(&self, _file: &File, _args: &[&CStr]) -> Result<()> { + Ok(()) + } + binder_fn_get_class!(Binder::<Self>); } diff --git a/libs/binder/rust/src/parcel.rs b/libs/binder/rust/src/parcel.rs index 6c34824a5e..a3f7620474 100644 --- a/libs/binder/rust/src/parcel.rs +++ b/libs/binder/rust/src/parcel.rs @@ -184,11 +184,17 @@ impl Parcel { } } + /// Returns the total size of the parcel. + pub fn get_data_size(&self) -> i32 { + unsafe { + // Safety: `Parcel` always contains a valid pointer to an `AParcel`, + // and this call is otherwise safe. + sys::AParcel_getDataSize(self.as_native()) + } + } + /// Move the current read/write position in the parcel. /// - /// The new position must be a position previously returned by - /// `self.get_data_position()`. - /// /// # Safety /// /// This method is safe if `pos` is less than the current size of the parcel @@ -219,6 +225,72 @@ impl Parcel { D::deserialize(self) } + /// Attempt to read a type that implements [`Deserialize`] from this + /// `Parcel` onto an existing value. This operation will overwrite the old + /// value partially or completely, depending on how much data is available. + pub fn read_onto<D: Deserialize>(&self, x: &mut D) -> Result<()> { + x.deserialize_from(self) + } + + /// Safely read a sized parcelable. + /// + /// Read the size of a parcelable, compute the end position + /// of that parcelable, then build a sized readable sub-parcel + /// and call a closure with the sub-parcel as its parameter. + /// The closure can keep reading data from the sub-parcel + /// until it runs out of input data. The closure is responsible + /// for calling [`ReadableSubParcel::has_more_data`] to check for + /// more data before every read, at least until Rust generators + /// are stabilized. + /// After the closure returns, skip to the end of the current + /// parcelable regardless of how much the closure has read. + /// + /// # Examples + /// + /// ```no_run + /// let mut parcelable = Default::default(); + /// parcel.sized_read(|subparcel| { + /// if subparcel.has_more_data() { + /// parcelable.a = subparcel.read()?; + /// } + /// if subparcel.has_more_data() { + /// parcelable.b = subparcel.read()?; + /// } + /// Ok(()) + /// }); + /// ``` + /// + pub fn sized_read<F>(&self, mut f: F) -> Result<()> + where + for<'a> F: FnMut(ReadableSubParcel<'a>) -> Result<()> + { + let start = self.get_data_position(); + let parcelable_size: i32 = self.read()?; + if parcelable_size < 0 { + return Err(StatusCode::BAD_VALUE); + } + + let end = start.checked_add(parcelable_size) + .ok_or(StatusCode::BAD_VALUE)?; + if end > self.get_data_size() { + return Err(StatusCode::NOT_ENOUGH_DATA); + } + + let subparcel = ReadableSubParcel { + parcel: self, + end_position: end, + }; + f(subparcel)?; + + // Advance the data position to the actual end, + // in case the closure read less data than was available + unsafe { + self.set_data_position(end)?; + } + + Ok(()) + } + /// Read a vector size from the `Parcel` and resize the given output vector /// to be correctly sized for that amount of data. /// @@ -264,6 +336,27 @@ impl Parcel { } } +/// A segment of a readable parcel, used for [`Parcel::sized_read`]. +pub struct ReadableSubParcel<'a> { + parcel: &'a Parcel, + end_position: i32, +} + +impl<'a> ReadableSubParcel<'a> { + /// Read a type that implements [`Deserialize`] from the sub-parcel. + pub fn read<D: Deserialize>(&self) -> Result<D> { + // The caller should have checked this, + // but it can't hurt to double-check + assert!(self.has_more_data()); + D::deserialize(self.parcel) + } + + /// Check if the sub-parcel has more data to read + pub fn has_more_data(&self) -> bool { + self.parcel.get_data_position() < self.end_position + } +} + // Internal APIs impl Parcel { pub(crate) fn write_binder(&mut self, binder: Option<&SpIBinder>) -> Result<()> { diff --git a/libs/binder/rust/src/parcel/parcelable.rs b/libs/binder/rust/src/parcel/parcelable.rs index f57788b87e..956ecfe998 100644 --- a/libs/binder/rust/src/parcel/parcelable.rs +++ b/libs/binder/rust/src/parcel/parcelable.rs @@ -39,6 +39,14 @@ pub trait Serialize { pub trait Deserialize: Sized { /// Deserialize an instance from the given [`Parcel`]. fn deserialize(parcel: &Parcel) -> Result<Self>; + + /// Deserialize an instance from the given [`Parcel`] onto the + /// current object. This operation will overwrite the old value + /// partially or completely, depending on how much data is available. + fn deserialize_from(&mut self, parcel: &Parcel) -> Result<()> { + *self = Self::deserialize(parcel)?; + Ok(()) + } } /// Helper trait for types that can be serialized as arrays. @@ -184,6 +192,14 @@ pub trait DeserializeOption: Deserialize { parcel.read().map(Some) } } + + /// Deserialize an Option of this type from the given [`Parcel`] onto the + /// current object. This operation will overwrite the current value + /// partially or completely, depending on how much data is available. + fn deserialize_option_from(this: &mut Option<Self>, parcel: &Parcel) -> Result<()> { + *this = Self::deserialize_option(parcel)?; + Ok(()) + } } /// Callback to allocate a vector for parcel array read functions. @@ -677,6 +693,75 @@ impl<T: DeserializeOption> Deserialize for Option<T> { fn deserialize(parcel: &Parcel) -> Result<Self> { DeserializeOption::deserialize_option(parcel) } + + fn deserialize_from(&mut self, parcel: &Parcel) -> Result<()> { + DeserializeOption::deserialize_option_from(self, parcel) + } +} + +/// Implement `Deserialize` trait and friends for a parcelable +/// +/// This is an internal macro used by the AIDL compiler to implement +/// `Deserialize`, `DeserializeArray` and `DeserializeOption` for +/// structured parcelables. The target type must implement a +/// `deserialize_parcelable` method with the following signature: +/// ```no_run +/// fn deserialize_parcelable( +/// &mut self, +/// parcel: &binder::parcel::Parcelable, +/// ) -> binder::Result<()> { +/// // ... +/// } +/// ``` +#[macro_export] +macro_rules! impl_deserialize_for_parcelable { + ($parcelable:ident) => { + impl $crate::parcel::Deserialize for $parcelable { + fn deserialize( + parcel: &$crate::parcel::Parcel, + ) -> $crate::Result<Self> { + $crate::parcel::DeserializeOption::deserialize_option(parcel) + .transpose() + .unwrap_or(Err($crate::StatusCode::UNEXPECTED_NULL)) + } + fn deserialize_from( + &mut self, + parcel: &$crate::parcel::Parcel, + ) -> $crate::Result<()> { + let status: i32 = parcel.read()?; + if status == 0 { + Err($crate::StatusCode::UNEXPECTED_NULL) + } else { + self.deserialize_parcelable(parcel) + } + } + } + + impl $crate::parcel::DeserializeArray for $parcelable {} + + impl $crate::parcel::DeserializeOption for $parcelable { + fn deserialize_option( + parcel: &$crate::parcel::Parcel, + ) -> $crate::Result<Option<Self>> { + let mut result = None; + Self::deserialize_option_from(&mut result, parcel)?; + Ok(result) + } + fn deserialize_option_from( + this: &mut Option<Self>, + parcel: &$crate::parcel::Parcel, + ) -> $crate::Result<()> { + let status: i32 = parcel.read()?; + if status == 0 { + *this = None; + Ok(()) + } else { + this.get_or_insert_with(Self::default) + .deserialize_parcelable(parcel) + } + } + } + } } #[test] diff --git a/libs/binder/rust/src/proxy.rs b/libs/binder/rust/src/proxy.rs index 4a6d118f07..e299963c9d 100644 --- a/libs/binder/rust/src/proxy.rs +++ b/libs/binder/rust/src/proxy.rs @@ -125,6 +125,21 @@ impl SpIBinder { } } +pub mod unstable_api { + use super::{sys, SpIBinder}; + + /// A temporary API to allow the client to create a `SpIBinder` from a `sys::AIBinder`. This is + /// needed to bridge RPC binder, which doesn't have Rust API yet. + /// TODO(b/184872979): remove once the Rust API is created. + /// + /// # Safety + /// + /// See `SpIBinder::from_raw`. + pub unsafe fn new_spibinder(ptr: *mut sys::AIBinder) -> Option<SpIBinder> { + SpIBinder::from_raw(ptr) + } +} + /// An object that can be associate with an [`InterfaceClass`]. pub trait AssociateClass { /// Check if this object is a valid object for the given interface class diff --git a/libs/binder/rust/tests/integration.rs b/libs/binder/rust/tests/integration.rs index 10b77f4840..da8907decb 100644 --- a/libs/binder/rust/tests/integration.rs +++ b/libs/binder/rust/tests/integration.rs @@ -23,6 +23,9 @@ use binder::{ FIRST_CALL_TRANSACTION, }; use std::convert::{TryFrom, TryInto}; +use std::ffi::CStr; +use std::fs::File; +use std::sync::Mutex; /// Name of service runner. /// @@ -50,13 +53,11 @@ fn main() -> Result<(), &'static str> { let extension_name = args.next(); { - let mut service = Binder::new(BnTest(Box::new(TestService { - s: service_name.clone(), - }))); + let mut service = Binder::new(BnTest(Box::new(TestService::new(&service_name)))); service.set_requesting_sid(true); if let Some(extension_name) = extension_name { let extension = - BnTest::new_binder(TestService { s: extension_name }, BinderFeatures::default()); + BnTest::new_binder(TestService::new(&extension_name), BinderFeatures::default()); service .set_extension(&mut extension.as_binder()) .expect("Could not add extension"); @@ -80,14 +81,24 @@ fn print_usage() { )); } -#[derive(Clone)] struct TestService { s: String, + dump_args: Mutex<Vec<String>>, +} + +impl TestService { + fn new(s: &str) -> Self { + Self { + s: s.to_string(), + dump_args: Mutex::new(Vec::new()), + } + } } #[repr(u32)] enum TestTransactionCode { Test = FIRST_CALL_TRANSACTION, + GetDumpArgs, GetSelinuxContext, } @@ -97,6 +108,7 @@ impl TryFrom<u32> for TestTransactionCode { fn try_from(c: u32) -> Result<Self, Self::Error> { match c { _ if c == TestTransactionCode::Test as u32 => Ok(TestTransactionCode::Test), + _ if c == TestTransactionCode::GetDumpArgs as u32 => Ok(TestTransactionCode::GetDumpArgs), _ if c == TestTransactionCode::GetSelinuxContext as u32 => { Ok(TestTransactionCode::GetSelinuxContext) } @@ -105,13 +117,24 @@ impl TryFrom<u32> for TestTransactionCode { } } -impl Interface for TestService {} +impl Interface for TestService { + fn dump(&self, _file: &File, args: &[&CStr]) -> binder::Result<()> { + let mut dump_args = self.dump_args.lock().unwrap(); + dump_args.extend(args.iter().map(|s| s.to_str().unwrap().to_owned())); + Ok(()) + } +} impl ITest for TestService { fn test(&self) -> binder::Result<String> { Ok(self.s.clone()) } + fn get_dump_args(&self) -> binder::Result<Vec<String>> { + let args = self.dump_args.lock().unwrap().clone(); + Ok(args) + } + fn get_selinux_context(&self) -> binder::Result<String> { let sid = ThreadState::with_calling_sid(|sid| sid.map(|s| s.to_string_lossy().into_owned())); @@ -124,6 +147,9 @@ pub trait ITest: Interface { /// Returns a test string fn test(&self) -> binder::Result<String>; + /// Return the arguments sent via dump + fn get_dump_args(&self) -> binder::Result<Vec<String>>; + /// Returns the caller's SELinux context fn get_selinux_context(&self) -> binder::Result<String>; } @@ -145,6 +171,7 @@ fn on_transact( ) -> binder::Result<()> { match code.try_into()? { TestTransactionCode::Test => reply.write(&service.test()?), + TestTransactionCode::GetDumpArgs => reply.write(&service.get_dump_args()?), TestTransactionCode::GetSelinuxContext => reply.write(&service.get_selinux_context()?), } } @@ -157,6 +184,13 @@ impl ITest for BpTest { reply.read() } + fn get_dump_args(&self) -> binder::Result<Vec<String>> { + let reply = + self.binder + .transact(TestTransactionCode::GetDumpArgs as TransactionCode, 0, |_| Ok(()))?; + reply.read() + } + fn get_selinux_context(&self) -> binder::Result<String> { let reply = self.binder.transact( TestTransactionCode::GetSelinuxContext as TransactionCode, @@ -172,6 +206,10 @@ impl ITest for Binder<BnTest> { self.0.test() } + fn get_dump_args(&self) -> binder::Result<Vec<String>> { + self.0.get_dump_args() + } + fn get_selinux_context(&self) -> binder::Result<String> { self.0.get_selinux_context() } @@ -432,18 +470,22 @@ mod tests { { let _process = ScopedServiceProcess::new(service_name); - let mut remote = binder::get_service(service_name); + let test_client: Strong<dyn ITest> = + binder::get_interface(service_name) + .expect("Did not get test binder service"); + let mut remote = test_client.as_binder(); assert!(remote.is_binder_alive()); remote.ping_binder().expect("Could not ping remote service"); - // We're not testing the output of dump here, as that's really a - // property of the C++ implementation. There is the risk that the - // method just does nothing, but we don't want to depend on any - // particular output from the underlying library. + let dump_args = ["dump", "args", "for", "testing"]; + let null_out = File::open("/dev/null").expect("Could not open /dev/null"); remote - .dump(&null_out, &[]) + .dump(&null_out, &dump_args) .expect("Could not dump remote service"); + + let remote_args = test_client.get_dump_args().expect("Could not fetched dumped args"); + assert_eq!(dump_args, remote_args[..], "Remote args don't match call to dump"); } // get/set_extensions is tested in test_extensions() @@ -504,9 +546,7 @@ mod tests { /// rust_ndk_interop.rs #[test] fn associate_existing_class() { - let service = Binder::new(BnTest(Box::new(TestService { - s: "testing_service".to_string(), - }))); + let service = Binder::new(BnTest(Box::new(TestService::new("testing_service")))); // This should succeed although we will have to treat the service as // remote. @@ -520,9 +560,7 @@ mod tests { fn reassociate_rust_binder() { let service_name = "testing_service"; let service_ibinder = BnTest::new_binder( - TestService { - s: service_name.to_string(), - }, + TestService::new(service_name), BinderFeatures::default(), ) .as_binder(); @@ -538,9 +576,7 @@ mod tests { fn weak_binder_upgrade() { let service_name = "testing_service"; let service = BnTest::new_binder( - TestService { - s: service_name.to_string(), - }, + TestService::new(service_name), BinderFeatures::default(), ); @@ -556,9 +592,7 @@ mod tests { let service_name = "testing_service"; let weak = { let service = BnTest::new_binder( - TestService { - s: service_name.to_string(), - }, + TestService::new(service_name), BinderFeatures::default(), ); @@ -572,9 +606,7 @@ mod tests { fn weak_binder_clone() { let service_name = "testing_service"; let service = BnTest::new_binder( - TestService { - s: service_name.to_string(), - }, + TestService::new(service_name), BinderFeatures::default(), ); @@ -593,15 +625,11 @@ mod tests { #[allow(clippy::eq_op)] fn binder_ord() { let service1 = BnTest::new_binder( - TestService { - s: "testing_service1".to_string(), - }, + TestService::new("testing_service1"), BinderFeatures::default(), ); let service2 = BnTest::new_binder( - TestService { - s: "testing_service2".to_string(), - }, + TestService::new("testing_service2"), BinderFeatures::default(), ); diff --git a/libs/binder/servicedispatcher.cpp b/libs/binder/servicedispatcher.cpp index f61df084fc..62df9b7f2f 100644 --- a/libs/binder/servicedispatcher.cpp +++ b/libs/binder/servicedispatcher.cpp @@ -14,7 +14,6 @@ * limitations under the License. */ -#include <stdint.h> #include <sysexits.h> #include <unistd.h> @@ -22,15 +21,18 @@ #include <android-base/file.h> #include <android-base/logging.h> -#include <android-base/parseint.h> #include <android-base/properties.h> #include <android-base/stringprintf.h> +#include <android/os/BnServiceManager.h> +#include <android/os/IServiceManager.h> #include <binder/IServiceManager.h> #include <binder/RpcServer.h> +using android::BBinder; using android::defaultServiceManager; using android::OK; using android::RpcServer; +using android::sp; using android::statusToString; using android::String16; using android::base::Basename; @@ -39,29 +41,40 @@ using android::base::InitLogging; using android::base::LogdLogger; using android::base::LogId; using android::base::LogSeverity; -using android::base::ParseUint; using android::base::StdioLogger; using android::base::StringPrintf; +using std::string_view_literals::operator""sv; namespace { + +using ServiceRetriever = decltype(&android::IServiceManager::checkService); + int Usage(const char* program) { + auto basename = Basename(program); auto format = R"(dispatch calls to RPC service. Usage: - %s [-n <num_threads>] <service_name> - -n <num_threads>: number of RPC threads added to the service (default 1). + %s [-g] <service_name> <service_name>: the service to connect to. + %s [-g] manager + Runs an RPC-friendly service that redirects calls to servicemanager. + + -g: use getService() instead of checkService(). + + If successful, writes port number and a new line character to stdout, and + blocks until killed. + Otherwise, writes error message to stderr and exits with non-zero code. )"; - LOG(ERROR) << StringPrintf(format, Basename(program).c_str()); + LOG(ERROR) << StringPrintf(format, basename.c_str(), basename.c_str()); return EX_USAGE; } -int Dispatch(const char* name, uint32_t numThreads) { +int Dispatch(const char* name, const ServiceRetriever& serviceRetriever) { auto sm = defaultServiceManager(); if (nullptr == sm) { LOG(ERROR) << "No servicemanager"; return EX_SOFTWARE; } - auto binder = sm->checkService(String16(name)); + auto binder = std::invoke(serviceRetriever, defaultServiceManager(), String16(name)); if (nullptr == binder) { LOG(ERROR) << "No service \"" << name << "\""; return EX_SOFTWARE; @@ -78,16 +91,124 @@ int Dispatch(const char* name, uint32_t numThreads) { return EX_SOFTWARE; } auto socket = rpcServer->releaseServer(); - auto status = binder->setRpcClientDebug(std::move(socket), numThreads); + auto keepAliveBinder = sp<BBinder>::make(); + auto status = binder->setRpcClientDebug(std::move(socket), keepAliveBinder); if (status != OK) { LOG(ERROR) << "setRpcClientDebug failed with " << statusToString(status); return EX_SOFTWARE; } - LOG(INFO) << "Finish setting up RPC on service " << name << " with " << numThreads - << " threads on port" << port; + LOG(INFO) << "Finish setting up RPC on service " << name << " on port " << port; + + std::cout << port << std::endl; + + TEMP_FAILURE_RETRY(pause()); + + PLOG(FATAL) << "TEMP_FAILURE_RETRY(pause()) exits; this should not happen!"; + __builtin_unreachable(); +} + +// Wrapper that wraps a BpServiceManager as a BnServiceManager. +class ServiceManagerProxyToNative : public android::os::BnServiceManager { +public: + ServiceManagerProxyToNative(const sp<android::os::IServiceManager>& impl) : mImpl(impl) {} + android::binder::Status getService(const std::string&, + android::sp<android::IBinder>*) override { + // We can't send BpBinder for regular binder over RPC. + return android::binder::Status::fromStatusT(android::INVALID_OPERATION); + } + android::binder::Status checkService(const std::string&, + android::sp<android::IBinder>*) override { + // We can't send BpBinder for regular binder over RPC. + return android::binder::Status::fromStatusT(android::INVALID_OPERATION); + } + android::binder::Status addService(const std::string&, const android::sp<android::IBinder>&, + bool, int32_t) override { + // We can't send BpBinder for RPC over regular binder. + return android::binder::Status::fromStatusT(android::INVALID_OPERATION); + } + android::binder::Status listServices(int32_t dumpPriority, + std::vector<std::string>* _aidl_return) override { + return mImpl->listServices(dumpPriority, _aidl_return); + } + android::binder::Status registerForNotifications( + const std::string&, const android::sp<android::os::IServiceCallback>&) override { + // We can't send BpBinder for RPC over regular binder. + return android::binder::Status::fromStatusT(android::INVALID_OPERATION); + } + android::binder::Status unregisterForNotifications( + const std::string&, const android::sp<android::os::IServiceCallback>&) override { + // We can't send BpBinder for RPC over regular binder. + return android::binder::Status::fromStatusT(android::INVALID_OPERATION); + } + android::binder::Status isDeclared(const std::string& name, bool* _aidl_return) override { + return mImpl->isDeclared(name, _aidl_return); + } + android::binder::Status getDeclaredInstances(const std::string& iface, + std::vector<std::string>* _aidl_return) override { + return mImpl->getDeclaredInstances(iface, _aidl_return); + } + android::binder::Status updatableViaApex(const std::string& name, + std::optional<std::string>* _aidl_return) override { + return mImpl->updatableViaApex(name, _aidl_return); + } + android::binder::Status registerClientCallback( + const std::string&, const android::sp<android::IBinder>&, + const android::sp<android::os::IClientCallback>&) override { + // We can't send BpBinder for RPC over regular binder. + return android::binder::Status::fromStatusT(android::INVALID_OPERATION); + } + android::binder::Status tryUnregisterService(const std::string&, + const android::sp<android::IBinder>&) override { + // We can't send BpBinder for RPC over regular binder. + return android::binder::Status::fromStatusT(android::INVALID_OPERATION); + } + android::binder::Status getServiceDebugInfo( + std::vector<android::os::ServiceDebugInfo>* _aidl_return) override { + return mImpl->getServiceDebugInfo(_aidl_return); + } + +private: + sp<android::os::IServiceManager> mImpl; +}; + +// Workaround for b/191059588. +// TODO(b/191059588): Once we can run RpcServer on single-threaded services, +// `servicedispatcher manager` should call Dispatch("manager") directly. +int wrapServiceManager(const ServiceRetriever& serviceRetriever) { + auto sm = defaultServiceManager(); + if (nullptr == sm) { + LOG(ERROR) << "No servicemanager"; + return EX_SOFTWARE; + } + auto service = std::invoke(serviceRetriever, defaultServiceManager(), String16("manager")); + if (nullptr == service) { + LOG(ERROR) << "No service called `manager`"; + return EX_SOFTWARE; + } + auto interface = android::os::IServiceManager::asInterface(service); + if (nullptr == interface) { + LOG(ERROR) << "Cannot cast service called `manager` to IServiceManager"; + return EX_SOFTWARE; + } + + // Work around restriction that doesn't allow us to send proxy over RPC. + interface = sp<ServiceManagerProxyToNative>::make(interface); + service = ServiceManagerProxyToNative::asBinder(interface); + auto rpcServer = RpcServer::make(); + rpcServer->iUnderstandThisCodeIsExperimentalAndIWillNotUseItInProduction(); + rpcServer->setRootObject(service); + unsigned int port; + if (!rpcServer->setupInetServer(0, &port)) { + LOG(ERROR) << "Unable to set up inet server"; + return EX_SOFTWARE; + } + LOG(INFO) << "Finish wrapping servicemanager with RPC on port " << port; std::cout << port << std::endl; - return EX_OK; + rpcServer->join(); + + LOG(FATAL) << "Wrapped servicemanager exits; this should not happen!"; + __builtin_unreachable(); } // Log to logd. For warning and more severe messages, also log to stderr. @@ -117,22 +238,24 @@ int main(int argc, char* argv[]) { } LOG(WARNING) << "WARNING: servicedispatcher is debug only. Use with caution."; - uint32_t numThreads = 1; int opt; - while (-1 != (opt = getopt(argc, argv, "n:"))) { + ServiceRetriever serviceRetriever = &android::IServiceManager::checkService; + while (-1 != (opt = getopt(argc, argv, "g"))) { switch (opt) { - case 'n': { - if (!ParseUint(optarg, &numThreads)) { - return Usage(argv[0]); - } + case 'g': { + serviceRetriever = &android::IServiceManager::getService; } break; default: { return Usage(argv[0]); } } } + if (optind + 1 != argc) return Usage(argv[0]); auto name = argv[optind]; - return Dispatch(name, numThreads); + if (name == "manager"sv) { + return wrapServiceManager(serviceRetriever); + } + return Dispatch(name, serviceRetriever); } diff --git a/libs/binder/tests/Android.bp b/libs/binder/tests/Android.bp index c7c899fcd8..d5990f7caa 100644 --- a/libs/binder/tests/Android.bp +++ b/libs/binder/tests/Android.bp @@ -62,6 +62,7 @@ cc_test { shared_libs: [ "libbase", "libbinder", + "liblog", "libutils", ], static_libs: [ @@ -104,6 +105,7 @@ cc_test { shared_libs: [ "libbase", "libbinder", + "liblog", "libutils", ], static_libs: [ @@ -327,3 +329,17 @@ cc_benchmark { "libutils", ], } + +cc_test_host { + name: "binderUtilsHostTest", + defaults: ["binder_test_defaults"], + srcs: ["binderUtilsHostTest.cpp"], + shared_libs: [ + "libbase", + "libbinder", + ], + static_libs: [ + "libgmock", + ], + test_suites: ["general-tests"], +} diff --git a/libs/binder/tests/IBinderRpcTest.aidl b/libs/binder/tests/IBinderRpcTest.aidl index b0c8b2d8b3..9e1078870c 100644 --- a/libs/binder/tests/IBinderRpcTest.aidl +++ b/libs/binder/tests/IBinderRpcTest.aidl @@ -55,6 +55,7 @@ interface IBinderRpcTest { oneway void sleepMsAsync(int ms); void doCallback(IBinderRpcCallback callback, boolean isOneway, boolean delayed, @utf8InCpp String value); + oneway void doCallbackAsync(IBinderRpcCallback callback, boolean isOneway, boolean delayed, @utf8InCpp String value); void die(boolean cleanup); void scheduleShutdown(); diff --git a/libs/binder/tests/binderLibTest.cpp b/libs/binder/tests/binderLibTest.cpp index c4eacfdc9e..4c3225f302 100644 --- a/libs/binder/tests/binderLibTest.cpp +++ b/libs/binder/tests/binderLibTest.cpp @@ -15,14 +15,13 @@ */ #include <errno.h> -#include <fcntl.h> -#include <fstream> #include <poll.h> #include <pthread.h> #include <stdio.h> #include <stdlib.h> #include <chrono> +#include <fstream> #include <thread> #include <gmock/gmock.h> @@ -31,8 +30,10 @@ #include <android-base/properties.h> #include <android-base/result-gmock.h> #include <android-base/result.h> +#include <android-base/strings.h> #include <android-base/unique_fd.h> #include <binder/Binder.h> +#include <binder/BpBinder.h> #include <binder/IBinder.h> #include <binder/IPCThreadState.h> #include <binder/IServiceManager.h> @@ -54,6 +55,7 @@ using namespace android; using namespace std::string_literals; using namespace std::chrono_literals; using android::base::testing::HasValue; +using android::base::testing::Ok; using testing::ExplainMatchResult; using testing::Not; using testing::WithParamInterface; @@ -112,8 +114,6 @@ enum BinderLibTestTranscationCode { BINDER_LIB_TEST_ECHO_VECTOR, BINDER_LIB_TEST_REJECT_BUF, BINDER_LIB_TEST_CAN_GET_SID, - BINDER_LIB_TEST_USLEEP, - BINDER_LIB_TEST_CREATE_TEST_SERVICE, }; pid_t start_server_process(int arg2, bool usePoll = false) @@ -1200,131 +1200,60 @@ public: } }; -class BinderLibRpcTest : public BinderLibRpcTestBase, public WithParamInterface<bool> { -public: - sp<IBinder> GetService() { - return GetParam() ? sp<IBinder>(addServer()) : sp<IBinder>(sp<BBinder>::make()); - } - static std::string ParamToString(const testing::TestParamInfo<ParamType> &info) { - return info.param ? "remote" : "local"; - } -}; - -TEST_P(BinderLibRpcTest, SetRpcMaxThreads) { - auto binder = GetService(); - ASSERT_TRUE(binder != nullptr); - auto [socket, port] = CreateSocket(); - ASSERT_TRUE(socket.ok()); - EXPECT_THAT(binder->setRpcClientDebug(std::move(socket), 1), StatusEq(OK)); -} - -TEST_P(BinderLibRpcTest, SetRpcClientNoFd) { - auto binder = GetService(); - ASSERT_TRUE(binder != nullptr); - EXPECT_THAT(binder->setRpcClientDebug(android::base::unique_fd(), 1), StatusEq(BAD_VALUE)); -} +class BinderLibRpcTest : public BinderLibRpcTestBase {}; -TEST_P(BinderLibRpcTest, SetRpcMaxThreadsZero) { - auto binder = GetService(); +TEST_F(BinderLibRpcTest, SetRpcClientDebug) { + auto binder = addServer(); ASSERT_TRUE(binder != nullptr); auto [socket, port] = CreateSocket(); ASSERT_TRUE(socket.ok()); - EXPECT_THAT(binder->setRpcClientDebug(std::move(socket), 0), StatusEq(BAD_VALUE)); + EXPECT_THAT(binder->setRpcClientDebug(std::move(socket), sp<BBinder>::make()), StatusEq(OK)); } -TEST_P(BinderLibRpcTest, SetRpcMaxThreadsTwice) { - auto binder = GetService(); +// Tests for multiple RpcServer's on the same binder object. +TEST_F(BinderLibRpcTest, SetRpcClientDebugTwice) { + auto binder = addServer(); ASSERT_TRUE(binder != nullptr); auto [socket1, port1] = CreateSocket(); ASSERT_TRUE(socket1.ok()); - EXPECT_THAT(binder->setRpcClientDebug(std::move(socket1), 1), StatusEq(OK)); + auto keepAliveBinder1 = sp<BBinder>::make(); + EXPECT_THAT(binder->setRpcClientDebug(std::move(socket1), keepAliveBinder1), StatusEq(OK)); auto [socket2, port2] = CreateSocket(); ASSERT_TRUE(socket2.ok()); - EXPECT_THAT(binder->setRpcClientDebug(std::move(socket2), 1), StatusEq(ALREADY_EXISTS)); + auto keepAliveBinder2 = sp<BBinder>::make(); + EXPECT_THAT(binder->setRpcClientDebug(std::move(socket2), keepAliveBinder2), StatusEq(OK)); } -INSTANTIATE_TEST_CASE_P(BinderLibTest, BinderLibRpcTest, testing::Bool(), - BinderLibRpcTest::ParamToString); - -class BinderLibTestService; -class BinderLibRpcClientTest : public BinderLibRpcTestBase, - public WithParamInterface<std::tuple<bool, uint32_t>> { +// Negative tests for RPC APIs on IBinder. Call should fail in the same way on both remote and +// local binders. +class BinderLibRpcTestP : public BinderLibRpcTestBase, public WithParamInterface<bool> { public: - static std::string ParamToString(const testing::TestParamInfo<ParamType> &info) { - auto [isRemote, numThreads] = info.param; - return (isRemote ? "remote" : "local") + "_server_with_"s + std::to_string(numThreads) + - "_threads"; + sp<IBinder> GetService() { + return GetParam() ? sp<IBinder>(addServer()) : sp<IBinder>(sp<BBinder>::make()); } - sp<IBinder> CreateRemoteService(int32_t id) { - Parcel data, reply; - status_t status = data.writeInt32(id); - EXPECT_THAT(status, StatusEq(OK)); - if (status != OK) return nullptr; - status = m_server->transact(BINDER_LIB_TEST_CREATE_TEST_SERVICE, data, &reply); - EXPECT_THAT(status, StatusEq(OK)); - if (status != OK) return nullptr; - sp<IBinder> ret; - status = reply.readStrongBinder(&ret); - EXPECT_THAT(status, StatusEq(OK)); - if (status != OK) return nullptr; - return ret; + static std::string ParamToString(const testing::TestParamInfo<ParamType> &info) { + return info.param ? "remote" : "local"; } }; -TEST_P(BinderLibRpcClientTest, Test) { - auto [isRemote, numThreadsParam] = GetParam(); - uint32_t numThreads = numThreadsParam; // ... to be captured in lambda - int32_t id = 0xC0FFEE00 + numThreads; - sp<IBinder> server = isRemote ? sp<IBinder>(CreateRemoteService(id)) - : sp<IBinder>(sp<BinderLibTestService>::make(id, false)); - ASSERT_EQ(isRemote, !!server->remoteBinder()); - ASSERT_THAT(GetId(server), HasValue(id)); - - unsigned int port = 0; - // Fake servicedispatcher. - { - auto [socket, socketPort] = CreateSocket(); - ASSERT_TRUE(socket.ok()); - port = socketPort; - ASSERT_THAT(server->setRpcClientDebug(std::move(socket), numThreads), StatusEq(OK)); - } - - auto callUsleep = [](sp<IBinder> server, uint64_t us) { - Parcel data, reply; - data.markForBinder(server); - const char *name = data.isForRpc() ? "RPC" : "binder"; - EXPECT_THAT(data.writeUint64(us), StatusEq(OK)); - EXPECT_THAT(server->transact(BINDER_LIB_TEST_USLEEP, data, &reply), StatusEq(OK)) - << "for " << name << " server"; - }; - - auto threadFn = [&](size_t threadNum) { - usleep(threadNum * 50 * 1000); // threadNum * 50ms. Need this to avoid SYN flooding. - auto rpcSession = RpcSession::make(); - ASSERT_TRUE(rpcSession->setupInetClient("127.0.0.1", port)); - auto rpcServerBinder = rpcSession->getRootObject(); - ASSERT_NE(nullptr, rpcServerBinder); - - EXPECT_EQ(OK, rpcServerBinder->pingBinder()); - - // Check that |rpcServerBinder| and |server| points to the same service. - EXPECT_THAT(GetId(rpcServerBinder), HasValue(id)); - - // Occupy the server thread. The server should still have enough threads to handle - // other connections. - // (numThreads - threadNum) * 100ms - callUsleep(rpcServerBinder, (numThreads - threadNum) * 100 * 1000); - }; - std::vector<std::thread> threads; - for (size_t i = 0; i < numThreads; ++i) threads.emplace_back(std::bind(threadFn, i)); - for (auto &t : threads) t.join(); +TEST_P(BinderLibRpcTestP, SetRpcClientDebugNoFd) { + auto binder = GetService(); + ASSERT_TRUE(binder != nullptr); + EXPECT_THAT(binder->setRpcClientDebug(android::base::unique_fd(), sp<BBinder>::make()), + StatusEq(BAD_VALUE)); } -INSTANTIATE_TEST_CASE_P(BinderLibTest, BinderLibRpcClientTest, - testing::Combine(testing::Bool(), testing::Range(1u, 10u)), - BinderLibRpcClientTest::ParamToString); +TEST_P(BinderLibRpcTestP, SetRpcClientDebugNoKeepAliveBinder) { + auto binder = GetService(); + ASSERT_TRUE(binder != nullptr); + auto [socket, port] = CreateSocket(); + ASSERT_TRUE(socket.ok()); + EXPECT_THAT(binder->setRpcClientDebug(std::move(socket), nullptr), StatusEq(UNEXPECTED_NULL)); +} +INSTANTIATE_TEST_CASE_P(BinderLibTest, BinderLibRpcTestP, testing::Bool(), + BinderLibRpcTestP::ParamToString); class BinderLibTestService : public BBinder { public: @@ -1640,18 +1569,6 @@ public: case BINDER_LIB_TEST_CAN_GET_SID: { return IPCThreadState::self()->getCallingSid() == nullptr ? BAD_VALUE : NO_ERROR; } - case BINDER_LIB_TEST_USLEEP: { - uint64_t us; - if (status_t status = data.readUint64(&us); status != NO_ERROR) return status; - usleep(us); - return NO_ERROR; - } - case BINDER_LIB_TEST_CREATE_TEST_SERVICE: { - int32_t id; - if (status_t status = data.readInt32(&id); status != NO_ERROR) return status; - reply->writeStrongBinder(sp<BinderLibTestService>::make(id, false)); - return NO_ERROR; - } default: return UNKNOWN_TRANSACTION; }; diff --git a/libs/binder/tests/binderRpcTest.cpp b/libs/binder/tests/binderRpcTest.cpp index 82f8a3e273..e452678755 100644 --- a/libs/binder/tests/binderRpcTest.cpp +++ b/libs/binder/tests/binderRpcTest.cpp @@ -214,7 +214,8 @@ public: if (delayed) { std::thread([=]() { ALOGE("Executing delayed callback: '%s'", value.c_str()); - (void)doCallback(callback, oneway, false, value); + Status status = doCallback(callback, oneway, false, value); + ALOGE("Delayed callback status: '%s'", status.toString8().c_str()); }).detach(); return Status::ok(); } @@ -226,6 +227,11 @@ public: return callback->sendCallback(value); } + Status doCallbackAsync(const sp<IBinderRpcCallback>& callback, bool oneway, bool delayed, + const std::string& value) override { + return doCallback(callback, oneway, delayed, value); + } + Status die(bool cleanup) override { if (cleanup) { exit(1); @@ -255,27 +261,17 @@ public: }; sp<IBinder> MyBinderRpcTest::mHeldBinder; -class Pipe { -public: - Pipe() { CHECK(android::base::Pipe(&mRead, &mWrite)); } - Pipe(Pipe&&) = default; - android::base::borrowed_fd readEnd() { return mRead; } - android::base::borrowed_fd writeEnd() { return mWrite; } - -private: - android::base::unique_fd mRead; - android::base::unique_fd mWrite; -}; - class Process { public: Process(Process&&) = default; - Process(const std::function<void(Pipe*)>& f) { + Process(const std::function<void(android::base::borrowed_fd /* writeEnd */)>& f) { + android::base::unique_fd writeEnd; + CHECK(android::base::Pipe(&mReadEnd, &writeEnd)) << strerror(errno); if (0 == (mPid = fork())) { // racey: assume parent doesn't crash before this is set prctl(PR_SET_PDEATHSIG, SIGHUP); - f(&mPipe); + f(writeEnd); exit(0); } @@ -285,11 +281,11 @@ public: waitpid(mPid, nullptr, 0); } } - Pipe* getPipe() { return &mPipe; } + android::base::borrowed_fd readEnd() { return mReadEnd; } private: pid_t mPid = 0; - Pipe mPipe; + android::base::unique_fd mReadEnd; }; static std::string allocateSocketAddress() { @@ -298,6 +294,11 @@ static std::string allocateSocketAddress() { return temp + "/binderRpcTest_" + std::to_string(id++); }; +static unsigned int allocateVsockPort() { + static unsigned int vsockPort = 3456; + return vsockPort++; +} + struct ProcessSession { // reference to process hosting a socket server Process host; @@ -385,6 +386,7 @@ static inline std::string PrintSocketType(const testing::TestParamInfo<SocketTyp return ""; } } + class BinderRpc : public ::testing::TestWithParam<SocketType> { public: // This creates a new process serving an interface on a certain number of @@ -396,13 +398,12 @@ public: SocketType socketType = GetParam(); + unsigned int vsockPort = allocateVsockPort(); std::string addr = allocateSocketAddress(); unlink(addr.c_str()); - static unsigned int vsockPort = 3456; - vsockPort++; auto ret = ProcessSession{ - .host = Process([&](Pipe* pipe) { + .host = Process([&](android::base::borrowed_fd writeEnd) { sp<RpcServer> server = RpcServer::make(); server->iUnderstandThisCodeIsExperimentalAndIWillNotUseItInProduction(); @@ -426,7 +427,7 @@ public: LOG_ALWAYS_FATAL("Unknown socket type"); } - CHECK(android::base::WriteFully(pipe->writeEnd(), &outPort, sizeof(outPort))); + CHECK(android::base::WriteFully(writeEnd, &outPort, sizeof(outPort))); configure(server); @@ -439,7 +440,7 @@ public: // always read socket, so that we have waited for the server to start unsigned int outPort = 0; - CHECK(android::base::ReadFully(ret.host.getPipe()->readEnd(), &outPort, sizeof(outPort))); + CHECK(android::base::ReadFully(ret.host.readEnd(), &outPort, sizeof(outPort))); if (socketType == SocketType::INET) { CHECK_NE(0, outPort); } @@ -978,35 +979,54 @@ TEST_P(BinderRpc, OnewayCallExhaustion) { TEST_P(BinderRpc, Callbacks) { const static std::string kTestString = "good afternoon!"; - for (bool oneway : {true, false}) { - for (bool delayed : {true, false}) { - auto proc = createRpcTestSocketServerProcess(1, 1, 1); - auto cb = sp<MyBinderRpcCallback>::make(); - - EXPECT_OK(proc.rootIface->doCallback(cb, oneway, delayed, kTestString)); - - using std::literals::chrono_literals::operator""s; - std::unique_lock<std::mutex> _l(cb->mMutex); - cb->mCv.wait_for(_l, 1s, [&] { return !cb->mValues.empty(); }); - - EXPECT_EQ(cb->mValues.size(), 1) << "oneway: " << oneway << "delayed: " << delayed; - if (cb->mValues.empty()) continue; - EXPECT_EQ(cb->mValues.at(0), kTestString) - << "oneway: " << oneway << "delayed: " << delayed; - - // since we are severing the connection, we need to go ahead and - // tell the server to shutdown and exit so that waitpid won't hang - EXPECT_OK(proc.rootIface->scheduleShutdown()); - - // since this session has a reverse connection w/ a threadpool, we - // need to manually shut it down - EXPECT_TRUE(proc.proc.sessions.at(0).session->shutdownAndWait(true)); - - proc.expectAlreadyShutdown = true; + for (bool callIsOneway : {true, false}) { + for (bool callbackIsOneway : {true, false}) { + for (bool delayed : {true, false}) { + auto proc = createRpcTestSocketServerProcess(1, 1, 1); + auto cb = sp<MyBinderRpcCallback>::make(); + + if (callIsOneway) { + EXPECT_OK(proc.rootIface->doCallbackAsync(cb, callbackIsOneway, delayed, + kTestString)); + } else { + EXPECT_OK( + proc.rootIface->doCallback(cb, callbackIsOneway, delayed, kTestString)); + } + + using std::literals::chrono_literals::operator""s; + std::unique_lock<std::mutex> _l(cb->mMutex); + cb->mCv.wait_for(_l, 1s, [&] { return !cb->mValues.empty(); }); + + EXPECT_EQ(cb->mValues.size(), 1) + << "callIsOneway: " << callIsOneway + << " callbackIsOneway: " << callbackIsOneway << " delayed: " << delayed; + if (cb->mValues.empty()) continue; + EXPECT_EQ(cb->mValues.at(0), kTestString) + << "callIsOneway: " << callIsOneway + << " callbackIsOneway: " << callbackIsOneway << " delayed: " << delayed; + + // since we are severing the connection, we need to go ahead and + // tell the server to shutdown and exit so that waitpid won't hang + EXPECT_OK(proc.rootIface->scheduleShutdown()); + + // since this session has a reverse connection w/ a threadpool, we + // need to manually shut it down + EXPECT_TRUE(proc.proc.sessions.at(0).session->shutdownAndWait(true)); + + proc.expectAlreadyShutdown = true; + } } } } +TEST_P(BinderRpc, OnewayCallbackWithNoThread) { + auto proc = createRpcTestSocketServerProcess(1); + auto cb = sp<MyBinderRpcCallback>::make(); + + Status status = proc.rootIface->doCallback(cb, true /*oneway*/, false /*delayed*/, "anything"); + EXPECT_EQ(WOULD_BLOCK, status.transactionError()); +} + TEST_P(BinderRpc, Die) { for (bool doDeathCleanup : {true, false}) { auto proc = createRpcTestSocketServerProcess(1); @@ -1083,15 +1103,33 @@ TEST_P(BinderRpc, Fds) { ASSERT_EQ(beforeFds, countFds()) << (system("ls -l /proc/self/fd/"), "fd leak?"); } -INSTANTIATE_TEST_CASE_P(PerSocket, BinderRpc, - ::testing::ValuesIn({ - SocketType::UNIX, -// TODO(b/185269356): working on host -#ifdef __BIONIC__ - SocketType::VSOCK, -#endif - SocketType::INET, - }), +static bool testSupportVsockLoopback() { + unsigned int vsockPort = allocateVsockPort(); + sp<RpcServer> server = RpcServer::make(); + server->iUnderstandThisCodeIsExperimentalAndIWillNotUseItInProduction(); + CHECK(server->setupVsockServer(vsockPort)); + server->start(); + + sp<RpcSession> session = RpcSession::make(); + bool okay = session->setupVsockClient(VMADDR_CID_LOCAL, vsockPort); + CHECK(server->shutdown()); + ALOGE("Detected vsock loopback supported: %d", okay); + return okay; +} + +static std::vector<SocketType> testSocketTypes() { + std::vector<SocketType> ret = {SocketType::UNIX, SocketType::INET}; + + static bool hasVsockLoopback = testSupportVsockLoopback(); + + if (hasVsockLoopback) { + ret.push_back(SocketType::VSOCK); + } + + return ret; +} + +INSTANTIATE_TEST_CASE_P(PerSocket, BinderRpc, ::testing::ValuesIn(testSocketTypes()), PrintSocketType); class BinderRpcServerRootObject : public ::testing::TestWithParam<std::tuple<bool, bool>> {}; diff --git a/libs/binder/tests/binderUtilsHostTest.cpp b/libs/binder/tests/binderUtilsHostTest.cpp new file mode 100644 index 0000000000..fb248369a0 --- /dev/null +++ b/libs/binder/tests/binderUtilsHostTest.cpp @@ -0,0 +1,108 @@ +/* + * Copyright (C) 2021 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. + */ + +#include <sysexits.h> + +#include <chrono> + +#include <android-base/result-gmock.h> +#include <android-base/strings.h> +#include <gmock/gmock.h> +#include <gtest/gtest.h> + +#include "../UtilsHost.h" + +using android::base::testing::Ok; +using testing::Optional; + +namespace android { + +TEST(UtilsHost, ExecuteImmediately) { + auto result = execute({"echo", "foo"}, nullptr); + ASSERT_THAT(result, Ok()); + EXPECT_THAT(result->exitCode, Optional(EX_OK)); + EXPECT_EQ(result->stdout, "foo\n"); +} + +TEST(UtilsHost, ExecuteLongRunning) { + auto now = std::chrono::system_clock::now(); + + { + std::vector<std::string> args{"sh", "-c", + "sleep 0.5 && echo -n f && sleep 0.5 && echo oo && sleep 1"}; + auto result = execute(std::move(args), [](const CommandResult& commandResult) { + return android::base::EndsWith(commandResult.stdout, "\n"); + }); + auto elapsed = std::chrono::system_clock::now() - now; + auto elapsedMs = std::chrono::duration_cast<std::chrono::milliseconds>(elapsed).count(); + EXPECT_GE(elapsedMs, 1000); + EXPECT_LT(elapsedMs, 2000); + + ASSERT_THAT(result, Ok()); + EXPECT_EQ(std::nullopt, result->exitCode); + EXPECT_EQ(result->stdout, "foo\n"); + } + + // ~CommandResult() called, child process is killed. + // Assert that the second sleep does not finish. + auto elapsed = std::chrono::system_clock::now() - now; + auto elapsedMs = std::chrono::duration_cast<std::chrono::milliseconds>(elapsed).count(); + EXPECT_LT(elapsedMs, 2000); +} + +TEST(UtilsHost, ExecuteLongRunning2) { + auto now = std::chrono::system_clock::now(); + + { + std::vector<std::string> args{"sh", "-c", + "sleep 2 && echo -n f && sleep 2 && echo oo && sleep 2"}; + auto result = execute(std::move(args), [](const CommandResult& commandResult) { + return android::base::EndsWith(commandResult.stdout, "\n"); + }); + auto elapsed = std::chrono::system_clock::now() - now; + auto elapsedMs = std::chrono::duration_cast<std::chrono::milliseconds>(elapsed).count(); + EXPECT_GE(elapsedMs, 4000); + EXPECT_LT(elapsedMs, 6000); + + ASSERT_THAT(result, Ok()); + EXPECT_EQ(std::nullopt, result->exitCode); + EXPECT_EQ(result->stdout, "foo\n"); + } + + // ~CommandResult() called, child process is killed. + // Assert that the second sleep does not finish. + auto elapsed = std::chrono::system_clock::now() - now; + auto elapsedMs = std::chrono::duration_cast<std::chrono::milliseconds>(elapsed).count(); + EXPECT_LT(elapsedMs, 6000); +} + +TEST(UtilsHost, KillWithSigKill) { + std::vector<std::string> args{"sh", "-c", "echo foo && sleep 10"}; + auto result = execute(std::move(args), [](const CommandResult& commandResult) { + // FOR TEST PURPOSE ONLY. DON'T DO THIS! + if (commandResult.pid.has_value()) { + (void)kill(*commandResult.pid, SIGKILL); + } + // FOR TEST PURPOSE ONLY. DON'T DO THIS! + return false; + }); + + ASSERT_THAT(result, Ok()); + EXPECT_EQ(std::nullopt, result->exitCode); + EXPECT_THAT(result->signal, Optional(SIGKILL)); +} + +} // namespace android diff --git a/libs/binder/tests/parcel_fuzzer/hwbinder.cpp b/libs/binder/tests/parcel_fuzzer/hwbinder.cpp index 0fec393e55..35b5ebca6f 100644 --- a/libs/binder/tests/parcel_fuzzer/hwbinder.cpp +++ b/libs/binder/tests/parcel_fuzzer/hwbinder.cpp @@ -148,28 +148,6 @@ std::vector<ParcelRead<::android::hardware::Parcel>> HWBINDER_PARCEL_READ_FUNCTI // should be null since we don't create any IPC objects CHECK(data == nullptr) << data; }, - [] (const ::android::hardware::Parcel& p, uint8_t size) { - FUZZ_LOG() << "about to readEmbeddedNativeHandle"; - size_t parent_buffer_handle = size & 0xf; - size_t parent_offset = size >> 4; - const native_handle_t* handle = nullptr; - status_t status = p.readEmbeddedNativeHandle(parent_buffer_handle, parent_offset, &handle); - FUZZ_LOG() << "readEmbeddedNativeHandle status: " << status << " handle: " << handle << " handle: " << handle; - - // should be null since we don't create any IPC objects - CHECK(handle == nullptr) << handle; - }, - [] (const ::android::hardware::Parcel& p, uint8_t size) { - FUZZ_LOG() << "about to readNullableEmbeddedNativeHandle"; - size_t parent_buffer_handle = size & 0xf; - size_t parent_offset = size >> 4; - const native_handle_t* handle = nullptr; - status_t status = p.readNullableEmbeddedNativeHandle(parent_buffer_handle, parent_offset, &handle); - FUZZ_LOG() << "readNullableEmbeddedNativeHandle status: " << status << " handle: " << handle << " handle: " << handle; - - // should be null since we don't create any IPC objects - CHECK(handle == nullptr) << handle; - }, [] (const ::android::hardware::Parcel& p, uint8_t /*data*/) { FUZZ_LOG() << "about to readNativeHandleNoDup"; const native_handle_t* handle = nullptr; @@ -180,14 +158,5 @@ std::vector<ParcelRead<::android::hardware::Parcel>> HWBINDER_PARCEL_READ_FUNCTI CHECK(handle == nullptr) << handle; CHECK(status != ::android::OK); }, - [] (const ::android::hardware::Parcel& p, uint8_t /*data*/) { - FUZZ_LOG() << "about to readNullableNativeHandleNoDup"; - const native_handle_t* handle = nullptr; - status_t status = p.readNullableNativeHandleNoDup(&handle); - FUZZ_LOG() << "readNullableNativeHandleNoDup status: " << status << " handle: " << handle; - - // should be null since we don't create any IPC objects - CHECK(handle == nullptr) << handle; - }, }; // clang-format on diff --git a/libs/binder/tests/unit_fuzzers/BinderFuzzFunctions.h b/libs/binder/tests/unit_fuzzers/BinderFuzzFunctions.h index 72c5bc4342..8d2b714b5c 100644 --- a/libs/binder/tests/unit_fuzzers/BinderFuzzFunctions.h +++ b/libs/binder/tests/unit_fuzzers/BinderFuzzFunctions.h @@ -73,10 +73,9 @@ static const std::vector<std::function<void(FuzzedDataProvider*, const sp<BBinde [](FuzzedDataProvider*, const sp<BBinder>& bbinder) -> void { bbinder->getDebugPid(); }, - [](FuzzedDataProvider* fdp, const sp<BBinder>& bbinder) -> void { - auto rpcMaxThreads = fdp->ConsumeIntegralInRange<uint32_t>(0, 20); + [](FuzzedDataProvider*, const sp<BBinder>& bbinder) -> void { (void)bbinder->setRpcClientDebug(android::base::unique_fd(), - rpcMaxThreads); + sp<BBinder>::make()); }}; } // namespace android |