osd: Fix PeeringState::proc_lease crash caused by race hazard

Fix race hazard in PeeringState::proc_lease where it read
a std::optional value which could be modified by another
thread at the same time causing an abort.

There is already a lock to protect accesses to this state
to make them thread safe, but the code wasn't using it.

Fixes: https://tracker.ceph.com/issues/54932
Assisted-by: IBM Bob 2.0.1
Signed-off-by: Bill Scales <bill_scales@uk.ibm.com>

diff --git a/src/osd/PeeringState.cc b/src/osd/PeeringState.cc
index c3ed6be810760fa7aa5705f258f4da8b0e13f960..41956b4150929d168046b2fb558cf4ba5ec45117 100644 (file)
--- a/src/osd/PeeringState.cc
+++ b/src/osd/PeeringState.cc
@@ -1328,11 +1328,14 @@ void PeeringState::proc_lease(const pg_lease_t& l)
     readable_until_ub_from_primary = l.readable_until_ub;
   }
 
+  std::optional<ceph::signedspan> peer_clock_delta_lb, peer_clock_delta_ub;
+  hb_stamps[0]->get_peer_clock_delta(&peer_clock_delta_lb, &peer_clock_delta_ub);
+
   ceph::signedspan ru = ceph::signedspan::zero();
   if (l.readable_until != ceph::signedspan::zero() &&
-      hb_stamps[0]->peer_clock_delta_ub) {
-    ru = l.readable_until - *hb_stamps[0]->peer_clock_delta_ub;
-    psdout(20) << " peer_clock_delta_ub " << *hb_stamps[0]->peer_clock_delta_ub
+      peer_clock_delta_ub) {
+    ru = l.readable_until - *peer_clock_delta_ub;
+    psdout(20) << " peer_clock_delta_ub " << *peer_clock_delta_ub
               << " -> ru " << ru << dendl;
   }
   if (ru > readable_until) {
@@ -1343,9 +1346,9 @@ void PeeringState::proc_lease(const pg_lease_t& l)
   }
 
   ceph::signedspan ruub;
-  if (hb_stamps[0]->peer_clock_delta_lb) {
-    ruub = l.readable_until_ub - *hb_stamps[0]->peer_clock_delta_lb;
-    psdout(20) << " peer_clock_delta_lb " << *hb_stamps[0]->peer_clock_delta_lb
+  if (peer_clock_delta_lb) {
+    ruub = l.readable_until_ub - *peer_clock_delta_lb;
+    psdout(20) << " peer_clock_delta_lb " << *peer_clock_delta_lb
               << " -> ruub " << ruub << dendl;
   } else {
     ruub = pl->get_mnow() + l.interval;

diff --git a/src/osd/PeeringState.h b/src/osd/PeeringState.h
index 94ee299cdfb8580b90b98d31e9564c274c12b69c..39296487a68d75c34cc20b9f277b549e085d2010 100644 (file)
--- a/src/osd/PeeringState.h
+++ b/src/osd/PeeringState.h
@@ -197,6 +197,14 @@ struct HeartbeatStamps : public RefCountedObject {
     peer_clock_delta_ub = delta_ub;
   }
 
+  void get_peer_clock_delta(
+      std::optional<ceph::signedspan> *out_lb,
+      std::optional<ceph::signedspan> *out_ub) const {
+    std::lock_guard l(lock);
+    *out_lb = peer_clock_delta_lb;
+    *out_ub = peer_clock_delta_ub;
+  }
+
 private:
   FRIEND_MAKE_REF(HeartbeatStamps);
   HeartbeatStamps(int o)