From f7690d35f7b1b9ec6c5de39f32128329dcce17da Mon Sep 17 00:00:00 2001 From: Radoslaw Zarzynski Date: Mon, 24 Jul 2017 09:24:54 -0400 Subject: [PATCH] rgw: fix memory leaks during Swift Static Website's error handling. Fixes: http://tracker.ceph.com/issues/20757 Signed-off-by: Radoslaw Zarzynski --- src/rgw/rgw_common.cc | 34 ++++++++++++++++------------------ src/rgw/rgw_common.h | 4 ++-- src/rgw/rgw_op.cc | 35 +++++++++++++++++++++-------------- src/rgw/rgw_rest_swift.cc | 3 ++- 4 files changed, 41 insertions(+), 35 deletions(-) diff --git a/src/rgw/rgw_common.cc b/src/rgw/rgw_common.cc index 72891f9c376d..17f9f934bdac 100644 --- a/src/rgw/rgw_common.cc +++ b/src/rgw/rgw_common.cc @@ -268,8 +268,6 @@ req_state::req_state(CephContext* _cct, RGWEnv* e, RGWUserInfo* u) content_started = false; format = 0; formatter = NULL; - bucket_acl = NULL; - object_acl = NULL; expect_cont = false; obj_size = 0; @@ -291,8 +289,6 @@ req_state::req_state(CephContext* _cct, RGWEnv* e, RGWUserInfo* u) req_state::~req_state() { delete formatter; - delete bucket_acl; - delete object_acl; } bool search_err(rgw_http_errors& errs, int err_no, bool is_website_redirect, int& http_ret, string& code) @@ -1138,18 +1134,18 @@ bool verify_bucket_permission_no_policy(struct req_state * const s, const int pe return false; return verify_bucket_permission_no_policy(s, - s->user_acl.get(), - s->bucket_acl, - perm); + s->user_acl.get(), + s->bucket_acl.get(), + perm); } bool verify_bucket_permission(struct req_state * const s, const uint64_t op) { return verify_bucket_permission(s, - s->bucket, + s->bucket, s->user_acl.get(), - s->bucket_acl, - s->iam_policy, + s->bucket_acl.get(), + s->iam_policy, op); } @@ -1293,19 +1289,21 @@ bool verify_object_permission_no_policy(struct req_state *s, int perm) if (!verify_requester_payer_permission(s)) return false; - return verify_object_permission_no_policy(s, s->user_acl.get(), - s->bucket_acl, s->object_acl, - perm); + return verify_object_permission_no_policy(s, + s->user_acl.get(), + s->bucket_acl.get(), + s->object_acl.get(), + perm); } bool verify_object_permission(struct req_state *s, uint64_t op) { return verify_object_permission(s, - rgw_obj(s->bucket, s->object), - s->user_acl.get(), - s->bucket_acl, - s->object_acl, - s->iam_policy, + rgw_obj(s->bucket, s->object), + s->user_acl.get(), + s->bucket_acl.get(), + s->object_acl.get(), + s->iam_policy, op); } diff --git a/src/rgw/rgw_common.h b/src/rgw/rgw_common.h index 699cd19788b7..7e455dce9832 100644 --- a/src/rgw/rgw_common.h +++ b/src/rgw/rgw_common.h @@ -1801,8 +1801,8 @@ struct req_state { } auth; std::unique_ptr user_acl; - RGWAccessControlPolicy *bucket_acl; - RGWAccessControlPolicy *object_acl; + std::unique_ptr bucket_acl; + std::unique_ptr object_acl; rgw::IAM::Environment env; boost::optional iam_policy; diff --git a/src/rgw/rgw_op.cc b/src/rgw/rgw_op.cc index c02b3d8396dd..00a88c6f9832 100644 --- a/src/rgw/rgw_op.cc +++ b/src/rgw/rgw_op.cc @@ -16,6 +16,7 @@ #include "common/Clock.h" #include "common/armor.h" +#include "common/backport14.h" #include "common/errno.h" #include "common/mime.h" #include "common/utf8.h" @@ -401,18 +402,17 @@ int rgw_build_bucket_policies(RGWRados* store, struct req_state* s) } if(s->dialect.compare("s3") == 0) { - s->bucket_acl = new RGWAccessControlPolicy_S3(s->cct); + s->bucket_acl = ceph::make_unique(s->cct); } else if(s->dialect.compare("swift") == 0) { /* We aren't allocating the account policy for those operations using * the Swift's infrastructure that don't really need req_state::user. * Typical example here is the implementation of /info. */ if (!s->user->user_id.empty()) { - s->user_acl = std::unique_ptr( - new RGWAccessControlPolicy_SWIFTAcct(s->cct)); + s->user_acl = ceph::make_unique(s->cct); } - s->bucket_acl = new RGWAccessControlPolicy_SWIFT(s->cct); + s->bucket_acl = ceph::make_unique(s->cct); } else { - s->bucket_acl = new RGWAccessControlPolicy(s->cct); + s->bucket_acl = ceph::make_unique(s->cct); } /* check if copy source is within the current domain */ @@ -457,7 +457,8 @@ int rgw_build_bucket_policies(RGWRados* store, struct req_state* s) s->bucket = s->bucket_info.bucket; if (s->bucket_exists) { - ret = read_bucket_policy(store, s, s->bucket_info, s->bucket_attrs, s->bucket_acl, s->bucket); + ret = read_bucket_policy(store, s, s->bucket_info, s->bucket_attrs, + s->bucket_acl.get(), s->bucket); acct_acl_user = { s->bucket_info.owner, s->bucket_acl->get_owner().get_display_name(), @@ -560,7 +561,7 @@ int rgw_build_object_policies(RGWRados *store, struct req_state *s, if (!s->bucket_exists) { return -ERR_NO_SUCH_BUCKET; } - s->object_acl = new RGWAccessControlPolicy(s->cct); + s->object_acl = ceph::make_unique(s->cct); rgw_obj obj(s->bucket, s->object); @@ -568,7 +569,9 @@ int rgw_build_object_policies(RGWRados *store, struct req_state *s, if (prefetch_data) { store->set_prefetch_data(s->obj_ctx, obj); } - ret = read_obj_policy(store, s, s->bucket_info, s->bucket_attrs, s->object_acl, s->iam_policy, s->bucket, s->object); + ret = read_obj_policy(store, s, s->bucket_info, s->bucket_attrs, + s->object_acl.get(), s->iam_policy, s->bucket, + s->object); } return ret; @@ -1327,7 +1330,7 @@ int RGWGetObj::handle_user_manifest(const char *prefix) } else { bucket = s->bucket; pbucket_info = &s->bucket_info; - bucket_acl = s->bucket_acl; + bucket_acl = s->bucket_acl.get(); bucket_policy = &s->iam_policy; } @@ -1464,7 +1467,7 @@ int RGWGetObj::handle_slo_manifest(bufferlist& bl) } } else { bucket = s->bucket; - bucket_acl = s->bucket_acl; + bucket_acl = s->bucket_acl.get(); bucket_policy = s->iam_policy.get_ptr(); } @@ -3923,7 +3926,8 @@ void RGWPutMetadataBucket::execute() * contain such keys yet. */ if (has_policy) { if (s->dialect.compare("swift") == 0) { - auto old_policy = static_cast(s->bucket_acl); + auto old_policy = \ + static_cast(s->bucket_acl.get()); auto new_policy = static_cast(&policy); new_policy->filter_merge(policy_rw_mask, old_policy); policy = *new_policy; @@ -4480,8 +4484,10 @@ void RGWGetACLs::pre_exec() void RGWGetACLs::execute() { stringstream ss; - RGWAccessControlPolicy *acl = (!s->object.empty() ? s->object_acl : s->bucket_acl); - RGWAccessControlPolicy_S3 *s3policy = static_cast(acl); + RGWAccessControlPolicy* const acl = \ + (!s->object.empty() ? s->object_acl.get() : s->bucket_acl.get()); + RGWAccessControlPolicy_S3* const s3policy = \ + static_cast(acl); s3policy->to_xml(ss); acls = ss.str(); } @@ -4574,7 +4580,8 @@ void RGWPutACLs::execute() } - RGWAccessControlPolicy *existing_policy = (s->object.empty() ? s->bucket_acl : s->object_acl); + RGWAccessControlPolicy* const existing_policy = \ + (s->object.empty() ? s->bucket_acl.get() : s->object_acl.get()); owner = existing_policy->get_owner(); diff --git a/src/rgw/rgw_rest_swift.cc b/src/rgw/rgw_rest_swift.cc index 90a829da5b92..f1b47a4fa545 100644 --- a/src/rgw/rgw_rest_swift.cc +++ b/src/rgw/rgw_rest_swift.cc @@ -384,7 +384,8 @@ static void dump_container_metadata(struct req_state *s, dump_header(s, "X-Container-Bytes-Used-Actual", bucket.size_rounded); if (s->object.empty()) { - auto swift_policy = static_cast(s->bucket_acl); + auto swift_policy = \ + static_cast(s->bucket_acl.get()); std::string read_acl, write_acl; swift_policy->to_str(read_acl, write_acl); -- 2.47.3