rgw: push hash calculater deeper

This might have been the culprit for #9307. Before we were calculating
the hash after the call to processor->handle_data(), however, that
method might have spliced the bufferlist, so we can't be sure that the
pointer that we were holding originally is still invalid. Instead, push
the hash calculation down. Added a new explicit complete_hash() call to
the processor, since when we're at complete() it's too late (we need to
have the hash at that point already).

Signed-off-by: Yehuda Sadeh <yehuda@redhat.com>
(cherry picked from commit d41c3e858c6f215792c67b8c2a42312cae07ece9)

Conflicts:
	src/rgw/rgw_rados.h

diff --git a/src/rgw/rgw_op.cc b/src/rgw/rgw_op.cc
index 76947480bb4f131da781b2f92d90c2ae72c86c39..457791fac3101bcd4d3094c26499f1c2448c0865 100644 (file)
--- a/src/rgw/rgw_op.cc
+++ b/src/rgw/rgw_op.cc
@@ -1566,22 +1566,15 @@ int RGWPutObj::user_manifest_iterate_cb(rgw_bucket& bucket, RGWObjEnt& ent, RGWA
 static int put_data_and_throttle(RGWPutObjProcessor *processor, bufferlist& data, off_t ofs,
                                  MD5 *hash, bool need_to_wait)
 {
-  const unsigned char *data_ptr = (hash ? (const unsigned char *)data.c_str() : NULL);
   bool again;
-  uint64_t len = data.length();
 
   do {
     void *handle;
 
-    int ret = processor->handle_data(data, ofs, &handle, &again);
+    int ret = processor->handle_data(data, ofs, hash, &handle, &again);
     if (ret < 0)
       return ret;
 
-    if (hash) {
-      hash->Update(data_ptr, len);
-      hash = NULL; /* only calculate hash once */
-    }
-
     ret = processor->throttle_data(handle, need_to_wait);
     if (ret < 0)
       return ret;
@@ -1719,6 +1712,7 @@ void RGWPutObj::execute()
   }
 
   if (need_calc_md5) {
+    processor->complete_hash(&hash);
     hash.Final(m);
 
     buf_to_hex(m, CEPH_CRYPTO_MD5_DIGESTSIZE, calc_md5);

diff --git a/src/rgw/rgw_rados.cc b/src/rgw/rgw_rados.cc
index 4ac2991b65ad28f631f26ddc03fd1373f7f37382..ffa8a723ef32c39e0421f87f90a0b56df01ce771 100644 (file)
--- a/src/rgw/rgw_rados.cc
+++ b/src/rgw/rgw_rados.cc
@@ -905,8 +905,10 @@ int RGWPutObjProcessor_Plain::prepare(RGWRados *store, void *obj_ctx, string *oi
   return 0;
 };
 
-int RGWPutObjProcessor_Plain::handle_data(bufferlist& bl, off_t _ofs, void **phandle, bool *again)
+int RGWPutObjProcessor_Plain::handle_data(bufferlist& bl, off_t _ofs, MD5 *hash, void **phandle, bool *again)
 {
+  assert(!hash);
+
   *again = false;
 
   if (ofs != _ofs)
@@ -1033,7 +1035,7 @@ int RGWPutObjProcessor_Atomic::write_data(bufferlist& bl, off_t ofs, void **phan
   return RGWPutObjProcessor_Aio::handle_obj_data(cur_obj, bl, ofs - cur_part_ofs, ofs, phandle, exclusive);
 }
 
-int RGWPutObjProcessor_Atomic::handle_data(bufferlist& bl, off_t ofs, void **phandle, bool *again)
+int RGWPutObjProcessor_Atomic::handle_data(bufferlist& bl, off_t ofs, MD5 *hash, void **phandle, bool *again)
 {
   *again = false;
 
@@ -1067,7 +1069,10 @@ int RGWPutObjProcessor_Atomic::handle_data(bufferlist& bl, off_t ofs, void **pha
   if (!data_ofs && !immutable_head()) {
     first_chunk.claim(bl);
     obj_len = (uint64_t)first_chunk.length();
-    int r = prepare_next_part(first_chunk.length());
+    if (hash) {
+      hash->Update((const byte *)first_chunk.c_str(), obj_len);
+    }
+    int r = prepare_next_part(obj_len);
     if (r < 0) {
       return r;
     }
@@ -1079,6 +1084,9 @@ int RGWPutObjProcessor_Atomic::handle_data(bufferlist& bl, off_t ofs, void **pha
   bool exclusive = (!write_ofs && immutable_head()); /* immutable head object, need to verify nothing exists there
                                                         we could be racing with another upload, to the same
                                                         object and cleanup can be messy */
+  if (hash) {
+    hash->Update((const byte *)bl.c_str(), bl.length());
+  }
   int ret = write_data(bl, write_ofs, phandle, exclusive);
   if (ret >= 0) { /* we might return, need to clear bl as it was already sent */
     bl.clear();
@@ -1086,6 +1094,11 @@ int RGWPutObjProcessor_Atomic::handle_data(bufferlist& bl, off_t ofs, void **pha
   return ret;
 }
 
+void RGWPutObjProcessor_Atomic::complete_hash(MD5 *hash)
+{
+  hash->Update((const byte *)pending_data_bl.c_str(), pending_data_bl.length());
+}
+
 
 int RGWPutObjProcessor_Atomic::prepare_init(RGWRados *store, void *obj_ctx, string *oid_rand)
 {
@@ -3028,7 +3041,7 @@ public:
 
     do {
       void *handle;
-      int ret = processor->handle_data(bl, ofs, &handle, &again);
+      int ret = processor->handle_data(bl, ofs, NULL, &handle, &again);
       if (ret < 0)
         return ret;
 
@@ -3462,7 +3475,7 @@ int RGWRados::copy_obj_data(void *ctx,
     do {
       void *handle;
 
-      ret = processor.handle_data(bl, ofs, &handle, &again);
+      ret = processor.handle_data(bl, ofs, NULL, &handle, &again);
       if (ret < 0) {
         return ret;
       }

diff --git a/src/rgw/rgw_rados.h b/src/rgw/rgw_rados.h
index dcaec8384a91e14a987ff515e19b4c66919092f0..9aefe1efee98ec796c121154eb61fd4af3a1f78f 100644 (file)
--- a/src/rgw/rgw_rados.h
+++ b/src/rgw/rgw_rados.h
@@ -553,8 +553,11 @@ public:
     obj_ctx = _o;
     return 0;
   };
-  virtual int handle_data(bufferlist& bl, off_t ofs, void **phandle, bool *again) = 0;
+  virtual int handle_data(bufferlist& bl, off_t ofs, MD5 *hash, void **phandle, bool *again) = 0;
   virtual int throttle_data(void *handle, bool need_to_wait) = 0;
+  virtual void complete_hash(MD5 *hash) {
+    assert(0);
+  }
   virtual int complete(string& etag, time_t *mtime, time_t set_mtime, map<string, bufferlist>& attrs);
 
   CephContext *ctx();
@@ -571,7 +574,7 @@ class RGWPutObjProcessor_Plain : public RGWPutObjProcessor
 
 protected:
   int prepare(RGWRados *store, void *obj_ctx, string *oid_rand);
-  int handle_data(bufferlist& bl, off_t ofs, void **phandle, bool *again);
+  int handle_data(bufferlist& bl, off_t ofs, MD5 *hash /* NULL expected */, void **phandle, bool *again);
   int do_complete(string& etag, time_t *mtime, time_t set_mtime, map<string, bufferlist>& attrs);
 
 public:
@@ -661,7 +664,8 @@ public:
   void set_extra_data_len(uint64_t len) {
     extra_data_len = len;
   }
-  virtual int handle_data(bufferlist& bl, off_t ofs, void **phandle, bool *again);
+  virtual int handle_data(bufferlist& bl, off_t ofs, MD5 *hash, void **phandle, bool *again);
+  virtual void complete_hash(MD5 *hash);
   bufferlist& get_extra_data() { return extra_data_bl; }
 };