From a73777a9025663149b6d6e1a33b9c3d2ecc89e11 Mon Sep 17 00:00:00 2001 From: Sage Weil Date: Fri, 21 Sep 2012 17:38:24 -0700 Subject: [PATCH 1/4] osd: return -EPERM on insufficient caps Send a failure to the client instead of dropping the request on the floor. Fixes: #3066 Signed-off-by: Sage Weil --- src/osd/PG.cc | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/osd/PG.cc b/src/osd/PG.cc index d82bdf329c9..ba3f7960865 100644 --- a/src/osd/PG.cc +++ b/src/osd/PG.cc @@ -1583,8 +1583,10 @@ void PG::do_request(OpRequestRef op) { // do any pending flush do_pending_flush(); - if (!op_has_sufficient_caps(op)) + if (!op_has_sufficient_caps(op)) { + osd->reply_op_error(op, -EPERM); return; + } if (must_delay_request(op)) { waiting_for_map.push_back(op); return; From ef29d90ef296030e2127639580882961a0fc676f Mon Sep 17 00:00:00 2001 From: Sage Weil Date: Fri, 21 Sep 2012 17:39:01 -0700 Subject: [PATCH 2/4] osd: some whitespace Signed-off-by: Sage Weil --- src/osd/PG.cc | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/osd/PG.cc b/src/osd/PG.cc index ba3f7960865..428d575045e 100644 --- a/src/osd/PG.cc +++ b/src/osd/PG.cc @@ -1583,6 +1583,7 @@ void PG::do_request(OpRequestRef op) { // do any pending flush do_pending_flush(); + if (!op_has_sufficient_caps(op)) { osd->reply_op_error(op, -EPERM); return; @@ -1590,9 +1591,11 @@ void PG::do_request(OpRequestRef op) if (must_delay_request(op)) { waiting_for_map.push_back(op); return; - } else if (can_discard_request(op)) { + } + if (can_discard_request(op)) { return; - } else if (!flushed) { + } + if (!flushed) { waiting_for_active.push_back(op); return; } From 53b18e3ce61e8c915ea0851654904b27b6838295 Mon Sep 17 00:00:00 2001 From: Sage Weil Date: Fri, 21 Sep 2012 17:55:08 -0700 Subject: [PATCH 3/4] osd: catch decoding errors from client ops Check for decode errors in client ops and return -EINVAL. Fixes: #2500 Signed-off-by: Sage Weil --- src/osd/ReplicatedPG.cc | 104 +++++++++++++++++++++++++++++++++------- 1 file changed, 88 insertions(+), 16 deletions(-) diff --git a/src/osd/ReplicatedPG.cc b/src/osd/ReplicatedPG.cc index cfbfe244382..997b129a1eb 100644 --- a/src/osd/ReplicatedPG.cc +++ b/src/osd/ReplicatedPG.cc @@ -236,7 +236,12 @@ int ReplicatedPG::get_pgls_filter(bufferlist::iterator& iter, PGLSFilter **pfilt string type; PGLSFilter *filter; - ::decode(type, iter); + try { + ::decode(type, iter); + } + catch (buffer::error& e) { + return -EINVAL; + } if (type.compare("parent") == 0) { filter = new PGLSParentFilter(iter); @@ -1942,7 +1947,13 @@ int ReplicatedPG::do_osd_ops(OpContext *ctx, vector& ops) case CEPH_OSD_CMPXATTR_MODE_U64: { uint64_t u64val; - ::decode(u64val, bp); + try { + ::decode(u64val, bp); + } + catch (buffer::error& e) { + result = -EINVAL; + goto fail; + } dout(10) << "CEPH_OSD_OP_CMPXATTR name=" << name << " val=" << u64val << " op=" << (int)op.xattr.cmp_op << " mode=" << (int)op.xattr.cmp_mode << dendl; result = do_xattr_cmp_u64(op.xattr.cmp_op, u64val, xattr); @@ -2135,7 +2146,13 @@ int ReplicatedPG::do_osd_ops(OpContext *ctx, vector& ops) if (osd_op.indata.length()) { bufferlist::iterator p = osd_op.indata.begin(); string category; - ::decode(category, p); + try { + ::decode(category, p); + } + catch (buffer::error& e) { + result = -EINVAL; + goto fail; + } if (category.size()) { if (obs.exists) { if (obs.oi.category != category) @@ -2424,8 +2441,14 @@ int ReplicatedPG::do_osd_ops(OpContext *ctx, vector& ops) while (!bp.end() && !result) { __u8 op; string key; - ::decode(op, bp); - ::decode(key, bp); + try { + ::decode(op, bp); + ::decode(key, bp); + } + catch (buffer::error& e) { + result = -EINVAL; + goto fail; + } dout(10) << "tmapup op " << (int)op << " key " << key << dendl; @@ -2456,7 +2479,13 @@ int ReplicatedPG::do_osd_ops(OpContext *ctx, vector& ops) if (op == CEPH_OSD_TMAP_SET) { bufferlist val; - ::decode(val, bp); + try { + ::decode(val, bp); + } + catch (buffer::error& e) { + result = -EINVAL; + goto fail; + } ::encode(key, newkeydata); ::encode(val, newkeydata); dout(20) << " set " << key << " " << val.length() << dendl; @@ -2467,7 +2496,13 @@ int ReplicatedPG::do_osd_ops(OpContext *ctx, vector& ops) break; } bufferlist val; - ::decode(val, bp); + try { + ::decode(val, bp); + } + catch (buffer::error& e) { + result = -EINVAL; + goto fail; + } ::encode(key, newkeydata); ::encode(val, newkeydata); dout(20) << " create " << key << " " << val.length() << dendl; @@ -2533,8 +2568,14 @@ int ReplicatedPG::do_osd_ops(OpContext *ctx, vector& ops) { string start_after; uint64_t max_return; - ::decode(start_after, bp); - ::decode(max_return, bp); + try { + ::decode(start_after, bp); + ::decode(max_return, bp); + } + catch (buffer::error& e) { + result = -EINVAL; + goto fail; + } set out_set; if (oi.uses_tmap && g_conf->osd_auto_upgrade_tmap) { @@ -2578,9 +2619,15 @@ int ReplicatedPG::do_osd_ops(OpContext *ctx, vector& ops) string start_after; uint64_t max_return; string filter_prefix; - ::decode(start_after, bp); - ::decode(max_return, bp); - ::decode(filter_prefix, bp); + try { + ::decode(start_after, bp); + ::decode(max_return, bp); + ::decode(filter_prefix, bp); + } + catch (buffer::error& e) { + result = -EINVAL; + goto fail; + } map out_set; if (oi.uses_tmap && g_conf->osd_auto_upgrade_tmap) { @@ -2644,7 +2691,13 @@ int ReplicatedPG::do_osd_ops(OpContext *ctx, vector& ops) case CEPH_OSD_OP_OMAPGETVALSBYKEYS: { set keys_to_get; - ::decode(keys_to_get, bp); + try { + ::decode(keys_to_get, bp); + } + catch (buffer::error& e) { + result = -EINVAL; + goto fail; + } map out; if (oi.uses_tmap && g_conf->osd_auto_upgrade_tmap) { dout(20) << "CEPH_OSD_OP_OMAPGET: " @@ -2677,7 +2730,13 @@ int ReplicatedPG::do_osd_ops(OpContext *ctx, vector& ops) break; } map > assertions; - ::decode(assertions, bp); + try { + ::decode(assertions, bp); + } + catch (buffer::error& e) { + result = -EINVAL; + goto fail; + } map out; set to_get; @@ -2738,7 +2797,13 @@ int ReplicatedPG::do_osd_ops(OpContext *ctx, vector& ops) } t.touch(coll, soid); map to_set; - ::decode(to_set, bp); + try { + ::decode(to_set, bp); + } + catch (buffer::error& e) { + result = -EINVAL; + goto fail; + } dout(20) << "setting vals: " << dendl; for (map::iterator i = to_set.begin(); i != to_set.end(); @@ -2785,7 +2850,13 @@ int ReplicatedPG::do_osd_ops(OpContext *ctx, vector& ops) } t.touch(coll, soid); set to_rm; - ::decode(to_rm, bp); + try { + ::decode(to_rm, bp); + } + catch (buffer::error& e) { + result = -EINVAL; + goto fail; + } t.omap_rmkeys(coll, soid, to_rm); } break; @@ -2798,6 +2869,7 @@ int ReplicatedPG::do_osd_ops(OpContext *ctx, vector& ops) ctx->bytes_read += osd_op.outdata.length(); + fail: if (result < 0 && (op.flags & CEPH_OSD_OP_FLAG_FAILOK)) result = 0; From a794c936ffafd007f0836e10f7e851b08d1a6b57 Mon Sep 17 00:00:00 2001 From: Samuel Just Date: Thu, 13 Sep 2012 14:13:18 -0700 Subject: [PATCH 4/4] ReplicatedPG: set op return version to pg version on ENOENT This will be useful for detecting read-delete races in radosgw. Signed-off-by: Samuel Just --- src/osd/ReplicatedPG.cc | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/osd/ReplicatedPG.cc b/src/osd/ReplicatedPG.cc index 997b129a1eb..cce8a94df3a 100644 --- a/src/osd/ReplicatedPG.cc +++ b/src/osd/ReplicatedPG.cc @@ -959,6 +959,8 @@ void ReplicatedPG::do_op(OpRequestRef op) if (result >= 0) ctx->reply->set_version(ctx->reply_version); + else if (result == -ENOENT) + ctx->reply->set_version(info.last_update); // read or error? if (ctx->op_t.empty() || result < 0) {