From 83b1cd4c44b96819588828137bcf5edf9452a9bb Mon Sep 17 00:00:00 2001 From: Dan Engelbrecht Date: Mon, 13 Jun 2022 23:15:09 +0200 Subject: Add validation to ZenCacheDiskLayer::CacheBucket::Scrub --- zenserver/cache/structuredcachestore.cpp | 144 +++++++++++++++++++++++++------ 1 file changed, 120 insertions(+), 24 deletions(-) (limited to 'zenserver') diff --git a/zenserver/cache/structuredcachestore.cpp b/zenserver/cache/structuredcachestore.cpp index 0ccd5d52a..ed602f3f7 100644 --- a/zenserver/cache/structuredcachestore.cpp +++ b/zenserver/cache/structuredcachestore.cpp @@ -1308,56 +1308,152 @@ ZenCacheDiskLayer::CacheBucket::SaveManifest() void ZenCacheDiskLayer::CacheBucket::Scrub(ScrubContext& Ctx) { - std::vector BadKeys; + std::vector BadKeys; + std::vector ChunkLocations; + std::vector ChunkIndexToChunkHash; + + auto ValidateEntry = [](ZenContentType ContentType, IoBuffer Buffer) { + if (ContentType == ZenContentType::kCbObject) + { + if (CbValidateError Error = ValidateCompactBinary(Buffer, CbValidateMode::All); Error != CbValidateError::None) + { + return false; + } + } + if (ContentType == ZenContentType::kCompressedBinary) + { + if (CompressedBuffer Compressed = CompressedBuffer::FromCompressed(SharedBuffer(Buffer)); !Compressed) + { + return false; + } + } + return true; + }; + RwLock::SharedLockScope _(m_IndexLock); + + size_t BlockChunkInitialCount = m_Index.size() / 4; + ChunkLocations.reserve(BlockChunkInitialCount); + ChunkIndexToChunkHash.reserve(BlockChunkInitialCount); + + for (auto& Kv : m_Index) { - RwLock::SharedLockScope _(m_IndexLock); + const IoHash& HashKey = Kv.first; + const DiskLocation& Loc = Kv.second.Location; - for (auto& Kv : m_Index) + if (Loc.IsFlagSet(DiskLocation::kStandaloneFile)) { - const IoHash& HashKey = Kv.first; - const DiskLocation& Loc = Kv.second.Location; + if (Loc.GetContentType() == ZenContentType::kBinary) + { + ExtendablePathBuilder<256> DataFilePath; + BuildPath(DataFilePath, HashKey); - ZenCacheValue Value; + RwLock::SharedLockScope ValueLock(LockForHash(HashKey)); - if (Loc.IsFlagSet(DiskLocation::kStandaloneFile)) - { - if (GetStandaloneCacheValue(Loc, HashKey, Value)) + std::error_code Ec; + uintmax_t size = std::filesystem::file_size(DataFilePath.ToPath(), Ec); + if (Ec) { - // Note: we cannot currently validate contents since we don't - // have a content hash! - continue; + BadKeys.push_back(HashKey); } + if (size != Loc.Size()) + { + BadKeys.push_back(HashKey); + } + // Note: we cannot currently validate contents since we don't + // have a content hash! + continue; + } + ZenCacheValue Value; + if (!GetStandaloneCacheValue(Loc, HashKey, Value)) + { + BadKeys.push_back(HashKey); + continue; } - else if (GetInlineCacheValue(Loc, Value)) + if (!ValidateEntry(Loc.GetContentType(), Value.Value)) { - // Validate contents + BadKeys.push_back(HashKey); continue; } - // Value not found - BadKeys.push_back(HashKey); + } + else + { + BlockStoreLocation Location = Loc.GetBlockLocation(m_PayloadAlignment); + ChunkLocations.push_back(Location); + ChunkIndexToChunkHash.push_back(HashKey); + continue; } } + m_BlockStore.IterateChunks( + ChunkLocations, + [&](size_t ChunkIndex, const void* Data, uint64_t Size) { + const IoHash& Hash = ChunkIndexToChunkHash[ChunkIndex]; + if (!Data) + { + // ChunkLocation out of range of stored blocks + BadKeys.push_back(Hash); + return; + } + IoBuffer Buffer(IoBuffer::Wrap, Data, Size); + if (!Buffer) + { + BadKeys.push_back(Hash); + return; + } + ZenContentType ContentType = m_Index.at(Hash).Location.GetContentType(); + if (!ValidateEntry(ContentType, Buffer)) + { + BadKeys.push_back(Hash); + return; + } + }, + [&](size_t ChunkIndex, BlockStoreFile& File, uint64_t Offset, uint64_t Size) { + const IoHash& Hash = ChunkIndexToChunkHash[ChunkIndex]; + IoBuffer Buffer(IoBuffer::BorrowedFile, File.GetBasicFile().Handle(), Offset, Size); + if (!Buffer) + { + BadKeys.push_back(Hash); + return; + } + ZenContentType ContentType = m_Index.at(Hash).Location.GetContentType(); + if (!ValidateEntry(ContentType, Buffer)) + { + BadKeys.push_back(Hash); + return; + } + }); + + _.ReleaseNow(); + if (BadKeys.empty()) { return; } + ZEN_ERROR("Scrubbing found #{} bad chunks in '{}'", BadKeys.size(), m_BucketDir / m_BucketName); + if (Ctx.RunRecovery()) { - RwLock::ExclusiveLockScope _(m_IndexLock); + // Deal with bad chunks by removing them from our lookup map + + std::vector LogEntries; + LogEntries.reserve(BadKeys.size()); - for (const IoHash& BadKey : BadKeys) { - // Log a tombstone and delete the in-memory index for the bad entry + RwLock::ExclusiveLockScope __(m_IndexLock); + for (const IoHash& BadKey : BadKeys) + { + // Log a tombstone and delete the in-memory index for the bad entry - const auto It = m_Index.find(BadKey); - DiskLocation Location = It->second.Location; - Location.Flags |= DiskLocation::kTombStone; - m_SlogFile.Append(DiskIndexEntry{.Key = BadKey, .Location = Location}); - m_Index.erase(BadKey); + const auto It = m_Index.find(BadKey); + DiskLocation Location = It->second.Location; + Location.Flags |= DiskLocation::kTombStone; + LogEntries.push_back(DiskIndexEntry{.Key = BadKey, .Location = Location}); + m_Index.erase(BadKey); + } } + m_SlogFile.Append(LogEntries); } // Let whomever it concerns know about the bad chunks. This could -- cgit v1.2.3 From 4164fbb55c7f4283b5fa165c6ce8321db9ade0c2 Mon Sep 17 00:00:00 2001 From: Dan Engelbrecht Date: Tue, 14 Jun 2022 09:57:12 +0200 Subject: clang format --- zenserver/projectstore.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'zenserver') diff --git a/zenserver/projectstore.h b/zenserver/projectstore.h index 2328f7fee..6a8730ee2 100644 --- a/zenserver/projectstore.h +++ b/zenserver/projectstore.h @@ -50,7 +50,7 @@ static_assert(IsPow2(sizeof(OplogEntry))); /** Project Store - A project store consists of a number of Projects. + A project store consists of a number of Projects. Each project contains a number of oplogs (short for "operation log"). UE uses one oplog per target platform to store the output of the cook process. -- cgit v1.2.3 From cf694b4f5c70592fc6054c58bee08ef49e2e8420 Mon Sep 17 00:00:00 2001 From: Dan Engelbrecht Date: Tue, 14 Jun 2022 10:21:28 +0200 Subject: small cleanup --- zenserver/cache/structuredcachestore.cpp | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) (limited to 'zenserver') diff --git a/zenserver/cache/structuredcachestore.cpp b/zenserver/cache/structuredcachestore.cpp index ed602f3f7..7bcd7b06a 100644 --- a/zenserver/cache/structuredcachestore.cpp +++ b/zenserver/cache/structuredcachestore.cpp @@ -1360,8 +1360,6 @@ ZenCacheDiskLayer::CacheBucket::Scrub(ScrubContext& Ctx) { BadKeys.push_back(HashKey); } - // Note: we cannot currently validate contents since we don't - // have a content hash! continue; } ZenCacheValue Value; @@ -1378,8 +1376,7 @@ ZenCacheDiskLayer::CacheBucket::Scrub(ScrubContext& Ctx) } else { - BlockStoreLocation Location = Loc.GetBlockLocation(m_PayloadAlignment); - ChunkLocations.push_back(Location); + ChunkLocations.emplace_back(Loc.GetBlockLocation(m_PayloadAlignment)); ChunkIndexToChunkHash.push_back(HashKey); continue; } -- cgit v1.2.3 From 304bb51b305e10cf2dc1fdda0bba0dec2bd19b2b Mon Sep 17 00:00:00 2001 From: Dan Engelbrecht Date: Tue, 14 Jun 2022 15:34:06 +0200 Subject: review feedback --- zenserver/cache/structuredcachestore.cpp | 85 ++++++++++++++++---------------- 1 file changed, 42 insertions(+), 43 deletions(-) (limited to 'zenserver') diff --git a/zenserver/cache/structuredcachestore.cpp b/zenserver/cache/structuredcachestore.cpp index 7bcd7b06a..4be33170c 100644 --- a/zenserver/cache/structuredcachestore.cpp +++ b/zenserver/cache/structuredcachestore.cpp @@ -1315,10 +1315,8 @@ ZenCacheDiskLayer::CacheBucket::Scrub(ScrubContext& Ctx) auto ValidateEntry = [](ZenContentType ContentType, IoBuffer Buffer) { if (ContentType == ZenContentType::kCbObject) { - if (CbValidateError Error = ValidateCompactBinary(Buffer, CbValidateMode::All); Error != CbValidateError::None) - { - return false; - } + CbValidateError Error = ValidateCompactBinary(Buffer, CbValidateMode::All); + return Error == CbValidateError::None; } if (ContentType == ZenContentType::kCompressedBinary) { @@ -1332,7 +1330,7 @@ ZenCacheDiskLayer::CacheBucket::Scrub(ScrubContext& Ctx) RwLock::SharedLockScope _(m_IndexLock); - size_t BlockChunkInitialCount = m_Index.size() / 4; + const size_t BlockChunkInitialCount = m_Index.size() / 4; ChunkLocations.reserve(BlockChunkInitialCount); ChunkIndexToChunkHash.reserve(BlockChunkInitialCount); @@ -1382,44 +1380,45 @@ ZenCacheDiskLayer::CacheBucket::Scrub(ScrubContext& Ctx) } } - m_BlockStore.IterateChunks( - ChunkLocations, - [&](size_t ChunkIndex, const void* Data, uint64_t Size) { - const IoHash& Hash = ChunkIndexToChunkHash[ChunkIndex]; - if (!Data) - { - // ChunkLocation out of range of stored blocks - BadKeys.push_back(Hash); - return; - } - IoBuffer Buffer(IoBuffer::Wrap, Data, Size); - if (!Buffer) - { - BadKeys.push_back(Hash); - return; - } - ZenContentType ContentType = m_Index.at(Hash).Location.GetContentType(); - if (!ValidateEntry(ContentType, Buffer)) - { - BadKeys.push_back(Hash); - return; - } - }, - [&](size_t ChunkIndex, BlockStoreFile& File, uint64_t Offset, uint64_t Size) { - const IoHash& Hash = ChunkIndexToChunkHash[ChunkIndex]; - IoBuffer Buffer(IoBuffer::BorrowedFile, File.GetBasicFile().Handle(), Offset, Size); - if (!Buffer) - { - BadKeys.push_back(Hash); - return; - } - ZenContentType ContentType = m_Index.at(Hash).Location.GetContentType(); - if (!ValidateEntry(ContentType, Buffer)) - { - BadKeys.push_back(Hash); - return; - } - }); + const auto ValidateSmallChunk = [&](size_t ChunkIndex, const void* Data, uint64_t Size) { + const IoHash& Hash = ChunkIndexToChunkHash[ChunkIndex]; + if (!Data) + { + // ChunkLocation out of range of stored blocks + BadKeys.push_back(Hash); + return; + } + IoBuffer Buffer(IoBuffer::Wrap, Data, Size); + if (!Buffer) + { + BadKeys.push_back(Hash); + return; + } + ZenContentType ContentType = m_Index.at(Hash).Location.GetContentType(); + if (!ValidateEntry(ContentType, Buffer)) + { + BadKeys.push_back(Hash); + return; + } + }; + + const auto ValidateLargeChunk = [&](size_t ChunkIndex, BlockStoreFile& File, uint64_t Offset, uint64_t Size) { + const IoHash& Hash = ChunkIndexToChunkHash[ChunkIndex]; + IoBuffer Buffer(IoBuffer::BorrowedFile, File.GetBasicFile().Handle(), Offset, Size); + if (!Buffer) + { + BadKeys.push_back(Hash); + return; + } + ZenContentType ContentType = m_Index.at(Hash).Location.GetContentType(); + if (!ValidateEntry(ContentType, Buffer)) + { + BadKeys.push_back(Hash); + return; + } + }; + + m_BlockStore.IterateChunks(ChunkLocations, ValidateSmallChunk, ValidateLargeChunk); _.ReleaseNow(); -- cgit v1.2.3 From 0ba3a7d1c67de9b6d94f835f6e90a1f2e115f94c Mon Sep 17 00:00:00 2001 From: Dan Engelbrecht Date: Tue, 14 Jun 2022 23:37:05 +0200 Subject: Make sure we don't try to create a ZipFS IoBuffer of zero size --- zenserver/frontend/frontend.cpp | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) (limited to 'zenserver') diff --git a/zenserver/frontend/frontend.cpp b/zenserver/frontend/frontend.cpp index 842587708..e203e0631 100644 --- a/zenserver/frontend/frontend.cpp +++ b/zenserver/frontend/frontend.cpp @@ -26,7 +26,7 @@ FindZipFsInBinary(const IoBuffer& BinBuffer) uintptr_t Cursor = uintptr_t(BinBuffer.GetData()); size_t BinSize = 0; - uint32_t Magic = *(uint32_t*)(BinBuffer.GetData()); + uint32_t Magic = *(uint32_t*)(Cursor); #if ZEN_PLATFORM_LINUX if (Magic == 0x464c457f) { @@ -134,6 +134,12 @@ FindZipFsInBinary(const IoBuffer& BinBuffer) return {}; } + size_t ZipFsSize = BinBuffer.Size() - BinSize; + if (!ZipFsSize) + { + return {}; + } + return IoBuffer(BinBuffer, BinSize); } -- cgit v1.2.3