From: Casey Bodley <cbodley@redhat.com>
Date: Mon, 18 Oct 2021 15:23:49 +0000 (-0400)
Subject: rgw: fix lock scope in ObjectCache::get()
X-Git-Tag: v17.1.0~585^2
X-Git-Url: http://git.apps.os.sepia.ceph.com/?a=commitdiff_plain;h=1aee987ec8f817541aab9cd21480b5351df38f69;p=ceph.git

rgw: fix lock scope in ObjectCache::get()

in the touch_lru() case, we promote the shared_lock to a unique_lock.
but because the unique_lock is in a nested scope, the lock drops with
its scope and we continue accessing the map without any protection

this moves the unique_lock up to function scope, where it's
constructed as unlocked with std::defer_lock. after promotion, this
lock will be held until the function returns

Fixes: https://tracker.ceph.com/issues/52800

Signed-off-by: Casey Bodley <cbodley@redhat.com>
---

diff --git a/src/rgw/rgw_cache.cc b/src/rgw/rgw_cache.cc
index 3a325c81c2c2a..c38f19df020db 100644
--- a/src/rgw/rgw_cache.cc
+++ b/src/rgw/rgw_cache.cc
@@ -14,6 +14,7 @@ int ObjectCache::get(const DoutPrefixProvider *dpp, const string& name, ObjectCa
 {
 
   std::shared_lock rl{lock};
+  std::unique_lock wl{lock, std::defer_lock}; // may be promoted to write lock
   if (!enabled) {
     return -ENOENT;
   }
@@ -30,7 +31,7 @@ int ObjectCache::get(const DoutPrefixProvider *dpp, const string& name, ObjectCa
        (ceph::coarse_mono_clock::now() - iter->second.info.time_added) > expiry) {
     ldpp_dout(dpp, 10) << "cache get: name=" << name << " : expiry miss" << dendl;
     rl.unlock();
-    std::unique_lock wl{lock};  // write lock for insertion
+    wl.lock(); // write lock for expiration
     // check that wasn't already removed by other thread
     iter = cache_map.find(name);
     if (iter != cache_map.end()) {
@@ -51,7 +52,7 @@ int ObjectCache::get(const DoutPrefixProvider *dpp, const string& name, ObjectCa
     ldpp_dout(dpp, 20) << "cache get: touching lru, lru_counter=" << lru_counter
                    << " promotion_ts=" << entry->lru_promotion_ts << dendl;
     rl.unlock();
-    std::unique_lock wl{lock};  // write lock for insertion
+    wl.lock(); // write lock for touch_lru()
     /* need to redo this because entry might have dropped off the cache */
     iter = cache_map.find(name);
     if (iter == cache_map.end()) {