~aleteoryx/muditaos

40d68bb7b534f36bfde548b55582189db64ed9c1 — Tomek Sobkowiak 5 years ago 0cea748
[EGD-4495] Check partitions during registering new disk image

Check partitions validity when registering disk.
Add unittest and test disk image generation.
M module-vfs/include/internal/purefs/blkdev/partition_parser.hpp => module-vfs/include/internal/purefs/blkdev/partition_parser.hpp +4 -0
@@ 2,6 2,9 @@
// For licensing, see https://github.com/mudita/MuditaOS/LICENSE.md

#pragma once

#include <purefs/blkdev/defs.hpp>

#include <vector>
#include <memory>
#include <array>


@@ 25,6 28,7 @@ namespace purefs::blkdev
            static auto read_partitions(const std::vector<uint8_t> &buffer, std::array<partition, 4> &parts) -> void;
            static auto is_extended(uint8_t type) -> bool;
            auto parse_extended(uint32_t lba, uint32_t count) -> int;
            auto check_partition(const std::shared_ptr<disk> disk, const partition &part) -> bool;

          private:
            const std::shared_ptr<disk> m_disk;

M module-vfs/src/purefs/blkdev/partition_parser.cpp => module-vfs/src/purefs/blkdev/partition_parser.cpp +31 -7
@@ 71,6 71,10 @@ namespace purefs::blkdev::internal
        for (auto &part : root_part) {
            if (is_extended(part.type))
                continue;

            if (!check_partition(m_disk, part))
                continue;

            if (part.num_sectors) {
                part.physical_number = part_no;
                part.mbr_number      = part_no;


@@ 88,6 92,26 @@ namespace purefs::blkdev::internal
        return ret;
    }

    auto partition_parser::check_partition(const std::shared_ptr<disk> disk, const partition &part) -> bool
    {
        auto sector_size        = disk->get_info(info_type::sector_size);
        unsigned long this_size = disk->get_info(info_type::sector_count) * sector_size;
        const auto poffset      = part.start_sector * sector_size;
        const auto psize        = part.num_sectors * sector_size;
        const auto pnext        = part.start_sector * sector_size + poffset;
        if ((poffset + psize > this_size) ||                                      // oversized
            (pnext < static_cast<unsigned long>(part.start_sector * sector_size)) // going backward
        ) {
            LOG_WARN("Part %d looks strange: start_sector %u offset %u next %u\n",
                     unsigned(part.mbr_number),
                     unsigned(part.start_sector),
                     unsigned(poffset),
                     unsigned(pnext));
            return false;
        }
        return true;
    }

    auto partition_parser::read_partitions(const std::vector<uint8_t> &buffer,
                                           std::array<partition, defs::num_parts> &parts) -> void
    {


@@ 112,8 136,8 @@ namespace purefs::blkdev::internal
        auto sector_size = m_disk->get_info(info_type::sector_size);
        int extended_part_num;
        std::array<partition, defs::num_parts> parts;
        auto current_sector = lba;
        auto this_size      = count;
        auto current_sector     = lba;
        unsigned long this_size = count * sector_size;
        if (sector_size < 0) {
            return sector_size;
        }


@@ 156,10 180,10 @@ namespace purefs::blkdev::internal
                /* Some sanity checks */
                const auto poffset = parts[partition_num].start_sector * sector_size;
                const auto psize   = parts[partition_num].num_sectors * sector_size;
                const auto pnext   = current_sector + poffset;
                if ((poffset + psize > this_size) || // oversized
                    (pnext < lba) ||                 // going backward
                    (pnext > lba + count)            // outsized
                const auto pnext   = current_sector * sector_size + poffset;
                if ((poffset + psize > this_size) ||                                  // oversized
                    (pnext < static_cast<unsigned long>(lba * sector_size)) ||        // going backward
                    (pnext > static_cast<unsigned long>((lba + count) * sector_size)) // outsized
                ) {
                    LOG_WARN("Part %d looks strange: current_sector %u offset %u next %u\n",
                             int(partition_num),


@@ 179,7 203,7 @@ namespace purefs::blkdev::internal
                break; /* nothing left to do */
            }
            /* Examine the next extended partition */
            current_sector = lba + parts[extended_part_num].start_sector * sector_size;
            current_sector = lba + parts[extended_part_num].start_sector;
            this_size      = parts[extended_part_num].num_sectors * sector_size;
        }
        return error;

M module-vfs/tests/CMakeLists.txt => module-vfs/tests/CMakeLists.txt +10 -2
@@ 1,11 1,21 @@
cmake_minimum_required(VERSION 3.12)

add_custom_target("test_disk_image")
add_custom_command(
        PRE_BUILD
        TARGET test_disk_image
        COMMAND ${CMAKE_CURRENT_SOURCE_DIR}/gendisktestimg.sh ${CMAKE_BINARY_DIR}
        WORKING_DIRECTORY ${CMAKE_BINARY_DIR}
)

add_catch2_executable(
    NAME vfs-disk
    SRCS
        ${CMAKE_CURRENT_LIST_DIR}/unittest_disk_manager.cpp
    LIBS
        module-vfs
    DEPS
        test_disk_image
)

add_catch2_executable(


@@ 17,7 27,6 @@ add_catch2_executable(
)

set(LITTLEFS_IMAGE "lfstest.img")

add_custom_command( 
    OUTPUT ${LITTLEFS_IMAGE}
    COMMAND ${CMAKE_CURRENT_SOURCE_DIR}/genlfstestimg.sh 1G ${LITTLEFS_IMAGE}


@@ 26,7 35,6 @@ add_custom_command(
)

add_custom_target( test_lfs_image DEPENDS ${LITTLEFS_IMAGE} )

add_catch2_executable(
    NAME vfs-littlefs
    SRCS

A module-vfs/tests/gendisktestimg.sh => module-vfs/tests/gendisktestimg.sh +41 -0
@@ 0,0 1,41 @@
#!/bin/bash -e
# Copyright (c) 2017-2021, Mudita Sp. z.o.o. All rights reserved.
# For licensing, see https://github.com/mudita/MuditaOS/LICENSE.md

#Copyright (c) 2017-2020, Mudita Sp. z.o.o. All rights reserved.
#For licensing, see https://github.com/mudita/MuditaOS/LICENSE.md

if [ $# -ne 1 ]; then
	echo "Error! Invalid argument count"
	exit -1
fi
IMAGE_FILE_DIR="$1"

dd if=/dev/zero of=$IMAGE_FILE_DIR/test_disk.img bs=512 count=8
sfdisk $IMAGE_FILE_DIR/test_disk.img << ==sfdisk
,1,L
,1,L
,,L
==sfdisk

dd if=/dev/zero of=$IMAGE_FILE_DIR/test_disk_ext.img bs=512 count=16
sfdisk $IMAGE_FILE_DIR/test_disk_ext.img << ==sfdisk
,1,L
,,E
,1,L
,1,L
,1,L
,1,L
,1,L
,,L
==sfdisk

dd if=/dev/zero of=$IMAGE_FILE_DIR/test_disk_bad_tmp.img bs=512 count=16
sfdisk $IMAGE_FILE_DIR/test_disk_bad_tmp.img << ==sfdisk
,1,L
==sfdisk

# Modify parition table to simply broke it, which ten can be tested.
{ head -c 458 $IMAGE_FILE_DIR/test_disk_bad_tmp.img; printf '\xFF'; tail -c +460 $IMAGE_FILE_DIR/test_disk_bad_tmp.img; } > $IMAGE_FILE_DIR/test_disk_bad.img
rm $IMAGE_FILE_DIR/test_disk_bad_tmp.img


M module-vfs/tests/unittest_disk_manager.cpp => module-vfs/tests/unittest_disk_manager.cpp +44 -5
@@ 7,8 7,11 @@

namespace
{
    constexpr auto disk_image = "PurePhone.img";
}
    constexpr auto disk_image          = "PurePhone.img";
    constexpr auto part_disk_image     = "test_disk.img";
    constexpr auto part_disk_image_ext = "test_disk_ext.img";
    constexpr auto part_disk_image_bad = "test_disk_bad.img";
} // namespace
TEST_CASE("Registering and unregistering device")
{
    using namespace purefs;


@@ 29,16 32,16 @@ TEST_CASE("Parsing and checking partititons")
{
    using namespace purefs;
    blkdev::disk_manager dm;
    auto disk = std::make_shared<blkdev::disk_image>(disk_image);
    auto disk = std::make_shared<blkdev::disk_image>(part_disk_image);
    REQUIRE(disk);
    REQUIRE(dm.register_device(disk, "emmc0") == 0);
    const auto parts = dm.partitions("emmc0");
    REQUIRE(parts.size() > 1);
    REQUIRE(parts.size() == 3);
    unsigned long prev_start = 0;
    int num{};
    for (const auto &part : parts) {
        REQUIRE(part.physical_number > 0);
        REQUIRE(part.start_sector >= 2048);
        REQUIRE(part.start_sector > 0);
        REQUIRE(part.num_sectors > 0);
        REQUIRE(part.start_sector >= prev_start);
        REQUIRE(part.type > 0);


@@ 47,6 50,42 @@ TEST_CASE("Parsing and checking partititons")
    }
}

TEST_CASE("Parsing and checking extended partititons")
{
    using namespace purefs;
    blkdev::disk_manager dm;
    auto disk = std::make_shared<blkdev::disk_image>(part_disk_image_ext);
    REQUIRE(disk);
    REQUIRE(dm.register_device(disk, "emmc0") == 0);
    const auto parts = dm.partitions("emmc0");

    REQUIRE(parts.size() == 7);
    unsigned long prev_start = 0;
    int num{};
    for (const auto &part : parts) {
        REQUIRE(part.physical_number > 0);
        REQUIRE(part.start_sector >= 1);
        REQUIRE(part.num_sectors > 0);
        REQUIRE(part.num_sectors < 0xFF);
        REQUIRE(part.start_sector >= prev_start);
        REQUIRE(part.type > 0);
        REQUIRE(part.name == ("emmc0part" + std::to_string(num++)));
        prev_start = part.num_sectors + part.start_sector;
    }
}

TEST_CASE("Parsing and checking invalid partititons")
{
    using namespace purefs;
    blkdev::disk_manager dm;
    auto disk = std::make_shared<blkdev::disk_image>(part_disk_image_bad);
    REQUIRE(disk);
    REQUIRE(dm.register_device(disk, "emmc0") == 0);
    const auto parts = dm.partitions("emmc0");

    REQUIRE(parts.size() == 0);
}

TEST_CASE("RW boundary checking")
{
    using namespace purefs;