From ef6dd7d3ded3ffb200e43152106edff9aeacb02f Mon Sep 17 00:00:00 2001
From: Zach Anderson <zra@google.com>
Date: Thu, 25 Apr 2019 21:35:53 +0000
Subject: [PATCH] [zircon] Revert "[zircon] Reformat zxcrypt and minfs
partitions when the partition is"
This reverts commit 2dcfdba2e3fe666404a8b8fafc5574cfed66056c.
Reason for revert: Failed boot on vim2
Original change's description:
> [zircon] Reformat zxcrypt and minfs partitions when the partition is
> missing. Added a unit test that failed without the changes.
>
> Change-Id: Idaf9913bf9006384e021f39047df1e8e8a855369
TBR=smklein@google.com,aarongreen@google.com,abarth@google.com,zarvox@google.com,jseibert@google.com
Change-Id: I0e755d84dc4839e8968308ca753aa1d9c9c5038b
---
.../core/devmgr/fshost/block-watcher.cpp | 51 -----
.../include/kms-stateless/kms-stateless.h | 2 +-
.../ulib/kms-stateless/kms-stateless.cpp | 2 +-
zircon/system/ulib/zxcrypt/fdio-volume.cpp | 12 +-
zircon/system/utest/BUILD.gn | 1 -
zircon/system/utest/fs-recovery/BUILD.gn | 31 ----
zircon/system/utest/fs-recovery/recovery.cpp | 175 ------------------
7 files changed, 8 insertions(+), 266 deletions(-)
delete mode 100644 zircon/system/utest/fs-recovery/BUILD.gn
delete mode 100644 zircon/system/utest/fs-recovery/recovery.cpp
diff --git a/zircon/system/core/devmgr/fshost/block-watcher.cpp b/zircon/system/core/devmgr/fshost/block-watcher.cpp
index 77e518e9989..7aa1582c830 100644
--- a/zircon/system/core/devmgr/fshost/block-watcher.cpp
+++ b/zircon/system/core/devmgr/fshost/block-watcher.cpp
@@ -494,53 +494,6 @@ int UnsealZxcrypt(void* arg) {
return 0;
}
-disk_format_t ReformatDataPartition(fbl::unique_fd fd, zx_handle_t disk_channel,
- const char* device_path) {
- zx_status_t call_status;
- fbl::StringBuffer<PATH_MAX> path;
- path.Resize(path.capacity());
- size_t path_len;
- // Both the zxcrypt and minfs partitions have the same gpt guid, so here we
- // determine which one we actually need to format. We do this by looking up
- // the topological path, if it is the zxcrypt driver, then we format it as
- // minfs, otherwise as zxcrypt.
- if (fuchsia_device_ControllerGetTopologicalPath(disk_channel, &call_status, path.data(),
- path.capacity(), &path_len) != ZX_OK) {
- return DISK_FORMAT_UNKNOWN;
- }
- const fbl::StringPiece kZxcryptPath("/zxcrypt/unsealed/block");
- if (fbl::StringPiece(path.begin() + path_len - kZxcryptPath.length()).compare(kZxcryptPath) == 0) {
- printf("fshost: Minfs data partition is corrupt. Will attempt to reformat %s\n", device_path);
- if (mkfs(device_path, DISK_FORMAT_MINFS, launch_stdio_sync, &default_mkfs_options) == ZX_OK) {
- return DISK_FORMAT_MINFS;
- }
- } else {
- printf("fshost: zxcrypt volume is corrupt. Will attempt to reformat %s\n", device_path);
- if (zxcrypt::FdioVolume::CreateWithDeviceKey(std::move(fd), nullptr) == ZX_OK) {
- return DISK_FORMAT_ZXCRYPT;
- }
- }
- return DISK_FORMAT_UNKNOWN;
-}
-
-// Attempts to reformat the partition at the device path. Returns the specific
-// disk format if successful and unknown otherwise. Currently only works for
-// minfs and zxcrypt data partitions.
-disk_format_t ReformatPartition(fbl::unique_fd fd, zx_handle_t disk_channel,
- const char* device_path) {
- zx_status_t call_status, io_status;
- fuchsia_hardware_block_partition_GUID guid;
- io_status = fuchsia_hardware_block_partition_PartitionGetTypeGuid(disk_channel, &call_status,
- &guid);
- if (io_status != ZX_OK || call_status != ZX_OK) {
- return DISK_FORMAT_UNKNOWN;
- }
- if (gpt_is_data_guid(guid.value, GPT_GUID_LEN)) {
- return ReformatDataPartition(std::move(fd), disk_channel, device_path);
- }
- return DISK_FORMAT_UNKNOWN;
-}
-
zx_status_t FormatMinfs(const fbl::unique_fd& block_device,
const fuchsia_hardware_block_BlockInfo& info) {
@@ -590,10 +543,6 @@ zx_status_t BlockDeviceAdded(int dirfd, int event, const char* name, void* cooki
return ZX_OK;
}
- if (df == DISK_FORMAT_UNKNOWN && !watcher->Netbooting()) {
- df = ReformatPartition(fd.duplicate(), disk->get(), device_path);
- }
-
if (info.flags & BLOCK_FLAG_BOOTPART) {
fuchsia_device_ControllerBind(disk->get(), BOOTPART_DRIVER_LIB,
STRLEN(BOOTPART_DRIVER_LIB), &call_status);
diff --git a/zircon/system/ulib/kms-stateless/include/kms-stateless/kms-stateless.h b/zircon/system/ulib/kms-stateless/include/kms-stateless/kms-stateless.h
index f0960fb1598..7d93c82f880 100644
--- a/zircon/system/ulib/kms-stateless/include/kms-stateless/kms-stateless.h
+++ b/zircon/system/ulib/kms-stateless/include/kms-stateless/kms-stateless.h
@@ -11,7 +11,7 @@ namespace kms_stateless {
// The callback called when a hardware key is successfully derived. Arguments to the callback
// is a unique_ptr of the key buffer and the key size.
- using GetHardwareDerivedKeyCallback = fbl::Function<zx_status_t(fbl::unique_ptr<uint8_t[]>,
+ using GetHardwareDerivedKeyCallback = fbl::Function<zx_status_t(fbl::unique_ptr<uint8_t>,
size_t)>;
// Get a hardware derived key using
diff --git a/zircon/system/ulib/kms-stateless/kms-stateless.cpp b/zircon/system/ulib/kms-stateless/kms-stateless.cpp
index f5d68ec0333..fe7d295f874 100644
--- a/zircon/system/ulib/kms-stateless/kms-stateless.cpp
+++ b/zircon/system/ulib/kms-stateless/kms-stateless.cpp
@@ -148,7 +148,7 @@ zx_status_t WatchTee(int dirfd, int event, const char* filename, void* cookie) {
fbl::StringBuffer<kMaxPathLen> device_path;
device_path.Append(kDeviceClass).Append("/").Append(filename);
// Hardware derived key is expected to be 128-bit AES key.
- fbl::unique_ptr<uint8_t[]> key_buffer(new uint8_t[kDerivedKeySize]);
+ fbl::unique_ptr<uint8_t> key_buffer(new uint8_t[kDerivedKeySize]);
size_t key_size = 0;
WatchTeeArgs* args = reinterpret_cast<WatchTeeArgs*>(cookie);
if (!GetKeyFromTeeDevice(device_path.c_str(), std::move(args->key_info), args->key_info_size,
diff --git a/zircon/system/ulib/zxcrypt/fdio-volume.cpp b/zircon/system/ulib/zxcrypt/fdio-volume.cpp
index 291f222467d..b97dd2fbf99 100644
--- a/zircon/system/ulib/zxcrypt/fdio-volume.cpp
+++ b/zircon/system/ulib/zxcrypt/fdio-volume.cpp
@@ -77,7 +77,7 @@ zx_status_t SelectKeySource(KeySource* out) {
}
}
-zx_status_t DoWithHardwareKey(fbl::Function<zx_status_t(fbl::unique_ptr<uint8_t[]>, size_t)>
+zx_status_t DoWithHardwareKey(fbl::Function<zx_status_t(fbl::unique_ptr<uint8_t>, size_t)>
callback) {
KeySource source;
zx_status_t rc;
@@ -91,13 +91,13 @@ zx_status_t DoWithHardwareKey(fbl::Function<zx_status_t(fbl::unique_ptr<uint8_t[
uint8_t key_info[kms_stateless::kExpectedKeyInfoSize] = {0};
memcpy(key_info, kHardwareKeyInfo, sizeof(kHardwareKeyInfo));
return kms_stateless::GetHardwareDerivedKey([&](
- fbl::unique_ptr<uint8_t[]> key_buffer, size_t key_size) {
+ fbl::unique_ptr<uint8_t> key_buffer, size_t key_size) {
return callback(std::move(key_buffer), key_size);
}, key_info);
}
case NullSource: {
// If we don't have secure storage for the root key, just use the null key
- fbl::unique_ptr<uint8_t[]> key_buffer(new uint8_t[kKeyLength]);
+ fbl::unique_ptr<uint8_t> key_buffer(new uint8_t[kKeyLength]);
memset(key_buffer.get(), 0, kKeyLength);
return callback(std::move(key_buffer), kKeyLength);
}
@@ -125,7 +125,7 @@ zx_status_t FdioVolumeManager::Unseal(const uint8_t* key, size_t key_len, uint8_
}
zx_status_t FdioVolumeManager::UnsealWithDeviceKey(uint8_t slot) {
- return DoWithHardwareKey([&](fbl::unique_ptr<uint8_t[]> key_buffer, size_t key_size) {
+ return DoWithHardwareKey([&](fbl::unique_ptr<uint8_t> key_buffer, size_t key_size) {
return Unseal(key_buffer.release(), key_size, slot);
});
}
@@ -199,7 +199,7 @@ zx_status_t FdioVolume::Create(fbl::unique_fd fd, const crypto::Secret& key,
zx_status_t FdioVolume::CreateWithDeviceKey(fbl::unique_fd&& fd,
fbl::unique_ptr<FdioVolume>* out) {
- return DoWithHardwareKey([&](fbl::unique_ptr<uint8_t[]> key_buffer, size_t key_size) {
+ return DoWithHardwareKey([&](fbl::unique_ptr<uint8_t> key_buffer, size_t key_size) {
crypto::Secret secret;
zx_status_t rc;
uint8_t* inner;
@@ -235,7 +235,7 @@ zx_status_t FdioVolume::Unlock(fbl::unique_fd fd, const crypto::Secret& key, key
zx_status_t FdioVolume::UnlockWithDeviceKey(fbl::unique_fd fd,
key_slot_t slot,
fbl::unique_ptr<FdioVolume>* out) {
- return DoWithHardwareKey([&](fbl::unique_ptr<uint8_t[]> key_buffer, size_t key_size) {
+ return DoWithHardwareKey([&](fbl::unique_ptr<uint8_t> key_buffer, size_t key_size) {
crypto::Secret secret;
zx_status_t rc;
uint8_t* inner;
diff --git a/zircon/system/utest/BUILD.gn b/zircon/system/utest/BUILD.gn
index 32d31a01a1c..f2513f3a834 100644
--- a/zircon/system/utest/BUILD.gn
+++ b/zircon/system/utest/BUILD.gn
@@ -107,7 +107,6 @@ if (current_cpu != "") {
"fs",
"fs-bench",
"fs-management",
- "fs-recovery",
"fs-test-utils",
"fs-vnode",
"futex-ownership",
diff --git a/zircon/system/utest/fs-recovery/BUILD.gn b/zircon/system/utest/fs-recovery/BUILD.gn
deleted file mode 100644
index 4911a286230..00000000000
--- a/zircon/system/utest/fs-recovery/BUILD.gn
+++ /dev/null
@@ -1,31 +0,0 @@
-# Copyright 2019 The Fuchsia Authors. All rights reserved.
-# Use of this source code is governed by a BSD-style license that can be
-# found in the LICENSE file.
-
-import("$zx/public/gn/resource.gni")
-
-generated_resource("zxcrypt_config.txt") {
- testonly = true
- contents = "null"
- outputs = [
- "config/zxcrypt",
- ]
-}
-
-
-test("fs-recovery") {
- sources = [
- "recovery.cpp",
- ]
- deps = [
- "$zx/system/ulib/devmgr-integration-test",
- "$zx/system/ulib/fbl",
- "$zx/system/ulib/fdio",
- "$zx/system/ulib/fs-management",
- "$zx/system/ulib/fvm",
- "$zx/system/fidl/fuchsia-hardware-ramdisk:c",
- "$zx/system/ulib/unittest",
- "$zx/system/ulib/zx",
- ":zxcrypt_config.txt",
- ]
-}
diff --git a/zircon/system/utest/fs-recovery/recovery.cpp b/zircon/system/utest/fs-recovery/recovery.cpp
deleted file mode 100644
index bb69561a1ca..00000000000
--- a/zircon/system/utest/fs-recovery/recovery.cpp
+++ /dev/null
@@ -1,175 +0,0 @@
-// Copyright 2019 The Fuchsia Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-#include <fcntl.h>
-
-#include <fs-management/fvm.h>
-#include <fs-management/mount.h>
-#include <fuchsia/hardware/ramdisk/c/fidl.h>
-#include <fvm/format.h>
-#include <lib/devmgr-integration-test/fixture.h>
-#include <lib/fdio/fd.h>
-#include <lib/fdio/fdio.h>
-#include <lib/zx/vmo.h>
-#include <unittest/unittest.h>
-
-namespace {
-
-using devmgr_integration_test::IsolatedDevmgr;
-
-const uint32_t kBlockCount = 1024 * 256;
-const uint32_t kBlockSize = 512;
-const uint32_t kSliceSize = (1 << 20);
-const size_t kDeviceSize = kBlockCount * kBlockSize;
-const char* kDataName = "fs-recovery-data";
-const char* kRamdiskPath = "misc/ramctl";
-
-// Test fixture that builds a ramdisk and destroys it when destructed.
-class FsRecoveryTest {
-public:
- // Create an IsolatedDevmgr that can load device drivers such as fvm,
- // zxcrypt, etc.
- bool Initialize() {
- BEGIN_HELPER;
- auto args = IsolatedDevmgr::DefaultArgs();
- args.disable_block_watcher = false;
- args.sys_device_driver = devmgr_integration_test::IsolatedDevmgr::kSysdevDriver;
- args.load_drivers.push_back(devmgr_integration_test::IsolatedDevmgr::kSysdevDriver);
- args.driver_search_paths.push_back("/boot/driver");
- ASSERT_EQ(IsolatedDevmgr::Create(std::move(args), &devmgr_), ZX_OK);
- END_HELPER;
- }
-
- // Create a ram disk that is back by a VMO, which is formatted to look like
- // an FVM volume.
- bool CreateFvmRamdisk(size_t device_size, size_t block_size) {
- BEGIN_HELPER;
-
- // Calculate total size of data + metadata.
- device_size = fbl::round_up(device_size, fvm::kBlockSize);
- size_t old_meta = fvm::MetadataSize(device_size, fvm::kBlockSize);
- size_t new_meta = fvm::MetadataSize(old_meta + device_size, fvm::kBlockSize);
- while (old_meta != new_meta) {
- old_meta = new_meta;
- new_meta = fvm::MetadataSize(old_meta + device_size, fvm::kBlockSize);
- }
- device_size = device_size + (new_meta * 2);
-
- zx::vmo disk;
- ASSERT_EQ(zx::vmo::create(device_size, 0, &disk), ZX_OK);
- int fd = -1;
- ASSERT_EQ(fdio_fd_create(disk.get(), &fd), ZX_OK);
- ASSERT_GE(fd, 0);
- ASSERT_EQ(fvm_init_with_size(fd, device_size, kSliceSize), ZX_OK);
-
- fbl::unique_fd ramdisk;
- ASSERT_TRUE(WaitForDevice(kRamdiskPath, &ramdisk));
-
- zx_handle_t ramctl;
- ASSERT_EQ(fdio_get_service_handle(ramdisk.get(), &ramctl), ZX_OK);
-
- size_t name_len = 0;
- zx_status_t status;
- ASSERT_EQ(fuchsia_hardware_ramdisk_RamdiskControllerCreateFromVmo(
- ramctl, disk.release(), &status, ramdisk_name_,
- sizeof(ramdisk_name_) - 1, &name_len),
- ZX_OK);
- ASSERT_EQ(status, ZX_OK);
- END_HELPER;
- }
-
- // Create a partition in the FVM volume that has the data guid.
- bool CreateFvmPartition(char* fvm_path) {
- BEGIN_HELPER;
-
- snprintf(fvm_path, PATH_MAX, "%s/%s/block/fvm", kRamdiskPath, ramdisk_name_);
- fbl::unique_fd fvm_fd;
- ASSERT_TRUE(WaitForDevice(fvm_path, &fvm_fd));
-
- // Allocate a FVM partition with the data guid but don't actually format the
- // partition.
- alloc_req_t req;
- memset(&req, 0, sizeof(alloc_req_t));
- req.slice_count = 1;
- static const uint8_t data_guid[GPT_GUID_LEN] = GUID_DATA_VALUE;
- memcpy(req.type, data_guid, GUID_LEN);
- snprintf(req.name, NAME_LEN, "%s", kDataName);
- // This attempts to return an fd to a partition under /dev/class/block
- // which is not running under the IsolatedDevmgr, thus we do not check
- // its return value.
- fvm_allocate_partition(fvm_fd.get(), &req);
-
- END_HELPER;
- }
-
- // Wait for the device to be available and then check to make sure it is
- // formatted of the passed in type. Since formatting can take some time
- // after the device becomes available, we must recheck.
- bool WaitForDiskFormat(const char* path, disk_format_t format, zx::duration deadline) {
- fbl::unique_fd fd;
- if (!WaitForDevice(path, &fd)) {
- return false;
- }
- while (deadline.get() > 0) {
- fd.reset(openat(devmgr_.devfs_root().get(), path, O_RDONLY));
- if (detect_disk_format(fd.get()) == format)
- return true;
- sleep(1);
- deadline -= zx::duration(ZX_SEC(1));
- }
- return false;
- }
-
-private:
- bool WaitForDevice(const char* path, fbl::unique_fd* fd) {
- BEGIN_HELPER;
- printf("Wait for device %s\n", path);
- ASSERT_EQ(devmgr_integration_test::RecursiveWaitForFile(devmgr_.devfs_root(), path,
- zx::deadline_after(zx::sec(5)), fd),
- ZX_OK);
-
- ASSERT_TRUE(*fd);
- END_HELPER;
- }
-
- char ramdisk_name_[fuchsia_hardware_ramdisk_MAX_NAME_LENGTH + 1];
- devmgr_integration_test::IsolatedDevmgr devmgr_;
-};
-
-bool EmptyPartitionRecoveryTest() {
- BEGIN_TEST;
-
- char fvm_path[PATH_MAX];
- fbl::unique_ptr<FsRecoveryTest> recovery(new FsRecoveryTest());
- ASSERT_TRUE(recovery->Initialize());
- // Creates an FVM partition under an isolated devmgr, but does not properly
- // create the data partitions.
- ASSERT_TRUE(recovery->CreateFvmRamdisk(kDeviceSize, kBlockSize));
- ASSERT_TRUE(recovery->CreateFvmPartition(fvm_path));
-
- // We then expect the devmgr to self-recover, i.e., format the zxcrypt/data
- // partitions as expected from the FVM partition.
-
- // First, wait for the zxcrypt partition to be formatted.
- char fvm_block_path[PATH_MAX];
- snprintf(fvm_block_path, sizeof(fvm_block_path),
- "%s/%s-p-1/block", fvm_path, kDataName);
- EXPECT_TRUE(recovery->WaitForDiskFormat(fvm_block_path, DISK_FORMAT_ZXCRYPT,
- zx::duration(ZX_SEC(3))));
-
- // Second, wait for the data partition to be formatted.
- char data_path[PATH_MAX];
- snprintf(data_path, sizeof(data_path),
- "%s/zxcrypt/unsealed/block", fvm_block_path);
- EXPECT_TRUE(recovery->WaitForDiskFormat(data_path, DISK_FORMAT_MINFS,
- zx::duration(ZX_SEC(3))));
-
- END_TEST;
-}
-
-BEGIN_TEST_CASE(FsRecoveryTest)
-RUN_TEST(EmptyPartitionRecoveryTest)
-END_TEST_CASE(FsRecoveryTest)
-
-} // namespace
--
GitLab