diff --git a/fs_mgr/fs_mgr_fstab.cpp b/fs_mgr/fs_mgr_fstab.cpp index 0b083e52d..d53dc54b5 100644 --- a/fs_mgr/fs_mgr_fstab.cpp +++ b/fs_mgr/fs_mgr_fstab.cpp @@ -525,34 +525,23 @@ bool EraseFstabEntry(Fstab* fstab, const std::string& mount_point) { } // namespace -bool ReadFstabFromFp(FILE* fstab_file, bool proc_mounts, Fstab* fstab_out) { +bool ParseFstabFromString(const std::string& fstab_str, bool proc_mounts, Fstab* fstab_out) { const int expected_fields = proc_mounts ? 4 : 5; - ssize_t len; - size_t alloc_len = 0; - char *line = NULL; - char* p; + 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'; - } - - /* Skip any leading whitespace */ - p = line; - while (isspace(*p)) { - p++; - } - /* ignore comments or empty lines */ - if (*p == '#' || *p == '\0') - continue; - + 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; + } + if (fields.size() < expected_fields) { LERROR << "Error parsing fstab: expected " << expected_fields << " fields, got " << fields.size(); - goto err; + return false; } FstabEntry entry; @@ -566,7 +555,7 @@ bool ReadFstabFromFp(FILE* fstab_file, bool proc_mounts, Fstab* fstab_out) { // For /proc/mounts, ignore everything after mnt_freq and mnt_passno if (!proc_mounts && !ParseFsMgrFlags(std::move(*it++), &entry)) { LERROR << "Error parsing fs_mgr_flags"; - goto err; + return false; } if (entry.fs_mgr_flags.logical) { @@ -578,21 +567,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, @@ -691,16 +676,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; } @@ -747,15 +734,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);