From 252897cbb7ca98c2fa2ea869a14365c706f4b86b Mon Sep 17 00:00:00 2001 From: Vikas Soni Date: Fri, 14 Jul 2023 01:19:04 +0000 Subject: [PATCH] Fix race condition in PassthroughProgramCache. PassthroughProgramCache has code to deal with multiple threads, but BlobCacheGet() does not hold the lock for the entire time it is called. This means it's possible for BlobCacheGet() to attempt to use a cache value that was deleted by another thread. BlobCacheGet() needs to hold the lock for the entirety of the function. Add a new PassthroughProgramCache::GetSize() method to handle the locking since BlobCacheGet() is a static method which can not hold lock. Bug: 1464739 Change-Id: Ibdbc78823fbf0032b8f0d84f0aafa249f93bf3d6 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4683916 Commit-Queue: Kenneth Russell Commit-Queue: Scott Violet Reviewed-by: Kenneth Russell Commit-Queue: vikas soni Reviewed-by: Scott Violet Auto-Submit: vikas soni Cr-Commit-Position: refs/heads/main@{#1170276} --- .../service/passthrough_program_cache.cc | 56 ++++++++++++------- .../service/passthrough_program_cache.h | 4 ++ 2 files changed, 39 insertions(+), 21 deletions(-) diff --git a/gpu/command_buffer/service/passthrough_program_cache.cc b/gpu/command_buffer/service/passthrough_program_cache.cc index e175f4bc8daded..50299adf2760a8 100644 --- a/gpu/command_buffer/service/passthrough_program_cache.cc +++ b/gpu/command_buffer/service/passthrough_program_cache.cc @@ -178,11 +178,44 @@ void PassthroughProgramCache::Set(Key&& key, Value&& value) { const PassthroughProgramCache::ProgramCacheValue* PassthroughProgramCache::Get( const Key& key) { - base::AutoLock auto_lock(lock_); + lock_.AssertAcquired(); ProgramLRUCache::iterator found = store_.Get(key); return found == store_.end() ? nullptr : &found->second; } +EGLsizeiANDROID PassthroughProgramCache::BlobCacheGetImpl( + const void* key, + EGLsizeiANDROID key_size, + void* value, + EGLsizeiANDROID value_size) { + if (key_size < 0) { + return 0; + } + + const uint8_t* key_begin = reinterpret_cast(key); + PassthroughProgramCache::Key entry_key(key_begin, key_begin + key_size); + + // Note that the |lock_| should be held during whole time ProgramCacheValue is + // being accessed below. + base::AutoLock auto_lock(lock_); + const PassthroughProgramCache::ProgramCacheValue* cache_value = + g_program_cache->Get(std::move(entry_key)); + + if (!cache_value) { + return 0; + } + + const PassthroughProgramCache::Value& entry_value = cache_value->data(); + + if (value_size > 0) { + if (static_cast(value_size) >= entry_value.size()) { + memcpy(value, entry_value.data(), entry_value.size()); + } + } + + return entry_value.size(); +} + void PassthroughProgramCache::BlobCacheSet(const void* key, EGLsizeiANDROID key_size, const void* value, @@ -211,26 +244,7 @@ EGLsizeiANDROID PassthroughProgramCache::BlobCacheGet( if (!g_program_cache) return 0; - if (key_size < 0) - return 0; - - const uint8_t* key_begin = reinterpret_cast(key); - PassthroughProgramCache::Key entry_key(key_begin, key_begin + key_size); - - const PassthroughProgramCache::ProgramCacheValue* cacheValue = - g_program_cache->Get(std::move(entry_key)); - - if (!cacheValue) - return 0; - - const PassthroughProgramCache::Value& entry_value = cacheValue->data(); - - if (value_size > 0) { - if (static_cast(value_size) >= entry_value.size()) - memcpy(value, entry_value.data(), entry_value.size()); - } - - return entry_value.size(); + return g_program_cache->BlobCacheGetImpl(key, key_size, value, value_size); } PassthroughProgramCache::ProgramCacheValue::ProgramCacheValue( diff --git a/gpu/command_buffer/service/passthrough_program_cache.h b/gpu/command_buffer/service/passthrough_program_cache.h index d47a1960f52f73..33a340fe2ba93d 100644 --- a/gpu/command_buffer/service/passthrough_program_cache.h +++ b/gpu/command_buffer/service/passthrough_program_cache.h @@ -105,6 +105,10 @@ class GPU_GLES2_EXPORT PassthroughProgramCache : public ProgramCache { bool CacheEnabled() const; const ProgramCacheValue* Get(const Key& key); + EGLsizeiANDROID BlobCacheGetImpl(const void* key, + EGLsizeiANDROID key_size, + void* value, + EGLsizeiANDROID value_size); friend class ProgramCacheValue;