librbd: update AioCompletion return value before evaluating pending count

If the pending count is decremented before the return value is updated,
there is a possibility of two ASIO threads concurrently decrementing the
pending count down from 2 -> 1 -> 0. In the second thread (the one that
performs the final decrement from 1 -> 0), it can finalize the completion
before the first thread has had a chance to update the return value.

Fixes: https://tracker.ceph.com/issues/47847
Signed-off-by: Jason Dillaman <dillaman@redhat.com>
(cherry picked from commit 94f3bce53c39017028ce44a80697f55af2a82e68)

diff --git a/src/librbd/io/AioCompletion.cc b/src/librbd/io/AioCompletion.cc
index 6644b608bd93e501f0ef5fdcbf2342e86a8dc14a..38eff1b802366e3aa67a5ca9f3e246683eba1c68 100644 (file)
--- a/src/librbd/io/AioCompletion.cc
+++ b/src/librbd/io/AioCompletion.cc
@@ -198,22 +198,22 @@ void AioCompletion::set_request_count(uint32_t count) {
 
 void AioCompletion::complete_request(ssize_t r)
 {
-  uint32_t previous_pending_count = pending_count--;
-  ceph_assert(previous_pending_count > 0);
-  auto pending_count = previous_pending_count - 1;
-
   ceph_assert(ictx != nullptr);
   CephContext *cct = ictx->cct;
 
   if (r > 0) {
     rval += r;
-  } else if (r != -EEXIST) {
+  } else if (r < 0 && r != -EEXIST) {
     // might race w/ another thread setting an error code but
     // first one wins
     int zero = 0;
     error_rval.compare_exchange_strong(zero, r);
   }
 
+  uint32_t previous_pending_count = pending_count--;
+  ceph_assert(previous_pending_count > 0);
+  auto pending_count = previous_pending_count - 1;
+
   ldout(cct, 20) << "cb=" << complete_cb << ", "
                  << "pending=" << pending_count << dendl;
   if (pending_count == 0) {