verity: do all hashing in dm-bht.c
We were doing the hashing of the disk blocks in dm-verity.c and hashing
hash blocks in dm-bht.c. We can simplifiy the code by doing all the
hashing in dm-bht.c. We couldn't do this earlier because we had
to handle unaligned reads (no longer an issue).
I also removed the BUG_ON in dm_bht_get_node because I wanted to use it
in dm_bht_check_block with depth == bht->depth.
The kernel piece has already been reviewed and submitted. What is different
in this CL are the modifications to the tests.
BUG=9752
TEST=Ran dm-verity.git unit tests. Ran platform_DMVerityCorruption on H/W.
kernel.git Review URL: http://codereview.chromium.org/6626037
Change-Id: Ic817a53f383f7727742c77c2064cdb0afe595c43
R=wad@chromium.org,taysom@chromium.org,ups@chromium.org
Review URL: http://codereview.chromium.org/6695038
diff --git a/dm-bht.c b/dm-bht.c
index 35713de..c9d1fca 100644
--- a/dm-bht.c
+++ b/dm-bht.c
@@ -163,9 +163,6 @@
unsigned int block_index)
{
unsigned int index = dm_bht_index_at_level(bht, depth, block_index);
- struct dm_bht_level *level = dm_bht_get_level(bht, depth);
-
- BUG_ON(index >= level->count);
return dm_bht_node(bht, entry, index % bht->node_count);
}
@@ -588,34 +585,28 @@
return 0;
}
-/* digest length and bht must be checked already */
+/* Check the disk block against the leaf hash. */
static int dm_bht_check_block(struct dm_bht *bht, unsigned int block_index,
- u8 *digest, int *entry_state)
+ const void *block, int *entry_state)
{
- int depth;
- unsigned int index;
- struct dm_bht_entry *entry;
-
- /* The leaves contain the block hashes */
- depth = bht->depth - 1;
-
- /* Index into the bottom level. Each entry in this level contains
- * nodes whose hashes are the direct hashes of one block of data on
- * disk.
- */
- index = block_index >> bht->node_count_shift;
- entry = &bht->levels[depth].entries[index];
+ unsigned int depth = bht->depth;
+ struct dm_bht_entry *parent = dm_bht_get_entry(bht, depth - 1,
+ block_index);
+ struct hash_desc *hash_desc = &bht->hash_desc[smp_processor_id()];
+ u8 digest[DM_BHT_MAX_DIGEST_SIZE];
+ u8 *node;
/* This call is only safe if all nodes along the path
* are already populated (i.e. READY) via dm_bht_populate.
*/
- BUG_ON(atomic_read(&entry->state) < DM_BHT_ENTRY_READY);
+ *entry_state = atomic_read(&parent->state);
+ BUG_ON(*entry_state < DM_BHT_ENTRY_READY);
- /* Index into the entry data */
- index = (block_index % bht->node_count) * bht->digest_size;
- if (memcmp(&entry->nodes[index], digest, bht->digest_size)) {
- DMCRIT("digest mismatch for block %u", block_index);
- dm_bht_log_mismatch(bht, &entry->nodes[index], digest);
+ node = dm_bht_get_node(bht, parent, depth, block_index);
+ if (dm_bht_compute_hash(bht, hash_desc, virt_to_page(block), digest) ||
+ dm_bht_compare_hash(bht, digest, node)) {
+ DMERR("failed to verify entry's hash against parent "
+ "(d=%u,bi=%u)", depth, block_index);
return DM_BHT_ENTRY_ERROR_MISMATCH;
}
return 0;
@@ -1061,25 +1052,18 @@
* dm_bht_verify_block - checks that all nodes in the path for @block are valid
* @bht: pointer to a dm_bht_create()d bht
* @block_index:specific block data is expected from
- * @digest: computed digest for the given block to be checked
- * @digest_len: length of @digest
+ * @block: virtual address of the block data in memory
*
* Returns 0 on success, 1 on missing data, and a negative error
* code on verification failure. All supporting functions called
* should return similarly.
*/
int dm_bht_verify_block(struct dm_bht *bht, unsigned int block_index,
- u8 *digest, unsigned int digest_len)
+ const void *block)
{
int unverified = 0;
int entry_state = 0;
- /* TODO(wad) do we really need this? */
- if (digest_len != bht->digest_size) {
- DMERR("invalid digest_len passed to verify_block");
- return -EINVAL;
- }
-
/* Make sure that the root has been verified */
if (atomic_read(&bht->root_state) != DM_BHT_ENTRY_VERIFIED) {
unverified = dm_bht_verify_root(bht, dm_bht_compare_hash);
@@ -1090,7 +1074,7 @@
}
/* Now check that the digest supplied matches the leaf hash */
- unverified = dm_bht_check_block(bht, block_index, digest, &entry_state);
+ unverified = dm_bht_check_block(bht, block_index, block, &entry_state);
if (unverified) {
DMERR_LIMIT("Block check failed for %u: %d", block_index,
unverified);
diff --git a/dm-bht.h b/dm-bht.h
index 37c501d..25c20b5 100644
--- a/dm-bht.h
+++ b/dm-bht.h
@@ -135,7 +135,7 @@
int dm_bht_populate(struct dm_bht *bht, void *read_cb_ctx,
unsigned int block_index);
int dm_bht_verify_block(struct dm_bht *bht, unsigned int block_index,
- u8 *digest, unsigned int digest_len);
+ const void *buf);
/* Functions for creating struct dm_bhts on disk. A newly created dm_bht
* should not be directly used for verification. (It should be repopulated.)
diff --git a/dm-bht_unittest.cc b/dm-bht_unittest.cc
index bf8c74c..96323b4 100644
--- a/dm-bht_unittest.cc
+++ b/dm-bht_unittest.cc
@@ -107,9 +107,7 @@
const char *digest_algorithm) {
NewBht(depth, total_blocks, digest_algorithm);
- u8 data[PAGE_SIZE];
- // Store all the block hashes of blocks of 0.
- memset(reinterpret_cast<void *>(data), 0, sizeof(data));
+ u8 data[PAGE_SIZE] = { 0 };
unsigned int blocks = total_blocks;
do {
EXPECT_EQ(dm_bht_store_block(bht_.get(), blocks - 1, data), 0);
@@ -151,16 +149,8 @@
// Set the root hash for a 0-filled image
static const char kRootDigest[] =
"45d65d6f9e5a962f4d80b5f1bd7a918152251c27bdad8c5f52b590c129833372";
- // This should match what dm_bht_store_block computed earlier.
- static const char kZeroDigest[] =
- "ad7facb2586fc6e966c004d7d1d16b024f5805ff7cb47c7a85dabd8b48892ca7";
- u8 digest[(sizeof(kZeroDigest) - 1) >> 1];
- // TODO(wad) write a test for hex_to_bin and bin_to_hex
- unsigned int digest_size = strlen(kZeroDigest) >> 1;
-
- dm_bht_hex_to_bin(digest,
- reinterpret_cast<const u8 *>(kZeroDigest),
- digest_size);
+ // A page of all zeros
+ static const char kZeroPage[PAGE_SIZE] = { 0 };
SetupBht(2, total_blocks, "sha256");
dm_bht_set_root_hexdigest(bht_.get(),
@@ -168,7 +158,7 @@
for (unsigned int blocks = 0; blocks < total_blocks; ++blocks) {
DLOG(INFO) << "verifying block: " << blocks;
- EXPECT_EQ(0, dm_bht_verify_block(bht_.get(), blocks, digest, digest_size));
+ EXPECT_EQ(0, dm_bht_verify_block(bht_.get(), blocks, kZeroPage));
}
EXPECT_EQ(0, dm_bht_destroy(bht_.get()));
@@ -179,16 +169,8 @@
// Set the root hash for a 0-filled image
static const char kRootDigest[] =
"c86619624d3456f711dbb94d4ad79a4b029f6fd3b5a4a90b155c856bf5b3409b";
- // This should match what dm_bht_store_block computed earlier.
- static const char kZeroDigest[] =
- "ad7facb2586fc6e966c004d7d1d16b024f5805ff7cb47c7a85dabd8b48892ca7";
- u8 digest[(sizeof(kZeroDigest) - 1) >> 1];
- // TODO(wad) write a test for hex_to_bin and bin_to_hex
- unsigned int digest_size = strlen(kZeroDigest) >> 1;
-
- dm_bht_hex_to_bin(digest,
- reinterpret_cast<const u8 *>(kZeroDigest),
- digest_size);
+ // A page of all zeros
+ static const char kZeroPage[PAGE_SIZE] = { 0 };
SetupBht(4, total_blocks, "sha256");
dm_bht_set_root_hexdigest(bht_.get(),
@@ -196,7 +178,7 @@
for (unsigned int blocks = 0; blocks < total_blocks; ++blocks) {
DLOG(INFO) << "verifying block: " << blocks;
- EXPECT_EQ(0, dm_bht_verify_block(bht_.get(), blocks, digest, digest_size));
+ EXPECT_EQ(0, dm_bht_verify_block(bht_.get(), blocks, kZeroPage));
}
EXPECT_EQ(0, dm_bht_destroy(bht_.get()));
@@ -207,16 +189,8 @@
// Set the root hash for a 0-filled image
static const char kRootDigest[] =
"c78d187c430465bd7831fe4908247b6ab5107e3a826d933b71e85aa9a932e03c";
- // This should match what dm_bht_store_block computed earlier.
- static const char kZeroDigest[] =
- "ad7facb2586fc6e966c004d7d1d16b024f5805ff7cb47c7a85dabd8b48892ca7";
- u8 digest[(sizeof(kZeroDigest) - 1) >> 1];
- // TODO(wad) write a test for hex_to_bin and bin_to_hex
- unsigned int digest_size = strlen(kZeroDigest) >> 1;
-
- dm_bht_hex_to_bin(digest,
- reinterpret_cast<const u8 *>(kZeroDigest),
- digest_size);
+ // A page of all zeros
+ static const char kZeroPage[PAGE_SIZE] = { 0 };
SetupBht(4, total_blocks, "sha256");
dm_bht_set_root_hexdigest(bht_.get(),
@@ -224,15 +198,17 @@
for (unsigned int blocks = 0; blocks < total_blocks; ++blocks) {
DLOG(INFO) << "verifying block: " << blocks;
- EXPECT_EQ(0, dm_bht_verify_block(bht_.get(), blocks, digest, digest_size));
+ EXPECT_EQ(0, dm_bht_verify_block(bht_.get(), blocks, kZeroPage));
}
EXPECT_EQ(0, dm_bht_destroy(bht_.get()));
}
-TEST_F(MemoryBhtTest, CreateThenVerifyBad) {
+TEST_F(MemoryBhtTest, CreateThenVerifyBadHashBlock) {
static const unsigned int total_blocks = 16384;
SetupBht(2, total_blocks, "sha256");
+ // A page of all zeros
+ static const char kZeroPage[PAGE_SIZE] = { 0 };
// Set the root hash for a 0-filled image
static const char kRootDigest[] =
"45d65d6f9e5a962f4d80b5f1bd7a918152251c27bdad8c5f52b590c129833372";
@@ -241,43 +217,51 @@
// TODO(wad) add tests for partial tree validity/verification
- // Corrupt one block value
+ // Corrupt one has hblock
static const unsigned int kBadBlock = 256;
u8 data[PAGE_SIZE];
memset(reinterpret_cast<void *>(data), 'A', sizeof(data));
EXPECT_EQ(dm_bht_store_block(bht_.get(), kBadBlock, data), 0);
- // This should match what dm_bht_store_block computed earlier.
- static const char kZeroDigest[] =
- "ad7facb2586fc6e966c004d7d1d16b024f5805ff7cb47c7a85dabd8b48892ca7";
- u8 digest[(sizeof(kZeroDigest) - 1) >> 1];
- // TODO(wad) write a test for hex_to_bin and bin_to_hex
- unsigned int digest_size = strlen(kZeroDigest) >> 1;
- dm_bht_hex_to_bin(digest,
- reinterpret_cast<const u8 *>(kZeroDigest),
- digest_size);
-
// Attempt to verify both the bad block and all the neighbors.
- EXPECT_LT(dm_bht_verify_block(bht_.get(), kBadBlock + 1, digest, digest_size),
- 0);
+ EXPECT_LT(dm_bht_verify_block(bht_.get(), kBadBlock + 1, kZeroPage), 0);
- EXPECT_LT(dm_bht_verify_block(bht_.get(), kBadBlock + 2, digest, digest_size),
- 0);
+ EXPECT_LT(dm_bht_verify_block(bht_.get(), kBadBlock + 2, kZeroPage), 0);
EXPECT_LT(dm_bht_verify_block(bht_.get(), kBadBlock + (bht_->node_count / 2),
- digest, digest_size),
+ kZeroPage),
0);
- EXPECT_LT(dm_bht_verify_block(bht_.get(), kBadBlock, digest, digest_size), 0);
+ EXPECT_LT(dm_bht_verify_block(bht_.get(), kBadBlock, kZeroPage), 0);
// Verify that the prior entry is untouched and still safe
- EXPECT_EQ(dm_bht_verify_block(bht_.get(), kBadBlock - 1, digest, digest_size),
- 0);
+ EXPECT_EQ(dm_bht_verify_block(bht_.get(), kBadBlock - 1, kZeroPage), 0);
// Same for the next entry
EXPECT_EQ(dm_bht_verify_block(bht_.get(), kBadBlock + bht_->node_count,
- digest, digest_size),
+ kZeroPage),
0);
EXPECT_EQ(0, dm_bht_destroy(bht_.get()));
}
+
+TEST_F(MemoryBhtTest, CreateThenVerifyBadDataBlock) {
+ static const unsigned int total_blocks = 384;
+ SetupBht(2, total_blocks, "sha256");
+ // Set the root hash for a 0-filled image
+ static const char kRootDigest[] =
+ "45d65d6f9e5a962f4d80b5f1bd7a918152251c27bdad8c5f52b590c129833372";
+ dm_bht_set_root_hexdigest(bht_.get(),
+ reinterpret_cast<const u8 *>(kRootDigest));
+ // A corrupt page
+ static const u8 kBadPage[PAGE_SIZE] = { 'A' };
+
+ EXPECT_LT(dm_bht_verify_block(bht_.get(), 0, kBadPage), 0);
+ EXPECT_LT(dm_bht_verify_block(bht_.get(), 127, kBadPage), 0);
+ EXPECT_LT(dm_bht_verify_block(bht_.get(), 128, kBadPage), 0);
+ EXPECT_LT(dm_bht_verify_block(bht_.get(), 255, kBadPage), 0);
+ EXPECT_LT(dm_bht_verify_block(bht_.get(), 256, kBadPage), 0);
+ EXPECT_LT(dm_bht_verify_block(bht_.get(), 383, kBadPage), 0);
+
+ EXPECT_EQ(0, dm_bht_destroy(bht_.get()));
+}
