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

Commit 20f86733 authored by David Anderson's avatar David Anderson Committed by Automerger Merge Worker
Browse files

Merge "ueventd: Fix a race condition in handling device-mapper events." am:... 

Merge "ueventd: Fix a race condition in handling device-mapper events." am: 2fb1c671 am: fce7261d am: 026be5b7

Original change: https://android-review.googlesource.com/c/platform/system/core/+/2592609



Change-Id: I60ece89765a7fb4ececb238ea9adc337fa583621
Signed-off-by: default avatarAutomerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>
parents fc067bc4 026be5b7

fs_mgr/libdm/dm.cpp

+19 −0
Original line number Diff line number Diff line
@@ -243,6 +243,25 @@ bool DeviceMapper::GetDeviceUniquePath(const std::string& name, std::string* pat 
    return true;

}



bool DeviceMapper::GetDeviceNameAndUuid(dev_t dev, std::string* name, std::string* uuid) {

    struct dm_ioctl io;

    InitIo(&io, {});

    io.dev = dev;



    if (ioctl(fd_, DM_DEV_STATUS, &io) < 0) {

        PLOG(ERROR) << "Failed to find device dev: " << major(dev) << ":" << minor(dev);

        return false;

    }



    if (name) {

        *name = io.name;

    }

    if (uuid) {

        *uuid = io.uuid;

    }

    return true;

}



std::optional<DeviceMapper::Info> DeviceMapper::GetDetailedInfo(const std::string& name) const {

    struct dm_ioctl io;

    InitIo(&io, name);

fs_mgr/libdm/dm_test.cpp

+65 −29
Original line number Diff line number Diff line
@@ -30,6 +30,7 @@ 
#include <thread>



#include <android-base/file.h>

#include <android-base/logging.h>

#include <android-base/scopeguard.h>

#include <android-base/strings.h>

#include <android-base/unique_fd.h>
@@ -42,16 +43,40 @@ 
using namespace std;

using namespace std::chrono_literals;

using namespace android::dm;

using unique_fd = android::base::unique_fd;

using android::base::make_scope_guard;

using android::base::unique_fd;



TEST(libdm, HasMinimumTargets) {

class DmTest : public ::testing::Test {

  protected:

    void SetUp() override {

        const testing::TestInfo* const test_info =

                testing::UnitTest::GetInstance()->current_test_info();

        test_name_ = test_info->name();

        test_full_name_ = test_info->test_suite_name() + "/"s + test_name_;



        LOG(INFO) << "Starting test: " << test_full_name_;

    }

    void TearDown() override {

        LOG(INFO) << "Tearing down test: " << test_full_name_;



        auto& dm = DeviceMapper::Instance();

        ASSERT_TRUE(dm.DeleteDeviceIfExists(test_name_));



        LOG(INFO) << "Teardown complete for test: " << test_full_name_;

    }



    std::string test_name_;

    std::string test_full_name_;

};



TEST_F(DmTest, HasMinimumTargets) {

    DmTargetTypeInfo info;



    DeviceMapper& dm = DeviceMapper::Instance();

    ASSERT_TRUE(dm.GetTargetByName("linear", &info));

}



TEST(libdm, DmLinear) {

TEST_F(DmTest, DmLinear) {

    unique_fd tmp1(CreateTempFile("file_1", 4096));

    ASSERT_GE(tmp1, 0);

    unique_fd tmp2(CreateTempFile("file_2", 4096));
@@ -127,7 +152,7 @@ TEST(libdm, DmLinear) { 
    ASSERT_TRUE(dev.Destroy());

}



TEST(libdm, DmSuspendResume) {

TEST_F(DmTest, DmSuspendResume) {

    unique_fd tmp1(CreateTempFile("file_suspend_resume", 512));

    ASSERT_GE(tmp1, 0);
@@ -156,7 +181,7 @@ TEST(libdm, DmSuspendResume) { 
    ASSERT_EQ(dm.GetState(dev.name()), DmDeviceState::ACTIVE);

}



TEST(libdm, DmVerityArgsAvb2) {

TEST_F(DmTest, DmVerityArgsAvb2) {

    std::string device = "/dev/block/platform/soc/1da4000.ufshc/by-name/vendor_a";

    std::string algorithm = "sha1";

    std::string digest = "4be7e823b8c40f7bd5c8ccd5123f0722c5baca21";
@@ -178,7 +203,7 @@ TEST(libdm, DmVerityArgsAvb2) { 
    EXPECT_EQ(target.GetParameterString(), expected);

}



TEST(libdm, DmSnapshotArgs) {

TEST_F(DmTest, DmSnapshotArgs) {

    DmTargetSnapshot target1(0, 512, "base", "cow", SnapshotStorageMode::Persistent, 8);

    if (DmTargetSnapshot::ReportsOverflow("snapshot")) {

        EXPECT_EQ(target1.GetParameterString(), "base cow PO 8");
@@ -200,7 +225,7 @@ TEST(libdm, DmSnapshotArgs) { 
    EXPECT_EQ(target3.name(), "snapshot-merge");

}



TEST(libdm, DmSnapshotOriginArgs) {

TEST_F(DmTest, DmSnapshotOriginArgs) {

    DmTargetSnapshotOrigin target(0, 512, "base");

    EXPECT_EQ(target.GetParameterString(), "base");

    EXPECT_EQ(target.name(), "snapshot-origin");
@@ -330,7 +355,7 @@ bool CheckSnapshotAvailability() { 
    return true;

}



TEST(libdm, DmSnapshot) {

TEST_F(DmTest, DmSnapshot) {

    if (!CheckSnapshotAvailability()) {

        return;

    }
@@ -374,7 +399,7 @@ TEST(libdm, DmSnapshot) { 
    ASSERT_EQ(read, data);

}



TEST(libdm, DmSnapshotOverflow) {

TEST_F(DmTest, DmSnapshotOverflow) {

    if (!CheckSnapshotAvailability()) {

        return;

    }
@@ -421,7 +446,7 @@ TEST(libdm, DmSnapshotOverflow) { 
    }

}



TEST(libdm, ParseStatusText) {

TEST_F(DmTest, ParseStatusText) {

    DmTargetSnapshot::Status status;



    // Bad inputs
@@ -448,7 +473,7 @@ TEST(libdm, ParseStatusText) { 
    EXPECT_TRUE(DmTargetSnapshot::ParseStatusText("Overflow", &status));

}



TEST(libdm, DmSnapshotMergePercent) {

TEST_F(DmTest, DmSnapshotMergePercent) {

    DmTargetSnapshot::Status status;



    // Correct input
@@ -502,7 +527,7 @@ TEST(libdm, DmSnapshotMergePercent) { 
    EXPECT_LE(DmTargetSnapshot::MergePercent(status, 0), 0.0);

}



TEST(libdm, CryptArgs) {

TEST_F(DmTest, CryptArgs) {

    DmTargetCrypt target1(0, 512, "sha1", "abcdefgh", 50, "/dev/loop0", 100);

    ASSERT_EQ(target1.name(), "crypt");

    ASSERT_TRUE(target1.Valid());
@@ -518,7 +543,7 @@ TEST(libdm, CryptArgs) { 
              "iv_large_sectors sector_size:64");

}



TEST(libdm, DefaultKeyArgs) {

TEST_F(DmTest, DefaultKeyArgs) {

    DmTargetDefaultKey target(0, 4096, "aes-xts-plain64", "abcdef0123456789", "/dev/loop0", 0);

    target.SetSetDun();

    ASSERT_EQ(target.name(), "default-key");
@@ -529,7 +554,7 @@ TEST(libdm, DefaultKeyArgs) { 
              "iv_large_sectors");

}



TEST(libdm, DefaultKeyLegacyArgs) {

TEST_F(DmTest, DefaultKeyLegacyArgs) {

    DmTargetDefaultKey target(0, 4096, "AES-256-XTS", "abcdef0123456789", "/dev/loop0", 0);

    target.SetUseLegacyOptionsFormat();

    ASSERT_EQ(target.name(), "default-key");
@@ -537,7 +562,7 @@ TEST(libdm, DefaultKeyLegacyArgs) { 
    ASSERT_EQ(target.GetParameterString(), "AES-256-XTS abcdef0123456789 /dev/loop0 0");

}



TEST(libdm, DeleteDeviceWithTimeout) {

TEST_F(DmTest, DeleteDeviceWithTimeout) {

    unique_fd tmp(CreateTempFile("file_1", 4096));

    ASSERT_GE(tmp, 0);

    LoopDevice loop(tmp, 10s);
@@ -561,7 +586,7 @@ TEST(libdm, DeleteDeviceWithTimeout) { 
    ASSERT_EQ(ENOENT, errno);

}



TEST(libdm, IsDmBlockDevice) {

TEST_F(DmTest, IsDmBlockDevice) {

    unique_fd tmp(CreateTempFile("file_1", 4096));

    ASSERT_GE(tmp, 0);

    LoopDevice loop(tmp, 10s);
@@ -580,7 +605,7 @@ TEST(libdm, IsDmBlockDevice) { 
    ASSERT_FALSE(dm.IsDmBlockDevice(loop.device()));

}



TEST(libdm, GetDmDeviceNameByPath) {

TEST_F(DmTest, GetDmDeviceNameByPath) {

    unique_fd tmp(CreateTempFile("file_1", 4096));

    ASSERT_GE(tmp, 0);

    LoopDevice loop(tmp, 10s);
@@ -601,7 +626,7 @@ TEST(libdm, GetDmDeviceNameByPath) { 
    ASSERT_EQ("libdm-test-dm-linear", *name);

}



TEST(libdm, GetParentBlockDeviceByPath) {

TEST_F(DmTest, GetParentBlockDeviceByPath) {

    unique_fd tmp(CreateTempFile("file_1", 4096));

    ASSERT_GE(tmp, 0);

    LoopDevice loop(tmp, 10s);
@@ -621,7 +646,7 @@ TEST(libdm, GetParentBlockDeviceByPath) { 
    ASSERT_EQ(loop.device(), *sub_block_device);

}



TEST(libdm, DeleteDeviceDeferredNoReferences) {

TEST_F(DmTest, DeleteDeviceDeferredNoReferences) {

    unique_fd tmp(CreateTempFile("file_1", 4096));

    ASSERT_GE(tmp, 0);

    LoopDevice loop(tmp, 10s);
@@ -647,7 +672,7 @@ TEST(libdm, DeleteDeviceDeferredNoReferences) { 
    ASSERT_EQ(ENOENT, errno);

}



TEST(libdm, DeleteDeviceDeferredWaitsForLastReference) {

TEST_F(DmTest, DeleteDeviceDeferredWaitsForLastReference) {

    unique_fd tmp(CreateTempFile("file_1", 4096));

    ASSERT_GE(tmp, 0);

    LoopDevice loop(tmp, 10s);
@@ -682,7 +707,7 @@ TEST(libdm, DeleteDeviceDeferredWaitsForLastReference) { 
    ASSERT_EQ(ENOENT, errno);

}



TEST(libdm, CreateEmptyDevice) {

TEST_F(DmTest, CreateEmptyDevice) {

    DeviceMapper& dm = DeviceMapper::Instance();

    ASSERT_TRUE(dm.CreateEmptyDevice("empty-device"));

    auto guard =
@@ -692,9 +717,7 @@ TEST(libdm, CreateEmptyDevice) { 
    ASSERT_EQ(DmDeviceState::SUSPENDED, dm.GetState("empty-device"));

}



TEST(libdm, UeventAfterLoadTable) {

    static const char* kDeviceName = "libdm-test-uevent-load-table";



TEST_F(DmTest, UeventAfterLoadTable) {

    struct utsname u;

    ASSERT_EQ(uname(&u), 0);
@@ -706,18 +729,31 @@ TEST(libdm, UeventAfterLoadTable) { 
    }



    DeviceMapper& dm = DeviceMapper::Instance();

    ASSERT_TRUE(dm.CreateEmptyDevice(kDeviceName));

    ASSERT_TRUE(dm.CreateEmptyDevice(test_name_));



    DmTable table;

    table.Emplace<DmTargetError>(0, 1);

    ASSERT_TRUE(dm.LoadTable(kDeviceName, table));

    ASSERT_TRUE(dm.LoadTable(test_name_, table));



    std::string ignore_path;

    ASSERT_TRUE(dm.WaitForDevice(kDeviceName, 5s, &ignore_path));

    ASSERT_TRUE(dm.WaitForDevice(test_name_, 5s, &ignore_path));



    auto info = dm.GetDetailedInfo(kDeviceName);

    auto info = dm.GetDetailedInfo(test_name_);

    ASSERT_TRUE(info.has_value());

    ASSERT_TRUE(info->IsSuspended());



    ASSERT_TRUE(dm.DeleteDevice(kDeviceName));

    ASSERT_TRUE(dm.DeleteDevice(test_name_));

}



TEST_F(DmTest, GetNameAndUuid) {

    auto& dm = DeviceMapper::Instance();

    ASSERT_TRUE(dm.CreatePlaceholderDevice(test_name_));



    dev_t dev;

    ASSERT_TRUE(dm.GetDeviceNumber(test_name_, &dev));



    std::string name, uuid;

    ASSERT_TRUE(dm.GetDeviceNameAndUuid(dev, &name, &uuid));

    ASSERT_EQ(name, test_name_);

    ASSERT_FALSE(uuid.empty());

}

fs_mgr/libdm/include/libdm/dm.h

+2 −0
Original line number Diff line number Diff line
@@ -298,6 +298,8 @@ class DeviceMapper final : public IDeviceMapper { 
    // a placeholder table containing dm-error.

    bool CreatePlaceholderDevice(const std::string& name);



    bool GetDeviceNameAndUuid(dev_t dev, std::string* name, std::string* uuid);



  private:

    // Maximum possible device mapper targets registered in the kernel.

    // This is only used to read the list of targets from kernel so we allocate

init/devices.cpp

+8 −10
Original line number Diff line number Diff line
@@ -32,6 +32,7 @@ 
#include <android-base/logging.h>

#include <android-base/stringprintf.h>

#include <android-base/strings.h>

#include <libdm/dm.h>

#include <private/android_filesystem_config.h>

#include <selinux/android.h>

#include <selinux/selinux.h>
@@ -112,17 +113,14 @@ static bool FindVbdDevicePrefix(const std::string& path, std::string* result) { 
// the supplied buffer with the dm module's instantiated name.

// If it doesn't start with a virtual block device, or there is some

// error, return false.

static bool FindDmDevice(const std::string& path, std::string* name, std::string* uuid) {

    if (!StartsWith(path, "/devices/virtual/block/dm-")) return false;

static bool FindDmDevice(const Uevent& uevent, std::string* name, std::string* uuid) {

    if (!StartsWith(uevent.path, "/devices/virtual/block/dm-")) return false;

    if (uevent.action == "remove") return false;  // Avoid error spam from ioctl



    if (!ReadFileToString("/sys" + path + "/dm/name", name)) {

        return false;

    }

    ReadFileToString("/sys" + path + "/dm/uuid", uuid);

    dev_t dev = makedev(uevent.major, uevent.minor);



    *name = android::base::Trim(*name);

    *uuid = android::base::Trim(*uuid);

    return true;

    auto& dm = android::dm::DeviceMapper::Instance();

    return dm.GetDeviceNameAndUuid(dev, name, uuid);

}



Permissions::Permissions(const std::string& name, mode_t perm, uid_t uid, gid_t gid,
@@ -392,7 +390,7 @@ std::vector<std::string> DeviceHandler::GetBlockDeviceSymlinks(const Uevent& uev 
        type = "pci";

    } else if (FindVbdDevicePrefix(uevent.path, &device)) {

        type = "vbd";

    } else if (FindDmDevice(uevent.path, &partition, &uuid)) {

    } else if (FindDmDevice(uevent, &partition, &uuid)) {

        std::vector<std::string> symlinks = {"/dev/block/mapper/" + partition};

        if (!uuid.empty()) {

            symlinks.emplace_back("/dev/block/mapper/by-uuid/" + uuid);