Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 451694e2 authored by David Anderson's avatar David Anderson
Browse files

liblp: Make it easier to test UpdatePartitionTable. 

This change makes the internal UpdatePartitionTable function more
testable by parameterizing its write functions. It also adds two tests,
one of which exposes a flaw in the current implementation.

Bug: 79173901
Test: liblp_test gtest
Change-Id: I3c4112794b97d577a27f035baeac2d42ac75f552
parent 21671eda

fs_mgr/liblp/Android.bp

+3 −0
Original line number Diff line number Diff line
@@ -46,6 +46,9 @@ cc_library_static { 
cc_test {

    name: "liblp_test",

    defaults: ["fs_mgr_defaults"],

    cppflags: [

        "-Wno-unused-parameter",

    ],

    static_libs: [

        "libbase",

        "liblog",

fs_mgr/liblp/include/liblp/writer.h

+4 −0
Original line number Diff line number Diff line
@@ -17,6 +17,7 @@ 
#ifndef LIBLP_WRITER_H

#define LIBLP_WRITER_H



#include <functional>

#include "metadata_format.h"



namespace android {
@@ -43,6 +44,9 @@ bool UpdatePartitionTable(const std::string& block_device, const LpMetadata& met 
bool FlashPartitionTable(int fd, const LpMetadata& metadata, uint32_t slot_number);

bool UpdatePartitionTable(int fd, const LpMetadata& metadata, uint32_t slot_number);



bool UpdatePartitionTable(int fd, const LpMetadata& metadata, uint32_t slot_number,

                          std::function<bool(int, const std::string&)> writer);



// Helper function to serialize geometry and metadata to a normal file, for

// flashing or debugging.

bool WriteToImageFile(const char* file, const LpMetadata& metadata);

fs_mgr/liblp/io_test.cpp

+82 −0
Original line number Diff line number Diff line
@@ -393,3 +393,85 @@ TEST(liblp, ImageFiles) { 
    unique_ptr<LpMetadata> imported = ReadFromImageFile(fd);

    ASSERT_NE(imported, nullptr);

}



class BadWriter {

  public:

    // When requested, write garbage instead of the requested bytes, then

    // return false.

    bool operator()(int fd, const std::string& blob) {

        if (++write_count_ == fail_on_write_) {

            std::unique_ptr<char[]> new_data = std::make_unique<char[]>(blob.size());

            memset(new_data.get(), 0xe5, blob.size());

            EXPECT_TRUE(android::base::WriteFully(fd, new_data.get(), blob.size()));

            return false;

        } else {

            return android::base::WriteFully(fd, blob.data(), blob.size());

        }

    }

    void FailOnWrite(int number) {

        fail_on_write_ = number;

        write_count_ = 0;

    }



  private:

    int fail_on_write_ = 0;

    int write_count_ = 0;

};



// Test that an interrupted flash operation on the "primary" copy of metadata

// is not fatal.

TEST(liblp, FlashPrimaryMetadataFailure) {

    // Initial state.

    unique_fd fd = CreateFlashedDisk();

    ASSERT_GE(fd, 0);



    BadWriter writer;



    // Read and write it back.

    writer.FailOnWrite(1);

    unique_ptr<LpMetadata> imported = ReadMetadata(fd, 0);

    ASSERT_NE(imported, nullptr);

    ASSERT_FALSE(UpdatePartitionTable(fd, *imported.get(), 0, writer));



    // We should still be able to read the backup copy.

    imported = ReadMetadata(fd, 0);

    ASSERT_NE(imported, nullptr);



    // Flash again, this time fail the backup copy. We should still be able

    // to read the primary.

    writer.FailOnWrite(2);

    ASSERT_FALSE(UpdatePartitionTable(fd, *imported.get(), 0, writer));

    imported = ReadMetadata(fd, 0);

    ASSERT_NE(imported, nullptr);

}



// Test that an interrupted flash operation on the "backup" copy of metadata

// is not fatal.

TEST(liblp, FlashBackupMetadataFailure) {

    // Initial state.

    unique_fd fd = CreateFlashedDisk();

    ASSERT_GE(fd, 0);



    BadWriter writer;



    // Read and write it back.

    writer.FailOnWrite(2);

    unique_ptr<LpMetadata> imported = ReadMetadata(fd, 0);

    ASSERT_NE(imported, nullptr);

    ASSERT_FALSE(UpdatePartitionTable(fd, *imported.get(), 0, writer));



    // We should still be able to read the primary copy.

    imported = ReadMetadata(fd, 0);

    ASSERT_NE(imported, nullptr);



    // Flash again, this time fail the primary copy. We should still be able

    // to read the primary.

    //

    // TODO(dvander): This is currently not handled correctly. liblp does not

    // guarantee both copies are in sync before the update. The ASSERT_EQ

    // will change to an ASSERT_NE when this is fixed.

    writer.FailOnWrite(1);

    ASSERT_FALSE(UpdatePartitionTable(fd, *imported.get(), 0, writer));

    imported = ReadMetadata(fd, 0);

    ASSERT_EQ(imported, nullptr);

}

fs_mgr/liblp/writer.cpp

+16 −6
Original line number Diff line number Diff line
@@ -130,8 +130,13 @@ static bool ValidateAndSerializeMetadata(int fd, const LpMetadata& metadata, std 
    return true;

}



static bool DefaultWriter(int fd, const std::string& blob) {

    return android::base::WriteFully(fd, blob.data(), blob.size());

}



static bool WriteMetadata(int fd, const LpMetadataGeometry& geometry, uint32_t slot_number,

                          const std::string& blob) {

                          const std::string& blob,

                          std::function<bool(int, const std::string&)> writer) {

    // Make sure we're writing to a valid metadata slot.

    if (slot_number >= geometry.metadata_slot_count) {

        LERROR << "Invalid logical partition metadata slot number.";
@@ -144,7 +149,7 @@ static bool WriteMetadata(int fd, const LpMetadataGeometry& geometry, uint32_t s 
        PERROR << __PRETTY_FUNCTION__ << "lseek failed: offset " << primary_offset;

        return false;

    }

    if (!android::base::WriteFully(fd, blob.data(), blob.size())) {

    if (!writer(fd, blob)) {

        PERROR << __PRETTY_FUNCTION__ << "write " << blob.size() << " bytes failed";

        return false;

    }
@@ -161,7 +166,7 @@ static bool WriteMetadata(int fd, const LpMetadataGeometry& geometry, uint32_t s 
               << " is within logical partition bounds, sector " << geometry.last_logical_sector;

        return false;

    }

    if (!android::base::WriteFully(fd, blob.data(), blob.size())) {

    if (!writer(fd, blob)) {

        PERROR << __PRETTY_FUNCTION__ << "backup write " << blob.size() << " bytes failed";

        return false;

    }
@@ -197,10 +202,11 @@ bool FlashPartitionTable(int fd, const LpMetadata& metadata, uint32_t slot_numbe 
    }



    // Write metadata to the correct slot, now that geometry is in place.

    return WriteMetadata(fd, metadata.geometry, slot_number, metadata_blob);

    return WriteMetadata(fd, metadata.geometry, slot_number, metadata_blob, DefaultWriter);

}



bool UpdatePartitionTable(int fd, const LpMetadata& metadata, uint32_t slot_number) {

bool UpdatePartitionTable(int fd, const LpMetadata& metadata, uint32_t slot_number,

                          std::function<bool(int, const std::string&)> writer) {

    // Before writing geometry and/or logical partition tables, perform some

    // basic checks that the geometry and tables are coherent, and will fit

    // on the given block device.
@@ -221,7 +227,7 @@ bool UpdatePartitionTable(int fd, const LpMetadata& metadata, uint32_t slot_numb 
        LERROR << "Incompatible geometry in new logical partition metadata";

        return false;

    }

    return WriteMetadata(fd, geometry, slot_number, blob);

    return WriteMetadata(fd, geometry, slot_number, blob, writer);

}



bool FlashPartitionTable(const std::string& block_device, const LpMetadata& metadata,
@@ -244,6 +250,10 @@ bool UpdatePartitionTable(const std::string& block_device, const LpMetadata& met 
    return UpdatePartitionTable(fd, metadata, slot_number);

}



bool UpdatePartitionTable(int fd, const LpMetadata& metadata, uint32_t slot_number) {

    return UpdatePartitionTable(fd, metadata, slot_number, DefaultWriter);

}



bool WriteToImageFile(int fd, const LpMetadata& input) {

    std::string geometry = SerializeGeometry(input.geometry);

    std::string padding(LP_METADATA_GEOMETRY_SIZE - geometry.size(), '\0');