diff --git a/fs_mgr/fs_mgr_fstab.cpp b/fs_mgr/fs_mgr_fstab.cpp index 9b5152910..809aa61c0 100644 --- a/fs_mgr/fs_mgr_fstab.cpp +++ b/fs_mgr/fs_mgr_fstab.cpp @@ -530,67 +530,37 @@ bool EraseFstabEntry(Fstab* fstab, const std::string& mount_point) { } // namespace -bool ReadFstabFromFp(FILE* fstab_file, bool proc_mounts, Fstab* fstab_out) { - ssize_t len; - size_t alloc_len = 0; - char *line = NULL; - const char *delim = " \t"; - char *save_ptr, *p; +bool ParseFstabFromString(const std::string& fstab_str, bool proc_mounts, Fstab* fstab_out) { + const int expected_fields = proc_mounts ? 4 : 5; + Fstab fstab; - while ((len = getline(&line, &alloc_len, fstab_file)) != -1) { - /* if the last character is a newline, shorten the string by 1 byte */ - if (line[len - 1] == '\n') { - line[len - 1] = '\0'; + for (const auto& line : android::base::Split(fstab_str, "\n")) { + auto fields = android::base::Tokenize(line, " \t"); + + // Ignore empty lines and comments. + if (fields.empty() || android::base::StartsWith(fields.front(), '#')) { + continue; } - /* Skip any leading whitespace */ - p = line; - while (isspace(*p)) { - p++; + if (fields.size() < expected_fields) { + LERROR << "Error parsing fstab: expected " << expected_fields << " fields, got " + << fields.size(); + return false; } - /* ignore comments or empty lines */ - if (*p == '#' || *p == '\0') - continue; FstabEntry entry; + auto it = fields.begin(); - if (!(p = strtok_r(line, delim, &save_ptr))) { - LERROR << "Error parsing mount source"; - goto err; - } - entry.blk_device = p; - - if (!(p = strtok_r(NULL, delim, &save_ptr))) { - LERROR << "Error parsing mount_point"; - goto err; - } - entry.mount_point = p; - - if (!(p = strtok_r(NULL, delim, &save_ptr))) { - LERROR << "Error parsing fs_type"; - goto err; - } - entry.fs_type = p; - - if (!(p = strtok_r(NULL, delim, &save_ptr))) { - LERROR << "Error parsing mount_flags"; - goto err; - } - - ParseMountFlags(p, &entry); + entry.blk_device = std::move(*it++); + entry.mount_point = std::move(*it++); + entry.fs_type = std::move(*it++); + ParseMountFlags(std::move(*it++), &entry); // For /proc/mounts, ignore everything after mnt_freq and mnt_passno - if (proc_mounts) { - p += strlen(p); - } else if (!(p = strtok_r(NULL, delim, &save_ptr))) { - LERROR << "Error parsing fs_mgr_options"; - goto err; - } - - if (!ParseFsMgrFlags(p, &entry)) { + if (!proc_mounts && !ParseFsMgrFlags(std::move(*it++), &entry)) { LERROR << "Error parsing fs_mgr_flags"; - goto err; + return false; } if (entry.fs_mgr_flags.logical) { @@ -602,21 +572,17 @@ bool ReadFstabFromFp(FILE* fstab_file, bool proc_mounts, Fstab* fstab_out) { if (fstab.empty()) { LERROR << "No entries found in fstab"; - goto err; + return false; } /* If an A/B partition, modify block device to be the real block device */ if (!fs_mgr_update_for_slotselect(&fstab)) { LERROR << "Error updating for slotselect"; - goto err; + return false; } - free(line); + *fstab_out = std::move(fstab); return true; - -err: - free(line); - return false; } void TransformFstabForDsu(Fstab* fstab, const std::string& dsu_slot, @@ -715,16 +681,18 @@ void EnableMandatoryFlags(Fstab* fstab) { } bool ReadFstabFromFile(const std::string& path, Fstab* fstab_out) { - auto fstab_file = std::unique_ptr{fopen(path.c_str(), "re"), fclose}; - if (!fstab_file) { - PERROR << __FUNCTION__ << "(): cannot open file: '" << path << "'"; + const bool is_proc_mounts = (path == "/proc/mounts"); + // /proc/mounts could be a symlink to /proc/self/mounts. + const bool follow_symlinks = is_proc_mounts; + + std::string fstab_str; + if (!android::base::ReadFileToString(path, &fstab_str, follow_symlinks)) { + PERROR << __FUNCTION__ << "(): failed to read file: '" << path << "'"; return false; } - bool is_proc_mounts = path == "/proc/mounts"; - Fstab fstab; - if (!ReadFstabFromFp(fstab_file.get(), is_proc_mounts, &fstab)) { + if (!ParseFstabFromString(fstab_str, is_proc_mounts, &fstab)) { LERROR << __FUNCTION__ << "(): failed to load fstab from : '" << path << "'"; return false; } @@ -771,15 +739,7 @@ bool ReadFstabFromDt(Fstab* fstab, bool verbose) { return false; } - std::unique_ptr fstab_file( - fmemopen(static_cast(const_cast(fstab_buf.c_str())), - fstab_buf.length(), "r"), fclose); - if (!fstab_file) { - if (verbose) PERROR << __FUNCTION__ << "(): failed to create a file stream for fstab dt"; - return false; - } - - if (!ReadFstabFromFp(fstab_file.get(), false, fstab)) { + if (!ParseFstabFromString(fstab_buf, /* proc_mounts = */ false, fstab)) { if (verbose) { LERROR << __FUNCTION__ << "(): failed to load fstab from kernel:" << std::endl << fstab_buf; diff --git a/fs_mgr/fuzz/fs_mgr_fstab_fuzzer.cpp b/fs_mgr/fuzz/fs_mgr_fstab_fuzzer.cpp index 1fddbf82d..6a8a19176 100644 --- a/fs_mgr/fuzz/fs_mgr_fstab_fuzzer.cpp +++ b/fs_mgr/fuzz/fs_mgr_fstab_fuzzer.cpp @@ -19,12 +19,8 @@ #include extern "C" int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size) { - std::unique_ptr fstab_file( - fmemopen(static_cast(const_cast(data)), size, "r"), fclose); - if (fstab_file == nullptr) { - return 0; - } + std::string make_fstab_str(reinterpret_cast(data), size); android::fs_mgr::Fstab fstab; - android::fs_mgr::ReadFstabFromFp(fstab_file.get(), /* proc_mounts= */ false, &fstab); + android::fs_mgr::ParseFstabFromString(make_fstab_str, /* proc_mounts = */ false, &fstab); return 0; } diff --git a/fs_mgr/include_fstab/fstab/fstab.h b/fs_mgr/include_fstab/fstab/fstab.h index d9c326d42..054300e8a 100644 --- a/fs_mgr/include_fstab/fstab/fstab.h +++ b/fs_mgr/include_fstab/fstab/fstab.h @@ -94,8 +94,8 @@ struct FstabEntry { // Unless explicitly requested, a lookup on mount point should always return the 1st one. using Fstab = std::vector; -// Exported for testability. Regular users should use ReadFstabFromfile(). -bool ReadFstabFromFp(FILE* fstab_file, bool proc_mounts, Fstab* fstab_out); +// Exported for testability. Regular users should use ReadFstabFromFile(). +bool ParseFstabFromString(const std::string& fstab_str, bool proc_mounts, Fstab* fstab_out); bool ReadFstabFromFile(const std::string& path, Fstab* fstab); bool ReadFstabFromDt(Fstab* fstab, bool verbose = true);