From 2d37797b68563f0e6219049ce003c3586e628d8c Mon Sep 17 00:00:00 2001 From: Mark Salyzyn Date: Tue, 2 May 2017 14:02:17 -0700 Subject: [PATCH 1/3] libcutils: fs_config: alias "system//" to "/" For the known partitions entrenched in the build system: vendor, oem and odm only. We will alias entries that reference system/ and / so that if either are specified, the rule will apply to both possible paths. Test: gTest libcutils-tests Bug: 37703469 Change-Id: Ida9405cbed323489a3d0599c1645e9be2c7b9d08 --- libcutils/fs_config.cpp | 34 ++++++++++++++++++++++++++++++++-- 1 file changed, 32 insertions(+), 2 deletions(-) diff --git a/libcutils/fs_config.cpp b/libcutils/fs_config.cpp index 2b3443f3f..e08ee707e 100644 --- a/libcutils/fs_config.cpp +++ b/libcutils/fs_config.cpp @@ -273,6 +273,36 @@ static int fs_config_open(int dir, int which, const char* target_out_path) { return fd; } +// if path is "vendor/", "oem/" or "odm/" +static bool is_partition(const char* path, size_t len) { + static const char* partitions[] = {"vendor/", "oem/", "odm/"}; + for (size_t i = 0; i < (sizeof(partitions) / sizeof(partitions[0])); ++i) { + size_t plen = strlen(partitions[i]); + if (len <= plen) continue; + if (!strncmp(path, partitions[i], plen)) return true; + } + return false; +} + +// alias prefixes of "/" to "system//" or +// "system//" to "/" +static bool prefix_cmp(const char* prefix, const char* path, size_t len) { + if (!strncmp(prefix, path, len)) return true; + + static const char system[] = "system/"; + if (!strncmp(path, system, strlen(system))) { + path += strlen(system); + } else if (len <= strlen(system)) { + return false; + } else if (strncmp(prefix, system, strlen(system))) { + return false; + } else { + prefix += strlen(system); + len -= strlen(system); + } + return is_partition(prefix, len) && !strncmp(prefix, path, len); +} + static bool fs_config_cmp(bool dir, const char* prefix, size_t len, const char* path, size_t plen) { if (dir) { if (plen < len) { @@ -281,13 +311,13 @@ static bool fs_config_cmp(bool dir, const char* prefix, size_t len, const char* } else { // If name ends in * then allow partial matches. if (prefix[len - 1] == '*') { - return !strncmp(prefix, path, len - 1); + return prefix_cmp(prefix, path, len - 1); } if (plen != len) { return false; } } - return !strncmp(prefix, path, len); + return prefix_cmp(prefix, path, len); } void fs_config(const char* path, int dir, const char* target_out_path, unsigned* uid, unsigned* gid, From fa39110a8f946df724d9cbee43566b83e4ee713b Mon Sep 17 00:00:00 2001 From: Mark Salyzyn Date: Wed, 3 May 2017 08:54:26 -0700 Subject: [PATCH 2/3] libcutils: fs_config test report aliases Instead of requiring aliases, let's report when we see system// all by itself, or in the company of the alias /. Report if we see duplicate entries. Add checking for overrides as well. Report any simple corruptions in internal table or in the override files. Test: gTest libcutils_test --gtest_filter=fs_config.* Bug: 37703469 Change-Id: Ia6a7e8c9bc9f553d0c1c313937b511b2073318a9 --- libcutils/tests/fs_config.cpp | 175 +++++++++++++++++++++++++++++----- 1 file changed, 150 insertions(+), 25 deletions(-) diff --git a/libcutils/tests/fs_config.cpp b/libcutils/tests/fs_config.cpp index 3917a0b2e..a62cd51a2 100644 --- a/libcutils/tests/fs_config.cpp +++ b/libcutils/tests/fs_config.cpp @@ -14,63 +14,188 @@ * limitations under the License. */ +#include + #include #include +#include +#include #include #include +#include -extern const struct fs_path_config* __for_testing_only__android_dirs; -extern const struct fs_path_config* __for_testing_only__android_files; +extern const fs_path_config* __for_testing_only__android_dirs; +extern const fs_path_config* __for_testing_only__android_files; -static void check_one(const struct fs_path_config* paths, const std::string& prefix, - const std::string& alternate) { - for (size_t idx = 0; paths[idx].prefix; ++idx) { - std::string path(paths[idx].prefix); - if (android::base::StartsWith(path, prefix.c_str())) { - path = alternate + path.substr(prefix.length()); - size_t second; - for (second = 0; paths[second].prefix; ++second) { - if (path == paths[second].prefix) break; +// Maximum entries in system/core/libcutils/fs_config.cpp:android_* before we +// hit a nullptr termination, before we declare the list is just too big or +// could be missing the nullptr. +static constexpr size_t max_idx = 4096; + +static bool check_unique(std::vector& paths, const std::string& config_name, + const std::string& prefix) { + bool retval = false; + + std::string alternate = "system/" + prefix; + + for (size_t idx = 0; idx < paths.size(); ++idx) { + size_t second; + std::string path(paths[idx]); + // check if there are multiple identical paths + for (second = idx + 1; second < paths.size(); ++second) { + if (path == paths[second]) { + GTEST_LOG_(ERROR) << "duplicate paths in " << config_name << ": " << paths[idx]; + retval = true; + break; } - if (!paths[second].prefix) { - // guaranteed to fail expectations, trigger test failure with - // a message that reports the violation as an inequality. - EXPECT_STREQ((prefix + path.substr(alternate.length())).c_str(), path.c_str()); + } + + // check if path is / + if (android::base::StartsWith(path, prefix.c_str())) { + // rebuild path to be system//... to check for alias + path = alternate + path.substr(prefix.size()); + for (second = 0; second < paths.size(); ++second) { + if (path == paths[second]) { + GTEST_LOG_(ERROR) << "duplicate alias paths in " << config_name << ": " + << paths[idx] << " and " << paths[second] + << " (remove latter)"; + retval = true; + break; + } + } + continue; + } + + // check if path is system// + if (android::base::StartsWith(path, alternate.c_str())) { + // rebuild path to be /... to check for alias + path = prefix + path.substr(alternate.size()); + for (second = 0; second < paths.size(); ++second) { + if (path == paths[second]) break; + } + if (second >= paths.size()) { + GTEST_LOG_(ERROR) << "replace path in " << config_name << ": " << paths[idx] + << " with " << path; + retval = true; } } } + return retval; } -static void check_two(const struct fs_path_config* paths, const std::string& prefix) { +static bool check_unique(const fs_path_config* paths, const char* type_name, + const std::string& prefix) { + std::string config("system/core/libcutils/fs_config.cpp:android_"); + config += type_name; + config += "[]"; + + bool retval = false; + std::vector paths_tmp; + for (size_t idx = 0; paths[idx].prefix; ++idx) { + if (idx > max_idx) { + GTEST_LOG_(WARNING) << config << ": has no end (missing null prefix)"; + retval = true; + break; + } + paths_tmp.push_back(paths[idx].prefix); + } + + return check_unique(paths_tmp, config, prefix) || retval; +} + +#define endof(pointer, field) (offsetof(typeof(*(pointer)), field) + sizeof((pointer)->field)) + +static bool check_unique(const std::string& config, const std::string& prefix) { + int retval = false; + + std::string data; + if (!android::base::ReadFileToString(config, &data)) return retval; + + const fs_path_config_from_file* pc = + reinterpret_cast(data.c_str()); + size_t len = data.size(); + + std::vector paths_tmp; + size_t entry_number = 0; + while (len > 0) { + uint16_t host_len = (len >= endof(pc, len)) ? pc->len : INT16_MAX; + if (host_len > len) { + GTEST_LOG_(WARNING) << config << ": truncated at entry " << entry_number << " (" + << host_len << " > " << len << ")"; + const std::string unknown("?"); + GTEST_LOG_(WARNING) + << config << ": entry[" << entry_number << "]={ " + << "len=" << ((len >= endof(pc, len)) + ? android::base::StringPrintf("%" PRIu16, pc->len) + : unknown) + << ", mode=" << ((len >= endof(pc, mode)) + ? android::base::StringPrintf("0%" PRIo16, pc->mode) + : unknown) + << ", uid=" << ((len >= endof(pc, uid)) + ? android::base::StringPrintf("%" PRIu16, pc->uid) + : unknown) + << ", gid=" << ((len >= endof(pc, gid)) + ? android::base::StringPrintf("%" PRIu16, pc->gid) + : unknown) + << ", capabilities=" + << ((len >= endof(pc, capabilities)) + ? android::base::StringPrintf("0x%" PRIx64, pc->capabilities) + : unknown) + << ", prefix=" + << ((len >= offsetof(fs_path_config_from_file, prefix)) + ? android::base::StringPrintf( + "\"%.*s...", (int)(len - offsetof(fs_path_config_from_file, prefix)), + pc->prefix) + : unknown) + << " }"; + retval = true; + break; + } + paths_tmp.push_back(pc->prefix); + + pc = reinterpret_cast(reinterpret_cast(pc) + + host_len); + len -= host_len; + ++entry_number; + } + + return check_unique(paths_tmp, config, prefix) || retval; +} + +void check_two(const fs_path_config* paths, const char* type_name, const char* prefix) { ASSERT_FALSE(paths == nullptr); - std::string alternate = "system/" + prefix; - check_one(paths, prefix, alternate); - check_one(paths, alternate, prefix); + ASSERT_FALSE(type_name == nullptr); + ASSERT_FALSE(prefix == nullptr); + bool check_internal = check_unique(paths, type_name, prefix); + EXPECT_FALSE(check_internal); + bool check_overrides = + check_unique(std::string("/") + prefix + "etc/fs_config_" + type_name, prefix); + EXPECT_FALSE(check_overrides); } TEST(fs_config, vendor_dirs_alias) { - check_two(__for_testing_only__android_dirs, "vendor/"); + check_two(__for_testing_only__android_dirs, "dirs", "vendor/"); } TEST(fs_config, vendor_files_alias) { - check_two(__for_testing_only__android_files, "vendor/"); + check_two(__for_testing_only__android_files, "files", "vendor/"); } TEST(fs_config, oem_dirs_alias) { - check_two(__for_testing_only__android_dirs, "oem/"); + check_two(__for_testing_only__android_dirs, "dirs", "oem/"); } TEST(fs_config, oem_files_alias) { - check_two(__for_testing_only__android_files, "oem/"); + check_two(__for_testing_only__android_files, "files", "oem/"); } TEST(fs_config, odm_dirs_alias) { - check_two(__for_testing_only__android_dirs, "odm/"); + check_two(__for_testing_only__android_dirs, "dirs", "odm/"); } TEST(fs_config, odm_files_alias) { - check_two(__for_testing_only__android_files, "odm/"); + check_two(__for_testing_only__android_files, "files", "odm/"); } From 2cf243528f582d251aee13d26127a8fd53ec3dd4 Mon Sep 17 00:00:00 2001 From: Mark Salyzyn Date: Wed, 3 May 2017 15:26:07 -0700 Subject: [PATCH 3/3] libcutils: fs_config: remove aliases Covered by the code automatically now. Test: gTest libcutils_test --gtest_filter=fs_config.* Bug: 37703469 Change-Id: Iad6ba65e023845aaea7a181b277a7383c44bd937 --- libcutils/fs_config.cpp | 18 ------------------ 1 file changed, 18 deletions(-) diff --git a/libcutils/fs_config.cpp b/libcutils/fs_config.cpp index e08ee707e..919b65bc3 100644 --- a/libcutils/fs_config.cpp +++ b/libcutils/fs_config.cpp @@ -139,14 +139,8 @@ static const struct fs_path_config android_files[] = { { 00600, AID_ROOT, AID_ROOT, 0, "odm/default.prop" }, { 00444, AID_ROOT, AID_ROOT, 0, odm_conf_dir + 1 }, { 00444, AID_ROOT, AID_ROOT, 0, odm_conf_file + 1 }, - { 00600, AID_ROOT, AID_ROOT, 0, "system/odm/build.prop" }, - { 00600, AID_ROOT, AID_ROOT, 0, "system/odm/default.prop" }, - { 00444, AID_ROOT, AID_ROOT, 0, "system/odm/etc/fs_config_dirs" }, - { 00444, AID_ROOT, AID_ROOT, 0, "system/odm/etc/fs_config_files" }, { 00444, AID_ROOT, AID_ROOT, 0, oem_conf_dir + 1 }, { 00444, AID_ROOT, AID_ROOT, 0, oem_conf_file + 1 }, - { 00444, AID_ROOT, AID_ROOT, 0, "system/oem/etc/fs_config_dirs" }, - { 00444, AID_ROOT, AID_ROOT, 0, "system/oem/etc/fs_config_files" }, { 00750, AID_ROOT, AID_SHELL, 0, "sbin/fs_mgr" }, { 00755, AID_ROOT, AID_SHELL, 0, "system/bin/crash_dump32" }, { 00755, AID_ROOT, AID_SHELL, 0, "system/bin/crash_dump64" }, @@ -163,10 +157,6 @@ static const struct fs_path_config android_files[] = { { 00555, AID_ROOT, AID_ROOT, 0, "system/etc/ppp/*" }, { 00555, AID_ROOT, AID_ROOT, 0, "system/etc/rc.*" }, { 00440, AID_ROOT, AID_ROOT, 0, "system/etc/recovery.img" }, - { 00600, AID_ROOT, AID_ROOT, 0, "system/vendor/build.prop" }, - { 00600, AID_ROOT, AID_ROOT, 0, "system/vendor/default.prop" }, - { 00444, AID_ROOT, AID_ROOT, 0, "system/vendor/etc/fs_config_dirs" }, - { 00444, AID_ROOT, AID_ROOT, 0, "system/vendor/etc/fs_config_files" }, { 00600, AID_ROOT, AID_ROOT, 0, "vendor/build.prop" }, { 00600, AID_ROOT, AID_ROOT, 0, "vendor/default.prop" }, { 00444, AID_ROOT, AID_ROOT, 0, ven_conf_dir + 1 }, @@ -200,17 +190,11 @@ static const struct fs_path_config android_files[] = { // Support Bluetooth legacy hal accessing /sys/class/rfkill // Support RT scheduling in Bluetooth - { 00700, AID_BLUETOOTH, AID_BLUETOOTH, CAP_MASK_LONG(CAP_NET_ADMIN) | - CAP_MASK_LONG(CAP_SYS_NICE), - "system/vendor/bin/hw/android.hardware.bluetooth@1.0-service" }, { 00700, AID_BLUETOOTH, AID_BLUETOOTH, CAP_MASK_LONG(CAP_NET_ADMIN) | CAP_MASK_LONG(CAP_SYS_NICE), "vendor/bin/hw/android.hardware.bluetooth@1.0-service" }, // Support wifi_hal_legacy administering a network interface. - { 00755, AID_WIFI, AID_WIFI, CAP_MASK_LONG(CAP_NET_ADMIN) | - CAP_MASK_LONG(CAP_NET_RAW), - "system/vendor/bin/hw/android.hardware.wifi@1.0-service" }, { 00755, AID_WIFI, AID_WIFI, CAP_MASK_LONG(CAP_NET_ADMIN) | CAP_MASK_LONG(CAP_NET_RAW), "vendor/bin/hw/android.hardware.wifi@1.0-service" }, @@ -233,8 +217,6 @@ static const struct fs_path_config android_files[] = { { 00755, AID_ROOT, AID_SHELL, 0, "system/bin/*" }, { 00755, AID_ROOT, AID_ROOT, 0, "system/lib/valgrind/*" }, { 00755, AID_ROOT, AID_ROOT, 0, "system/lib64/valgrind/*" }, - { 00755, AID_ROOT, AID_SHELL, 0, "system/vendor/bin/*" }, - { 00755, AID_ROOT, AID_SHELL, 0, "system/vendor/xbin/*" }, { 00755, AID_ROOT, AID_SHELL, 0, "system/xbin/*" }, { 00755, AID_ROOT, AID_SHELL, 0, "vendor/bin/*" }, { 00755, AID_ROOT, AID_SHELL, 0, "vendor/xbin/*" },