From 1c75e88055e06da2939f9f4bd294625b76792815 Mon Sep 17 00:00:00 2001 From: cmumford Date: Mon, 2 Oct 2017 13:57:41 -0700 Subject: [PATCH] Fix use of uninitialized value in LRUHandle. If leveldb::Options::block_cache is set to a cache of zero capacity then it is possible for LRUHandle::next to be used without having been set. Conditional jump or move depends on uninitialised value(s): leveldb::(anonymous namespace)::LRUHandle::key() const (cache.cc:58) leveldb::(anonymous namespace)::LRUCache::Unref(leveldb::(anonymous namespace)::LRUHandle*) (cache.cc:234) leveldb::(anonymous namespace)::LRUCache::Release(leveldb::Cache::Handle*) (cache.cc:266) leveldb::(anonymous namespace)::ShardedLRUCache::Release(leveldb::Cache::Handle*) (cache.cc:375) leveldb::CacheTest::Insert(int, int, int) (cache_test.cc:59) This bug forced a commit reversion in Chromium. For more information see https://bugs.chromium.org/p/chromium/issues/detail?id=761398#c4 ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=170749054 --- util/cache.cc | 5 ++++- util/cache_test.cc | 8 ++++++++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/util/cache.cc b/util/cache.cc index ce46886..97b82ea 100644 --- a/util/cache.cc +++ b/util/cache.cc @@ -288,7 +288,10 @@ Cache::Handle* LRUCache::Insert( LRU_Append(&in_use_, e); usage_ += charge; FinishErase(table_.Insert(e)); - } // else don't cache. (Tests use capacity_==0 to turn off caching.) + } else { + // don't cache. (It is valid to set capacity_==0 to turn off caching.) + e->next = NULL; + } while (usage_ > capacity_ && lru_.next != &lru_) { LRUHandle* old = lru_.next; diff --git a/util/cache_test.cc b/util/cache_test.cc index 468f7a6..246ab8e 100644 --- a/util/cache_test.cc +++ b/util/cache_test.cc @@ -219,6 +219,14 @@ TEST(CacheTest, Prune) { ASSERT_EQ(-1, Lookup(2)); } +TEST(CacheTest, ZeroSizeCache) { + delete cache_; + cache_ = NewLRUCache(0); + + Insert(1, 100); + ASSERT_EQ(-1, Lookup(1)); +} + } // namespace leveldb int main(int argc, char** argv) {