From 1ab02c36bee95e85558f6815b2836e9ee6a26b19 Mon Sep 17 00:00:00 2001 From: Kristian Monsen Date: Tue, 1 Nov 2011 14:56:51 +0000 Subject: Part of fix for bug 5523834, backporting cache fixes This is hopefully a fix for bug 5255299 Cherry-picking CL http://src.chromium.org/viewvc/chrome?view=rev&revision=93059 Change-Id: I2295cb29dc4ac1f97ceb492ea77a205a2b5c7a05 --- net/disk_cache/backend_impl.cc | 9 +-------- net/disk_cache/entry_impl.cc | 39 +++++++++++++++++++++++++++++++++++---- net/disk_cache/entry_impl.h | 3 +++ net/disk_cache/entry_unittest.cc | 26 ++++++++++++++++++++++++++ 4 files changed, 65 insertions(+), 12 deletions(-) diff --git a/net/disk_cache/backend_impl.cc b/net/disk_cache/backend_impl.cc index c5b7724..e5d889f 100644 --- a/net/disk_cache/backend_impl.cc +++ b/net/disk_cache/backend_impl.cc @@ -723,14 +723,7 @@ EntryImpl* BackendImpl::CreateEntryImpl(const std::string& key) { } } - int num_blocks; - size_t key1_len = sizeof(EntryStore) - offsetof(EntryStore, key); - if (key.size() < key1_len || - key.size() > static_cast(kMaxInternalKeyLength)) - num_blocks = 1; - else - num_blocks = static_cast((key.size() - key1_len) / 256 + 2); - + int num_blocks = EntryImpl::NumBlocksForEntry(key.size()); if (!block_files_.CreateBlock(BLOCK_256, num_blocks, &entry_address)) { LOG(ERROR) << "Create entry failed " << key.c_str(); stats_.OnEvent(Stats::CREATE_ERROR); diff --git a/net/disk_cache/entry_impl.cc b/net/disk_cache/entry_impl.cc index 73c67df..0dbfc94 100644 --- a/net/disk_cache/entry_impl.cc +++ b/net/disk_cache/entry_impl.cc @@ -584,12 +584,26 @@ bool EntryImpl::SanityCheck() { return false; Addr key_addr(stored->long_key); - if (stored->key_len <= kMaxInternalKeyLength && key_addr.is_initialized()) + if ((stored->key_len <= kMaxInternalKeyLength && key_addr.is_initialized()) || + (stored->key_len > kMaxInternalKeyLength && !key_addr.is_initialized())) return false; if (!key_addr.SanityCheck()) return false; + if (key_addr.is_initialized() && + ((stored->key_len <= kMaxBlockSize && key_addr.is_separate_file()) || + (stored->key_len > kMaxBlockSize && key_addr.is_block_file()))) + return false; + + int num_blocks = NumBlocksForEntry(stored->key_len); + if (entry_.address().num_blocks() != num_blocks) + return false; + + // The key must be NULL terminated. + if (!key_addr.is_initialized() && stored->key[stored->key_len]) + return false; + if (stored->hash != Hash(GetKey())) return false; @@ -663,6 +677,20 @@ const net::BoundNetLog& EntryImpl::net_log() const { return net_log_; } +// static +int EntryImpl::NumBlocksForEntry(int key_size) { + // The longest key that can be stored using one block. + int key1_len = + static_cast(sizeof(EntryStore) - offsetof(EntryStore, key)); + + if (key_size < key1_len || key_size > kMaxInternalKeyLength) + return 1; + + return ((key_size - key1_len) / 256 + 2); +} + +// ------------------------------------------------------------------------ + void EntryImpl::Doom() { backend_->background_queue()->DoomEntryImpl(this); } @@ -673,7 +701,8 @@ void EntryImpl::Close() { std::string EntryImpl::GetKey() const { CacheEntryBlock* entry = const_cast(&entry_); - if (entry->Data()->key_len <= kMaxInternalKeyLength) + int key_len = entry->Data()->key_len; + if (key_len <= kMaxInternalKeyLength) return std::string(entry->Data()->key); // We keep a copy of the key so that we can always return it, even if the @@ -691,9 +720,11 @@ std::string EntryImpl::GetKey() const { File* key_file = const_cast(this)->GetBackingFile(address, kKeyFileIndex); + if (!offset && key_file->GetLength() != static_cast(key_len + 1)) + return std::string(); + if (!key_file || - !key_file->Read(WriteInto(&key_, entry->Data()->key_len + 1), - entry->Data()->key_len + 1, offset)) + !key_file->Read(WriteInto(&key_, key_len + 1), key_len + 1, offset)) key_.clear(); return key_; } diff --git a/net/disk_cache/entry_impl.h b/net/disk_cache/entry_impl.h index fab676a..16f9e07 100644 --- a/net/disk_cache/entry_impl.h +++ b/net/disk_cache/entry_impl.h @@ -124,6 +124,9 @@ class EntryImpl : public Entry, public base::RefCounted { const net::BoundNetLog& net_log() const; + // Returns the number of blocks needed to store an EntryStore. + static int NumBlocksForEntry(int key_size); + // Entry interface. virtual void Doom(); virtual void Close(); diff --git a/net/disk_cache/entry_unittest.cc b/net/disk_cache/entry_unittest.cc index 9c6b28a..ce3696c 100644 --- a/net/disk_cache/entry_unittest.cc +++ b/net/disk_cache/entry_unittest.cc @@ -1944,3 +1944,29 @@ TEST_F(DiskCacheEntryTest, CancelSparseIO) { EXPECT_EQ(0, cb5.GetResult(rv)); entry->Close(); } + +// Tests that we perform sanity checks on an entry's key. Note that there are +// other tests that exercise sanity checks by using saved corrupt files. +TEST_F(DiskCacheEntryTest, KeySanityCheck) { + UseCurrentThread(); + InitCache(); + std::string key("the first key"); + disk_cache::Entry* entry; + ASSERT_EQ(net::OK, CreateEntry(key, &entry)); + + disk_cache::EntryImpl* entry_impl = + static_cast(entry); + disk_cache::EntryStore* store = entry_impl->entry()->Data(); + + // We have reserved space for a short key (one block), let's say that the key + // takes more than one block, and remove the NULLs after the actual key. + store->key_len = 800; + memset(store->key + key.size(), 'k', sizeof(store->key) - key.size()); + entry_impl->entry()->set_modified(); + entry->Close(); + + // We have a corrupt entry. Now reload it. We should NOT read beyond the + // allocated buffer here. + ASSERT_NE(net::OK, OpenEntry(key, &entry)); + DisableIntegrityCheck(); +} -- cgit v1.1