mon: handle bad snapshot removal reqs gracefully

Snapshot deletion requests on snap ids larger than the snap_seq of
the pool will leave the pool in a state with snap_seq being less
than max(removed_snaps).

This is bad because further deletion requests to a pool in this state
might crash the mon in some cases: the deletion also inserts the new
snap_seq into the removed_snaps set -- which might already exist
in this case and trigger an assert.

Such bad requests will be generated by rbd clients without a fix for
issue #21567.

The change in OSDMonitor prevents pools from getting into this state
and may prevent old broken clients from incorrectly deleting snaps.
The change in osd_types avoids a crash if a pool is already in this
state.

Fixes #18746

Signed-off-by: Paul Emmerich <paul.emmerich@croit.io>

diff --git a/src/mon/OSDMonitor.cc b/src/mon/OSDMonitor.cc
index 7a49a793a4f9e2b571fa26af492039a0025247cb..2991cebb968163e6f9e9ce2b5a10a8721e7b9b84 100644 (file)
--- a/src/mon/OSDMonitor.cc
+++ b/src/mon/OSDMonitor.cc
@@ -11250,6 +11250,10 @@ bool OSDMonitor::prepare_pool_op(MonOpRequestRef op)
 
   case POOL_OP_DELETE_UNMANAGED_SNAP:
     if (!pp.is_removed_snap(m->snapid)) {
+      if (m->snapid > pp.get_snap_seq()) {
+        _pool_op_reply(op, -ENOENT, osdmap.get_epoch());
+        return false;
+      }
       pp.remove_unmanaged_snap(m->snapid);
       pending_inc.new_removed_snaps[m->pool].insert(m->snapid);
       changed = true;

diff --git a/src/osd/osd_types.cc b/src/osd/osd_types.cc
index bf53249c0a7f58f6b2812e675ee23d1c7cda0935..a532ba53845b25445727ce0719f74fd6bb60761e 100644 (file)
--- a/src/osd/osd_types.cc
+++ b/src/osd/osd_types.cc
@@ -1366,8 +1366,10 @@ void pg_pool_t::remove_unmanaged_snap(snapid_t s)
   assert(is_unmanaged_snaps_mode());
   removed_snaps.insert(s);
   snap_seq = snap_seq + 1;
-  // add in the new seq, just to try to keep the interval_set contiguous
-  removed_snaps.insert(get_snap_seq());
+  // try to add in the new seq, just to try to keep the interval_set contiguous
+  if (!removed_snaps.contains(get_snap_seq())) {
+    removed_snaps.insert(get_snap_seq());
+  }
 }
 
 SnapContext pg_pool_t::get_snap_context() const