pw_kvs: Verify checksum on write
- Replace placeholder VerifyEntry function with the real verification
function, VerifyChecksumInFlash.
- Have VerifyChecksumInFlash read everything from flash, not just the
value.
- Fix missing bytes_to_read updates.
Change-Id: If856c60dee710bea128684fd1bb22d3e7fc62793
diff --git a/pw_kvs/format.cc b/pw_kvs/format.cc
index 739783d..819b541 100644
--- a/pw_kvs/format.cc
+++ b/pw_kvs/format.cc
@@ -49,31 +49,31 @@
return algorithm->Verify(checksum_bytes());
}
-Status EntryHeader::VerifyChecksumInFlash(
- FlashPartition* partition,
- FlashPartition::Address header_address,
- ChecksumAlgorithm* algorithm,
- string_view key) const {
+Status EntryHeader::VerifyChecksumInFlash(FlashPartition* partition,
+ FlashPartition::Address address,
+ ChecksumAlgorithm* algorithm) const {
if (algorithm == nullptr) {
return checksum() == kNoChecksum ? Status::OK : Status::DATA_LOSS;
}
- CalculateChecksum(algorithm, key);
+ algorithm->Reset();
- // Read the value piece-by-piece into a small buffer.
+ // Read the entire entry piece-by-piece into a small buffer.
// TODO: This read may be unaligned. The partition can handle this, but
// consider creating a API that skips the intermediate buffering.
byte buffer[32];
- size_t bytes_to_read = value_length();
- FlashPartition::Address address =
- header_address + sizeof(*this) + key_length();
+ address += checked_data_offset();
+ size_t bytes_to_read = entry_size() - checked_data_offset();
while (bytes_to_read > 0u) {
const size_t read_size = std::min(sizeof(buffer), bytes_to_read);
+
TRY(partition->Read(address, read_size, buffer));
- address += read_size;
algorithm->Update(buffer, read_size);
+
+ address += read_size;
+ bytes_to_read -= read_size;
}
return algorithm->Verify(checksum_bytes());
@@ -83,9 +83,8 @@
const string_view key,
span<const byte> value) const {
algorithm->Reset();
- algorithm->Update(reinterpret_cast<const byte*>(this) +
- offsetof(EntryHeader, key_value_length_),
- sizeof(*this) - offsetof(EntryHeader, key_value_length_));
+ algorithm->Update(reinterpret_cast<const byte*>(this) + checked_data_offset(),
+ sizeof(*this) - checked_data_offset());
algorithm->Update(as_bytes(span(key)));
algorithm->Update(value);
}
diff --git a/pw_kvs/key_value_store.cc b/pw_kvs/key_value_store.cc
index 39790ae..f106f9f 100644
--- a/pw_kvs/key_value_store.cc
+++ b/pw_kvs/key_value_store.cc
@@ -167,7 +167,7 @@
const string_view key(key_buffer.data(), header.key_length());
TRY(header.VerifyChecksumInFlash(
- &partition_, key_descriptor.address, entry_header_format_.checksum, key));
+ &partition_, key_descriptor.address, entry_header_format_.checksum));
key_descriptor.key_hash = HashKey(key);
DBG("Key hash: %zx (%zu)",
@@ -579,7 +579,8 @@
address, {as_bytes(span(&header, 1)), as_bytes(span(key)), value}));
if (options_.verify_on_write) {
- TRY(VerifyEntry(sector, key_descriptor));
+ TRY(header.VerifyChecksumInFlash(
+ &partition_, address, entry_header_format_.checksum));
}
// TODO: UPDATE last_written_sector_ appropriately
@@ -591,20 +592,6 @@
return Status::OK;
}
-Status KeyValueStore::VerifyEntry(SectorDescriptor* sector,
- KeyDescriptor* key_descriptor) {
- // TODO: Remove this once checksums are fully implemented.
- return Status::OK;
-
- if (entry_header_format_.checksum == nullptr) {
- return Status::OK;
- }
- // TODO: Implement me!
- (void)sector;
- (void)key_descriptor;
- return Status::UNIMPLEMENTED;
-}
-
void KeyValueStore::LogDebugInfo() {
const size_t sector_count = partition_.sector_count();
const size_t sector_size_bytes = partition_.sector_size_bytes();
diff --git a/pw_kvs/public/pw_kvs/key_value_store.h b/pw_kvs/public/pw_kvs/key_value_store.h
index 8632c22..0c974f2 100644
--- a/pw_kvs/public/pw_kvs/key_value_store.h
+++ b/pw_kvs/public/pw_kvs/key_value_store.h
@@ -284,8 +284,6 @@
std::string_view key,
span<const std::byte> value);
- Status VerifyEntry(SectorDescriptor* sector, KeyDescriptor* key_descriptor);
-
bool AddressInSector(const SectorDescriptor& sector, Address address) const {
const Address sector_base = SectorBaseAddress(§or);
const Address sector_end = sector_base + partition_.sector_size_bytes();
diff --git a/pw_kvs/pw_kvs_private/format.h b/pw_kvs/pw_kvs_private/format.h
index a1867e4..2fefd6e 100644
--- a/pw_kvs/pw_kvs_private/format.h
+++ b/pw_kvs/pw_kvs_private/format.h
@@ -43,8 +43,7 @@
Status VerifyChecksumInFlash(FlashPartition* partition,
FlashPartition::Address header_address,
- ChecksumAlgorithm* algorithm,
- std::string_view key) const;
+ ChecksumAlgorithm* algorithm) const;
size_t entry_size() const {
return sizeof(*this) + key_length() + value_length();
@@ -72,13 +71,17 @@
static constexpr uint32_t kKeyLengthMask = 0b111111;
static constexpr uint32_t kValueLengthShift = 8;
+ static constexpr size_t checked_data_offset() {
+ return offsetof(EntryHeader, key_value_length_);
+ }
+
span<const std::byte> checksum_bytes() const {
return as_bytes(span(&checksum_, 1));
}
void CalculateChecksum(ChecksumAlgorithm* algorithm,
std::string_view key,
- span<const std::byte> value = {}) const;
+ span<const std::byte> value) const;
uint32_t magic_;
uint32_t checksum_;
