diff options
4 files changed, 43 insertions, 48 deletions
diff --git a/kernel/kernel/filesystems/fat32/file_allocation_table.tests.cpp b/kernel/kernel/filesystems/fat32/file_allocation_table.tests.cpp index bcc85a51..b840dbd5 100644 --- a/kernel/kernel/filesystems/fat32/file_allocation_table.tests.cpp +++ b/kernel/kernel/filesystems/fat32/file_allocation_table.tests.cpp @@ -45,7 +45,8 @@ SCENARIO_METHOD(kernel::tests::vfs::storage_boot_module_fixture, "Fat32 cluster THEN("the root directory occupies a single cluster") { - auto const chain = kernel::filesystems::fat32::cluster_chain(driver_state->boot_sector.root_cluster, *driver_state); + auto const chain = + kernel::filesystems::fat32::cluster_chain(driver_state->boot_sector.root_cluster, *driver_state); REQUIRE(chain); REQUIRE(*chain == kstd::vector<std::uint32_t>{2}); } @@ -99,8 +100,8 @@ SCENARIO("Fat32 cluster chain traversal", "[filesystem][fat32][fat]") REQUIRE_FALSE(kernel::filesystems::fat32::is_valid_cluster(0, *driver_state)); REQUIRE_FALSE(kernel::filesystems::fat32::is_valid_cluster(1, *driver_state)); REQUIRE(kernel::filesystems::fat32::is_valid_cluster(2, *driver_state)); - REQUIRE(kernel::filesystems::fat32::is_valid_cluster(17, *driver_state)); - REQUIRE_FALSE(kernel::filesystems::fat32::is_valid_cluster(18, *driver_state)); + REQUIRE(kernel::filesystems::fat32::is_valid_cluster(69'632 + 1, *driver_state)); + REQUIRE_FALSE(kernel::filesystems::fat32::is_valid_cluster(69'632 + 2, *driver_state)); } THEN("cluster offsets start at the data region") @@ -129,7 +130,7 @@ SCENARIO("Fat32 cluster chain traversal", "[filesystem][fat32][fat]") REQUIRE(read_fat_entry(7, *driver_state)->type == fat_entry_type::end_of_chain); REQUIRE(read_fat_entry(8, *driver_state)->type == fat_entry_type::end_of_chain); REQUIRE(read_fat_entry(8, *driver_state)->value == 0x0FFF'FFFF); - REQUIRE(read_fat_entry(18, *driver_state).error() == errc::invalid_cluster); + REQUIRE(read_fat_entry(69632 + 2, *driver_state).error() == errc::invalid_cluster); } THEN("a multi cluster chain is followed to its end") @@ -161,8 +162,8 @@ SCENARIO("Fat32 cluster chain traversal", "[filesystem][fat32][fat]") { REQUIRE(next_cluster(0, *driver_state).error() == errc::invalid_cluster); REQUIRE(next_cluster(1, *driver_state).error() == errc::invalid_cluster); - REQUIRE(next_cluster(18, *driver_state).error() == errc::invalid_cluster); - REQUIRE(cluster_chain(18, *driver_state).error() == errc::invalid_cluster); + REQUIRE(next_cluster(69632 + 2, *driver_state).error() == errc::invalid_cluster); + REQUIRE(cluster_chain(69632 + 2, *driver_state).error() == errc::invalid_cluster); } THEN("a chain running into a free cluster is corrupted") @@ -184,7 +185,7 @@ SCENARIO("Fat32 cluster chain traversal", "[filesystem][fat32][fat]") THEN("a chain pointing outside the data region is corrupted") { - write_fat_entry(*device, 12, 100); + write_fat_entry(*device, 12, 69632 + 2); REQUIRE(next_cluster(12, *driver_state).error() == errc::corrupted_cluster_chain); } diff --git a/kernel/kernel/filesystems/fat32/filesystem.cpp b/kernel/kernel/filesystems/fat32/filesystem.cpp index 22e5c0ef..e37b6ce3 100644 --- a/kernel/kernel/filesystems/fat32/filesystem.cpp +++ b/kernel/kernel/filesystems/fat32/filesystem.cpp @@ -80,16 +80,31 @@ namespace kernel::filesystems::fat32 fat32 }; + //! basic boot-sector validation + //! + //! @param bs populated boot-sector + //! @return true on valid boot-sector, else false + auto validate_boot_sector(boot_sector const & bs) -> bool + { + auto const fat_region_sectors = std::uint64_t{bs.number_of_fats} * bs.fat_size_32; + auto const total_sectors = bs.total_sectors_16 != 0 ? std::uint64_t{bs.total_sectors_16} : bs.total_sectors_32; + + return bs.bytes_per_sector >= constants::min_bytes_per_sector && + bs.bytes_per_sector <= constants::max_bytes_per_sector && bs.reserved_sector_count > 0 && + bs.number_of_fats > 0 && fat_region_sectors + bs.reserved_sector_count < total_sectors && + std::has_single_bit(bs.bytes_per_sector) && std::has_single_bit(bs.sectors_per_cluster); + } + //! Derive the fat-type with the variables in the boot-sector. The differentiation is made by number of clusters in - //! the data-region following microsoft specification 1.03. This function doesn't validate the provided boot_sector and - //! necessitates that it's valid. + //! the data-region following microsoft specification 1.03. This function doesn't validate the provided boot_sector + //! and necessitates that it's valid. //! - //! @param bs populated boot_sector + //! @param bs populated boot-sector //! @return returns fat12, fat16 or fat32 auto get_fat_type(boot_sector const & bs) -> fat_type { // is zero for FAT32 because root directory is stored in data-sector - auto const root_dir_size_fat16 = std::uint64_t{bs.root_entry_count} * 32; // todo maybe constant + auto const root_dir_size_fat16 = std::uint64_t{bs.root_entry_count} * 32; // todo maybe constant // ceiling divide auto const root_dir_sectors_fat16 = (root_dir_size_fat16 + bs.bytes_per_sector - 1) / bs.bytes_per_sector; @@ -132,8 +147,14 @@ namespace kernel::filesystems::fat32 return kstd::failure(errc::invalid_boot_signature); } - if (auto fat_type = get_fat_type(boot_sector); fat_type != fat_type::fat32) { - return kstd::failure(errc::invalid_boot_sector); // todo custom error + if (!validate_boot_sector(boot_sector)) + { + return kstd::failure(errc::invalid_boot_sector); + } + + if (auto fat_type = get_fat_type(boot_sector); fat_type != fat_type::fat32) + { + return kstd::failure(errc::invalid_boot_sector); // todo custom error } return boot_sector; @@ -142,8 +163,7 @@ namespace kernel::filesystems::fat32 //! Derive the location of the file allocation table and the data region from the boot sector. //! //! @param state The driver state whose boot sector to use and whose layout fields to fill. - //! @return Nothing on success, an error if the boot sector describes an impossible layout. - [[nodiscard]] auto compute_layout(driver_state & state) -> kstd::result<void> + auto compute_layout(driver_state & state) -> void { auto const & boot_sector = state.boot_sector; @@ -153,30 +173,13 @@ namespace kernel::filesystems::fat32 : std::uint64_t{boot_sector.total_sectors_16}; auto const fat_sectors = std::uint64_t{boot_sector.number_of_fats} * boot_sector.fat_size_32; auto const data_start_sector = boot_sector.reserved_sector_count + fat_sectors; - - if (bytes_per_sector < constants::min_bytes_per_sector || bytes_per_sector > constants::max_bytes_per_sector || - !std::has_single_bit(bytes_per_sector) || !std::has_single_bit(sectors_per_cluster) || - boot_sector.reserved_sector_count == 0 || fat_sectors == 0 || data_start_sector >= total_sectors) - { - return kstd::failure(errc::invalid_boot_sector); - } - auto const cluster_count = (total_sectors - data_start_sector) / sectors_per_cluster; - auto const fat_capacity = - std::uint64_t{boot_sector.fat_size_32} * bytes_per_sector / constants::fat_entry_size.value; - - if (cluster_count == 0 || cluster_count + constants::first_data_cluster > fat_capacity) - { - return kstd::failure(errc::invalid_boot_sector); - } state.sector_size = kstd::bytes{bytes_per_sector}; state.cluster_size = state.sector_size * sectors_per_cluster; state.fat_start_sector = boot_sector.reserved_sector_count; state.data_start_sector = static_cast<std::uint32_t>(data_start_sector); state.cluster_count = static_cast<std::uint32_t>(cluster_count); - - return {}; } } // namespace @@ -210,10 +213,7 @@ namespace kernel::filesystems::fat32 } driver_state->boot_sector = *boot_sector; - if (auto layout_result = compute_layout(*driver_state); !layout_result) - { - return kstd::failure(layout_result.error()); - } + compute_layout(*driver_state); auto const root_cluster = std::uint32_t{driver_state->boot_sector.root_cluster}; if (!is_valid_cluster(root_cluster, *driver_state)) diff --git a/kernel/kernel/filesystems/fat32/filesystem.tests.cpp b/kernel/kernel/filesystems/fat32/filesystem.tests.cpp index b2f4af62..b71a4150 100644 --- a/kernel/kernel/filesystems/fat32/filesystem.tests.cpp +++ b/kernel/kernel/filesystems/fat32/filesystem.tests.cpp @@ -128,13 +128,6 @@ SCENARIO("Fat32 filesystem mount validates the boot sector", "[filesystem][fat32 kernel::tests::filesystems::fat32::setup_mock_fat32_layout(*device, boot_sector); REQUIRE(fs->mount(dev_inode).error() == kernel::filesystems::fat32::errc::invalid_boot_sector); } - - THEN("a root cluster outside the data region is rejected") - { - boot_sector.root_cluster = 18; - kernel::tests::filesystems::fat32::setup_mock_fat32_layout(*device, boot_sector); - REQUIRE(fs->mount(dev_inode).error() == kernel::filesystems::fat32::errc::invalid_cluster); - } } } diff --git a/kernel/kernel/test_support/filesystems/fat32.cpp b/kernel/kernel/test_support/filesystems/fat32.cpp index 4513a53a..9f854b39 100644 --- a/kernel/kernel/test_support/filesystems/fat32.cpp +++ b/kernel/kernel/test_support/filesystems/fat32.cpp @@ -17,9 +17,9 @@ namespace kernel::tests::filesystems::fat32 namespace { constexpr auto sector_size = 512_B; - constexpr std::uint16_t reserved_sectors = 1; + constexpr std::uint16_t reserved_sector_count = 1; constexpr std::uint32_t fat_sectors = 1; - constexpr std::uint32_t data_sectors = 16; + constexpr std::uint32_t data_sectors = 69632; auto write_bytes(kernel::tests::devices::block_device & device, kstd::bytes offset, void const * source, kstd::bytes size) -> void @@ -39,9 +39,9 @@ namespace kernel::tests::filesystems::fat32 auto boot_sector = kernel::filesystems::fat32::boot_sector{}; boot_sector.bytes_per_sector = static_cast<std::uint16_t>(sector_size.value); boot_sector.sectors_per_cluster = 1; - boot_sector.reserved_sector_count = reserved_sectors; + boot_sector.reserved_sector_count = reserved_sector_count; boot_sector.number_of_fats = 1; - boot_sector.total_sectors_32 = reserved_sectors + fat_sectors + data_sectors; + boot_sector.total_sectors_32 = reserved_sector_count + fat_sectors + data_sectors; boot_sector.fat_size_32 = fat_sectors; boot_sector.root_cluster = kernel::filesystems::fat32::constants::first_data_cluster; boot_sector.boot_signature = kernel::filesystems::fat32::constants::boot_signature; @@ -67,7 +67,8 @@ namespace kernel::tests::filesystems::fat32 auto write_fat_entry(kernel::tests::devices::block_device & device, std::uint32_t cluster, std::uint32_t value) -> void { - auto const offset = reserved_sectors * sector_size + cluster * kernel::filesystems::fat32::constants::fat_entry_size; + auto const offset = + reserved_sector_count * sector_size + cluster * kernel::filesystems::fat32::constants::fat_entry_size; write_bytes(device, offset, &value, kstd::size_of(value)); } } // namespace kernel::tests::filesystems::fat32 |