From d41c3e858c6f215792c67b8c2a42312cae07ece9 Mon Sep 17 00:00:00 2001 From: Yehuda Sadeh Date: Fri, 12 Sep 2014 14:07:44 -0700 Subject: [PATCH] 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 --- src/rgw/rgw_op.cc | 10 ++-------- src/rgw/rgw_rados.cc | 23 ++++++++++++++++++----- src/rgw/rgw_rados.h | 10 +++++++--- 3 files changed, 27 insertions(+), 16 deletions(-) diff --git a/src/rgw/rgw_op.cc b/src/rgw/rgw_op.cc index 6340fb14fb4e..7522fc52a532 100644 --- a/src/rgw/rgw_op.cc +++ b/src/rgw/rgw_op.cc @@ -1575,22 +1575,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; @@ -1728,6 +1721,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 e903736c5153..7e87674dbb3c 100644 --- 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) { @@ -3045,7 +3058,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; @@ -3510,7 +3523,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 0042df2f4a4e..669cb2d71ec0 100644 --- a/src/rgw/rgw_rados.h +++ b/src/rgw/rgw_rados.h @@ -565,8 +565,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& attrs); CephContext *ctx(); @@ -583,7 +586,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& attrs); public: @@ -673,7 +676,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; } }; -- 2.47.3