From: Casey Bodley <cbodley@redhat.com>
Date: Fri, 23 Aug 2019 17:45:04 +0000 (-0400)
Subject: rgw: RGWCoroutine::call(nullptr) sets retcode=0
X-Git-Tag: v12.2.13~48^2
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=678ec2992b568f458a99d1dfdceb11cbe4c43195;p=ceph.git

rgw: RGWCoroutine::call(nullptr) sets retcode=0

callers generally expect 'yield call(...);' to set retcode with the
result of the given coroutine. it's valid to call() a nullptr, but
that's not always clear at the call site - for example, this critical
piece of code in RGWBucketSyncSingleEntryCR::operate():

      if (sync_status == 0) {
        /* update marker */
        set_status() << "calling marker_tracker->finish(" << entry_marker << ")";
        yield call(marker_tracker->finish(entry_marker));
        sync_status = retcode;
      }

marker_tracker->finish() only returns a coroutine when it needs to flush
the marker; otherwise it returns a nullptr. as a result, 'retcode' may
not be assigned as expected by the call(), and a previous value is
mistakenly returned up the stack

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

Signed-off-by: Casey Bodley <cbodley@redhat.com>
(cherry picked from commit 7d12a11a14c4a9c8e814b907d83dc07783e11e80)
---

diff --git a/src/rgw/rgw_coroutine.cc b/src/rgw/rgw_coroutine.cc
index 152aead410e..93d2e9e16c0 100644
--- a/src/rgw/rgw_coroutine.cc
+++ b/src/rgw/rgw_coroutine.cc
@@ -733,7 +733,12 @@ void RGWCoroutinesManagerRegistry::dump(Formatter *f) const {
 
 void RGWCoroutine::call(RGWCoroutine *op)
 {
-  stack->call(op);
+  if (op) {
+    stack->call(op);
+  } else {
+    // the call()er expects this to set a retcode
+    retcode = 0;
+  }
 }
 
 RGWCoroutinesStack *RGWCoroutine::spawn(RGWCoroutine *op, bool wait)