From c20c0c2cdd593f6b54bf467167a5f2ed3c602ab8 Mon Sep 17 00:00:00 2001 From: Sandeep Patil Date: Thu, 23 Feb 2017 16:09:34 -0800 Subject: [PATCH 1/2] early_mount: fs_mgr: move all fstab logic into fs_mgr With init parsing fstab fragments from kernel separately, the fs_mgr would completely miss the device tree entries. That leads to things like 'adb remount' to go through without warning for verity even if /system is verified. This happens because 'verity_update_state' completely misses the partitions passed to android through the device tree. solution is to teach fs_mgr about device tree fstab entries and add 2 new public APIs. 1. fs_mgr_read_fstab_dt() - reads device tree and returns fstab generated from it. 2. fs_mgr_read_fstab_default() - reads both device tree fstab and /fstab.{ro.hardware} and returns the combined table. This also reduces the hardcoded /fstab.{ro.hardware} occurence only to fs_mgr and for eveyone who wants to read the "default" fstab must be changed to call fs_mgr_read_fstab_default() instead. e.g. adb. b/27805372 Test: Angler was used since it has 2 early mounted partitions instead of one. 1 verified and 1 unverified. - Boot angler successfully without early mount - Boot angler successfully with /vendor early mount and test if 'adb remount' warns us about verity - Boot angler successfully with both /system and /vendor early mounted and ensure 'adb remount' warns us about verity. - check partitions.system.verified status after /system early mount ot ensure it is set to VERITY_MODE_DEFAULT. - 'adb disable-verity' with early mounted /system doesn't work due to missing changes in adb TODO: change adb to use the new fs_mgr_read_fstab_default() API Change-Id: I82038d87c7a44488e938acce2cc1082c08f6f73a Signed-off-by: Sandeep Patil --- fs_mgr/fs_mgr_boot_config.cpp | 5 +- fs_mgr/fs_mgr_fstab.cpp | 187 +++++++++++++++++++++++++++++++ fs_mgr/fs_mgr_priv.h | 2 + fs_mgr/fs_mgr_priv_boot_config.h | 2 + fs_mgr/fs_mgr_verity.cpp | 23 +--- fs_mgr/include/fs_mgr.h | 2 + init/init.cpp | 115 ++++--------------- 7 files changed, 219 insertions(+), 117 deletions(-) diff --git a/fs_mgr/fs_mgr_boot_config.cpp b/fs_mgr/fs_mgr_boot_config.cpp index ae442cff8..9decb27f3 100644 --- a/fs_mgr/fs_mgr_boot_config.cpp +++ b/fs_mgr/fs_mgr_boot_config.cpp @@ -49,8 +49,7 @@ bool fs_mgr_get_boot_config(const std::string& key, std::string* out_val) { } // lastly, check the device tree - static const std::string android_dt_dir("/proc/device-tree/firmware/android"); - std::string file_name = android_dt_dir + "/compatible"; + std::string file_name = kAndroidDtDir + "/compatible"; std::string dt_value; if (android::base::ReadFileToString(file_name, &dt_value)) { if (dt_value != "android,firmware") { @@ -58,7 +57,7 @@ bool fs_mgr_get_boot_config(const std::string& key, std::string* out_val) { return false; } - file_name = android_dt_dir + "/" + key; + file_name = kAndroidDtDir + "/" + key; // DT entries terminate with '\0' but so do the properties if (android::base::ReadFileToString(file_name, out_val)) { return true; diff --git a/fs_mgr/fs_mgr_fstab.cpp b/fs_mgr/fs_mgr_fstab.cpp index 10e70d6e5..f98978780 100644 --- a/fs_mgr/fs_mgr_fstab.cpp +++ b/fs_mgr/fs_mgr_fstab.cpp @@ -15,6 +15,7 @@ */ #include +#include #include #include #include @@ -22,6 +23,10 @@ #include #include +#include +#include +#include + #include "fs_mgr_priv.h" struct fs_mgr_flag_values { @@ -290,6 +295,110 @@ static int parse_flags(char *flags, struct flag_list *fl, return f; } +static bool is_dt_compatible() { + std::string file_name = kAndroidDtDir + "/compatible"; + std::string dt_value; + if (android::base::ReadFileToString(file_name, &dt_value)) { + // trim the trailing '\0' out, otherwise the comparison + // will produce false-negatives. + dt_value.resize(dt_value.size() - 1); + if (dt_value == "android,firmware") { + return true; + } + } + + return false; +} + +static bool is_dt_fstab_compatible() { + std::string dt_value; + std::string file_name = kAndroidDtDir + "/fstab/compatible"; + + if (android::base::ReadFileToString(file_name, &dt_value)) { + // trim the trailing '\0' out, otherwise the comparison + // will produce false-negatives. + dt_value.resize(dt_value.size() - 1); + if (dt_value == "android,fstab") { + return true; + } + } + + return false; +} + +static std::string read_fstab_from_dt() { + std::string fstab; + if (!is_dt_compatible() || !is_dt_fstab_compatible()) { + return fstab; + } + + std::string fstabdir_name = kAndroidDtDir + "/fstab"; + std::unique_ptr fstabdir(opendir(fstabdir_name.c_str()), closedir); + if (!fstabdir) return fstab; + + dirent* dp; + while ((dp = readdir(fstabdir.get())) != NULL) { + // skip over name and compatible + if (dp->d_type != DT_DIR) { + continue; + } + + // skip if its not 'vendor', 'odm' or 'system' + if (strcmp(dp->d_name, "odm") && strcmp(dp->d_name, "system") && + strcmp(dp->d_name, "vendor")) { + continue; + } + + // create \n + std::vector fstab_entry; + std::string file_name; + std::string value; + file_name = android::base::StringPrintf("%s/%s/dev", fstabdir_name.c_str(), dp->d_name); + if (!android::base::ReadFileToString(file_name, &value)) { + LERROR << "dt_fstab: Failed to find device for partition " << dp->d_name; + fstab.clear(); + break; + } + // trim the terminating '\0' out + value.resize(value.size() - 1); + fstab_entry.push_back(value); + fstab_entry.push_back(android::base::StringPrintf("/%s", dp->d_name)); + + file_name = android::base::StringPrintf("%s/%s/type", fstabdir_name.c_str(), dp->d_name); + if (!android::base::ReadFileToString(file_name, &value)) { + LERROR << "dt_fstab: Failed to find type for partition " << dp->d_name; + fstab.clear(); + break; + } + value.resize(value.size() - 1); + fstab_entry.push_back(value); + + file_name = android::base::StringPrintf("%s/%s/mnt_flags", fstabdir_name.c_str(), dp->d_name); + if (!android::base::ReadFileToString(file_name, &value)) { + LERROR << "dt_fstab: Failed to find type for partition " << dp->d_name; + fstab.clear(); + break; + } + value.resize(value.size() - 1); + fstab_entry.push_back(value); + + file_name = android::base::StringPrintf("%s/%s/fsmgr_flags", fstabdir_name.c_str(), dp->d_name); + if (!android::base::ReadFileToString(file_name, &value)) { + LERROR << "dt_fstab: Failed to find type for partition " << dp->d_name; + fstab.clear(); + break; + } + value.resize(value.size() - 1); + fstab_entry.push_back(value); + + fstab += android::base::Join(fstab_entry, " "); + fstab += '\n'; + } + + return fstab; +} + + struct fstab *fs_mgr_read_fstab_file(FILE *fstab_file) { int cnt, entries; @@ -444,6 +553,84 @@ struct fstab *fs_mgr_read_fstab(const char *fstab_path) return fstab; } +/* Returns fstab entries parsed from the device tree if they + * exist + */ +struct fstab *fs_mgr_read_fstab_dt() +{ + std::string fstab_buf = read_fstab_from_dt(); + if (fstab_buf.empty()) { + return NULL; + } + + std::unique_ptr fstab_file( + fmemopen(static_cast(const_cast(fstab_buf.c_str())), + fstab_buf.length(), "r"), fclose); + if (!fstab_file) { + return NULL; + } + + struct fstab *fstab = fs_mgr_read_fstab_file(fstab_file.get()); + if (!fstab) { + LERROR << "failed to load fstab from kernel:" << std::endl << fstab_buf; + } + + return fstab; +} + +/* combines fstab entries passed in from device tree with + * the ones found in /fstab. + */ +struct fstab *fs_mgr_read_fstab_default() +{ + struct fstab *fstab = fs_mgr_read_fstab_dt(); + std::string hw; + if (!fs_mgr_get_boot_config("hardware", &hw)) { + // if we fail to find this, return whatever was found in device tree + LWARNING << "failed to find device hardware name"; + return fstab; + } + + std::string default_fstab = FSTAB_PREFIX + hw; + struct fstab *f = fs_mgr_read_fstab(default_fstab.c_str()); + if (!f) { + // return what we have + LWARNING << "failed to read fstab entries from '" << default_fstab << "'"; + return fstab; + } + + // return the fstab read from file if device tree doesn't + // have one, other wise merge the two + if (!fstab) { + fstab = f; + } else { + int total_entries = fstab->num_entries + f->num_entries; + fstab->recs = static_cast(realloc( + fstab->recs, total_entries * (sizeof(struct fstab_rec)))); + if (!fstab->recs) { + LERROR << "failed to allocate fstab recs"; + fstab->num_entries = 0; + fs_mgr_free_fstab(fstab); + return NULL; + } + + for (int i = fstab->num_entries, j = 0; i < total_entries; i++, j++) { + // copy everything and *not* strdup + fstab->recs[i] = f->recs[j]; + } + + // free up fstab entries read from file, but don't cleanup + // the strings within f->recs[X] to make sure they are accessible + // through fstab->recs[X]. + free(f->fstab_filename); + free(f); + + fstab->num_entries = total_entries; + } + + return fstab; +} + void fs_mgr_free_fstab(struct fstab *fstab) { int i; diff --git a/fs_mgr/fs_mgr_priv.h b/fs_mgr/fs_mgr_priv.h index 478c145d3..95295d8af 100644 --- a/fs_mgr/fs_mgr_priv.h +++ b/fs_mgr/fs_mgr_priv.h @@ -41,6 +41,8 @@ #define PWARNING PLOG(WARNING) << FS_MGR_TAG #define PERROR PLOG(ERROR) << FS_MGR_TAG +const std::string FSTAB_PREFIX("/fstab."); + __BEGIN_DECLS #define CRYPTO_TMPFS_OPTIONS "size=256m,mode=0771,uid=1000,gid=1000" diff --git a/fs_mgr/fs_mgr_priv_boot_config.h b/fs_mgr/fs_mgr_priv_boot_config.h index 74bb5eb07..8773d33b0 100644 --- a/fs_mgr/fs_mgr_priv_boot_config.h +++ b/fs_mgr/fs_mgr_priv_boot_config.h @@ -20,6 +20,8 @@ #include #include +const std::string kAndroidDtDir("/proc/device-tree/firmware/android"); + bool fs_mgr_get_boot_config(const std::string& key, std::string* out_val); #endif /* __CORE_FS_MGR_PRIV_BOOTCONFIG_H */ diff --git a/fs_mgr/fs_mgr_verity.cpp b/fs_mgr/fs_mgr_verity.cpp index 2c9b0a974..5b81a54af 100644 --- a/fs_mgr/fs_mgr_verity.cpp +++ b/fs_mgr/fs_mgr_verity.cpp @@ -46,8 +46,6 @@ #include "fs_mgr_priv.h" #include "fs_mgr_priv_dm_ioctl.h" -#define FSTAB_PREFIX "/fstab." - #define VERITY_TABLE_RSA_KEY "/verity_key" #define VERITY_TABLE_HASH_IDX 8 #define VERITY_TABLE_SALT_IDX 9 @@ -694,8 +692,6 @@ static int load_verity_state(struct fstab_rec *fstab, int *mode) int fs_mgr_load_verity_state(int *mode) { - char fstab_filename[PROPERTY_VALUE_MAX + sizeof(FSTAB_PREFIX)]; - char propbuf[PROPERTY_VALUE_MAX]; int rc = -1; int i; int current; @@ -705,13 +701,9 @@ int fs_mgr_load_verity_state(int *mode) * logging mode, in which case return that */ *mode = VERITY_MODE_DEFAULT; - property_get("ro.hardware", propbuf, ""); - snprintf(fstab_filename, sizeof(fstab_filename), FSTAB_PREFIX"%s", propbuf); - - fstab = fs_mgr_read_fstab(fstab_filename); - + fstab = fs_mgr_read_fstab_default(); if (!fstab) { - LERROR << "Failed to read " << fstab_filename; + LERROR << "Failed to read default fstab"; goto out; } @@ -745,7 +737,6 @@ int fs_mgr_update_verity_state(fs_mgr_verity_state_callback callback) { alignas(dm_ioctl) char buffer[DM_BUF_SIZE]; bool system_root = false; - char fstab_filename[PROPERTY_VALUE_MAX + sizeof(FSTAB_PREFIX)]; std::string mount_point; char propbuf[PROPERTY_VALUE_MAX]; const char *status; @@ -765,22 +756,16 @@ int fs_mgr_update_verity_state(fs_mgr_verity_state_callback callback) } fd = TEMP_FAILURE_RETRY(open("/dev/device-mapper", O_RDWR | O_CLOEXEC)); - if (fd == -1) { PERROR << "Error opening device mapper"; goto out; } - property_get("ro.hardware", propbuf, ""); - snprintf(fstab_filename, sizeof(fstab_filename), FSTAB_PREFIX"%s", propbuf); - property_get("ro.build.system_root_image", propbuf, ""); system_root = !strcmp(propbuf, "true"); - - fstab = fs_mgr_read_fstab(fstab_filename); - + fstab = fs_mgr_read_fstab_default(); if (!fstab) { - LERROR << "Failed to read " << fstab_filename; + LERROR << "Failed to read default fstab"; goto out; } diff --git a/fs_mgr/include/fs_mgr.h b/fs_mgr/include/fs_mgr.h index 0402b5593..52f27ab51 100644 --- a/fs_mgr/include/fs_mgr.h +++ b/fs_mgr/include/fs_mgr.h @@ -85,6 +85,8 @@ struct fstab_rec { typedef void (*fs_mgr_verity_state_callback)(struct fstab_rec *fstab, const char *mount_point, int mode, int status); +struct fstab *fs_mgr_read_fstab_default(); +struct fstab *fs_mgr_read_fstab_dt(); struct fstab *fs_mgr_read_fstab_file(FILE *fstab_file); struct fstab *fs_mgr_read_fstab(const char *fstab_path); void fs_mgr_free_fstab(struct fstab *fstab); diff --git a/init/init.cpp b/init/init.cpp index 05f2cfd66..3cb12760b 100644 --- a/init/init.cpp +++ b/init/init.cpp @@ -502,26 +502,30 @@ static bool is_dt_compatible() { std::string dt_value; std::string file_name = StringPrintf("%s/compatible", android_dt_dir); - android::base::ReadFileToString(file_name, &dt_value); - if (!dt_value.compare("android,firmware")) { - LOG(ERROR) << "firmware/android is not compatible with 'android,firmware'"; - return false; + if (android::base::ReadFileToString(file_name, &dt_value)) { + // trim the trailing '\0' out, otherwise the comparison + // will produce false-negatives. + dt_value.resize(dt_value.size() - 1); + if (dt_value == "android,firmware") { + return true; + } } - return true; + return false; } static bool is_dt_fstab_compatible() { std::string dt_value; std::string file_name = StringPrintf("%s/%s/compatible", android_dt_dir, "fstab"); - android::base::ReadFileToString(file_name, &dt_value); - if (!dt_value.compare("android,fstab")) { - LOG(ERROR) << "firmware/android/fstab is not compatible with 'android,fstab'"; - return false; + if (android::base::ReadFileToString(file_name, &dt_value)) { + dt_value.resize(dt_value.size() - 1); + if (dt_value == "android,fstab") { + return true; + } } - return true; + return false; } static void process_kernel_dt() { @@ -664,78 +668,6 @@ static void set_usb_controller() { } } -static std::string import_dt_fstab() { - std::string fstab; - if (!is_dt_compatible() || !is_dt_fstab_compatible()) { - return fstab; - } - - std::string fstabdir_name = StringPrintf("%s/fstab", android_dt_dir); - std::unique_ptr fstabdir(opendir(fstabdir_name.c_str()), closedir); - if (!fstabdir) return fstab; - - dirent* dp; - while ((dp = readdir(fstabdir.get())) != NULL) { - // skip over name and compatible - if (dp->d_type != DT_DIR) { - continue; - } - - // skip if its not 'vendor', 'odm' or 'system' - if (strcmp(dp->d_name, "odm") && strcmp(dp->d_name, "system") && - strcmp(dp->d_name, "vendor")) { - continue; - } - - // create \n - std::vector fstab_entry; - std::string file_name; - std::string value; - file_name = StringPrintf("%s/%s/dev", fstabdir_name.c_str(), dp->d_name); - if (!android::base::ReadFileToString(file_name, &value)) { - LOG(ERROR) << "dt_fstab: Failed to find device for partition " << dp->d_name; - fstab.clear(); - break; - } - // trim the terminating '\0' out - value.resize(value.size() - 1); - fstab_entry.push_back(value); - fstab_entry.push_back(StringPrintf("/%s", dp->d_name)); - - file_name = StringPrintf("%s/%s/type", fstabdir_name.c_str(), dp->d_name); - if (!android::base::ReadFileToString(file_name, &value)) { - LOG(ERROR) << "dt_fstab: Failed to find type for partition " << dp->d_name; - fstab.clear(); - break; - } - value.resize(value.size() - 1); - fstab_entry.push_back(value); - - file_name = StringPrintf("%s/%s/mnt_flags", fstabdir_name.c_str(), dp->d_name); - if (!android::base::ReadFileToString(file_name, &value)) { - LOG(ERROR) << "dt_fstab: Failed to find type for partition " << dp->d_name; - fstab.clear(); - break; - } - value.resize(value.size() - 1); - fstab_entry.push_back(value); - - file_name = StringPrintf("%s/%s/fsmgr_flags", fstabdir_name.c_str(), dp->d_name); - if (!android::base::ReadFileToString(file_name, &value)) { - LOG(ERROR) << "dt_fstab: Failed to find type for partition " << dp->d_name; - fstab.clear(); - break; - } - value.resize(value.size() - 1); - fstab_entry.push_back(value); - - fstab += android::base::Join(fstab_entry, " "); - fstab += '\n'; - } - - return fstab; -} - static bool early_mount_one(struct fstab_rec* rec) { if (rec && fs_mgr_is_verified(rec)) { // setup verity and create the dm-XX block device @@ -770,23 +702,16 @@ static bool early_mount_one(struct fstab_rec* rec) { /* Early mount vendor and ODM partitions. The fstab is read from device-tree. */ static bool early_mount() { - std::string fstab = import_dt_fstab(); - if (fstab.empty()) { - LOG(INFO) << "Early mount skipped (missing fstab in device tree)"; + // first check if device tree fstab entries are compatible + if (!is_dt_fstab_compatible()) { + LOG(INFO) << "Early mount skipped (missing/incompatible fstab in device tree)"; return true; } - std::unique_ptr fstab_file( - fmemopen(static_cast(const_cast(fstab.c_str())), fstab.length(), "r"), fclose); - if (!fstab_file) { - PLOG(ERROR) << "Early mount failed to open fstab file in memory"; - return false; - } - - std::unique_ptr tab( - fs_mgr_read_fstab_file(fstab_file.get()), fs_mgr_free_fstab); + std::unique_ptr tab( + fs_mgr_read_fstab_dt(), fs_mgr_free_fstab); if (!tab) { - LOG(ERROR) << "Early mount fsmgr failed to load fstab from kernel:" << std::endl << fstab; + LOG(ERROR) << "Early mount failed to read fstab from device tree"; return false; } From f134fe07b842e4bd4fb9194a5806b19417ceaeef Mon Sep 17 00:00:00 2001 From: Sandeep Patil Date: Thu, 23 Feb 2017 16:26:21 -0800 Subject: [PATCH 2/2] adb: early-mount: fix verity toggle through adb for early mount verity toggle through adb failed to work for early mount due to fstab entries being moved into kernel/dt. This change fixes that by using the new fs_mgr_read_fstab_default() api that will make sure all fstab entries (from dt as well as from /fstab.{ro.hardware}) are combined before returning the fstab object. b/27805372 Test: early mount /system and /vendor on angler. - test adb disable-verity && adb root to ensure rebooted instance doesn't have 'partition.system.verified' property set. i.e. verity is indeed disabled. - test adb enable-verity && adb root to ensure rebooted instance does have 'partition.system.verified' property set, i.e. verity is enabled. also verified by trying to do 'adb remount' which correctly warns about verity Change-Id: Iffdcc0a0b959a65c326219bd4e9e41cba9ec7c88 Signed-off-by: Sandeep Patil --- adb/set_verity_enable_state_service.cpp | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/adb/set_verity_enable_state_service.cpp b/adb/set_verity_enable_state_service.cpp index f9e028b8d..76b156d66 100644 --- a/adb/set_verity_enable_state_service.cpp +++ b/adb/set_verity_enable_state_service.cpp @@ -108,11 +108,10 @@ void set_verity_enabled_state_service(int fd, void* cookie) { return; } - std::string fstab_filename = "/fstab." + android::base::GetProperty("ro.hardware", ""); - - fstab = fs_mgr_read_fstab(fstab_filename.c_str()); + // read all fstab entries at once from all sources + fstab = fs_mgr_read_fstab_default(); if (!fstab) { - WriteFdFmt(fd, "Failed to open %s\nMaybe run adb root?\n", fstab_filename.c_str()); + WriteFdFmt(fd, "Failed to read fstab\nMaybe run adb root?\n"); return; }