rgw/multisite: Fix use-after-move in retry logic in logbacking

Fitting a retry loop around a call that took the lock changed the
logic to potentially have the lock be used after moving.

Thanks to Suyash Dongre suyashd999@gmail.com for finding it.

This patch also fixes a lock being held across an I/O operation.

Fixes: https://tracker.ceph.com/issues/66340
Signed-off-by: Adam Emerson <aemerson@redhat.com>
(cherry picked from commit 80cf2ca3b44246ad8f2e333182b3c8b1835a71f7)

Fixed: https://tracker.ceph.com/issues/68053

Signed-off-by: Adam Emerson <aemerson@redhat.com>

diff --git a/src/rgw/driver/rados/rgw_log_backing.cc b/src/rgw/driver/rados/rgw_log_backing.cc
index 325d051090139c83aeff93c3460c36846a134676..110a54015a33ee0269d16e86975e18d1f1a0cfa7 100644 (file)
--- a/src/rgw/driver/rados/rgw_log_backing.cc
+++ b/src/rgw/driver/rados/rgw_log_backing.cc
@@ -444,13 +444,17 @@ bs::error_code logback_generations::write(const DoutPrefixProvider *dpp, entries
     encode(e, bl);
     op.write_full(bl);
     cls_version_inc(op);
+    auto oldv = version;
+    l.unlock();
     auto r = rgw_rados_operate(dpp, ioctx, oid, &op, y);
     if (r == 0) {
+      if (oldv != version) {
+       return { ECANCELED, bs::system_category() };
+      }
       entries_ = std::move(e);
       version.inc();
       return {};
     }
-    l.unlock();
     if (r < 0 && r != -ECANCELED) {
       ldpp_dout(dpp, -1) << __PRETTY_FUNCTION__ << ":" << __LINE__
                 << ": failed reading oid=" << oid
@@ -609,17 +613,19 @@ bs::error_code logback_generations::remove_empty(const DoutPrefixProvider *dpp,
     if (ec) return ec;
     auto tries = 0;
     entries_t new_entries;
-    std::unique_lock l(m);
-    ceph_assert(!entries_.empty());
+    entries_t es;
+    auto now = ceph::real_clock::now();
     {
-      auto i = lowest_nomempty(entries_);
-      if (i == entries_.begin()) {
-       return {};
+      std::unique_lock l(m);
+      ceph_assert(!entries_.empty());
+      {
+       auto i = lowest_nomempty(entries_);
+       if (i == entries_.begin()) {
+         return {};
+       }
       }
+      l.unlock();
     }
-    entries_t es;
-    auto now = ceph::real_clock::now();
-    l.unlock();
     do {
       std::copy_if(entries_.cbegin(), entries_.cend(),
                   std::inserter(es, es.end()),
@@ -646,7 +652,7 @@ bs::error_code logback_generations::remove_empty(const DoutPrefixProvider *dpp,
          es2.erase(i);
        }
       }
-      l.lock();
+      std::unique_lock l(m);
       es.clear();
       ec = write(dpp, std::move(es2), std::move(l), y);
       ++tries;