Merge pull request #11936: hammer: librados: bad flags can crash the osd

Reviewed-by: Nathan Cutler <ncutler@suse.com>
ceph · Nov 20, 2016 · 6609bbb · 6609bbb
2 parents f8d9992 + 42bd025
commit 6609bbb
Show file tree

Hide file tree

Showing 7 changed files with 42 additions and 12 deletions.
diff --git a/src/include/rados/librados.hpp b/src/include/rados/librados.hpp
@@ -656,6 +656,7 @@ namespace librados
                    size_t len);
     int read(const std::string& oid, bufferlist& bl, size_t len, uint64_t off);
     int remove(const std::string& oid);
+    int remove(const std::string& oid, int flags);
     int trunc(const std::string& oid, uint64_t size);
     int mapext(const std::string& o, uint64_t off, size_t len, std::map<uint64_t,uint64_t>& m);
     int sparse_read(const std::string& o, std::map<uint64_t,uint64_t>& m, bufferlist& bl, size_t len, uint64_t off);

diff --git a/src/librados/IoCtxImpl.cc b/src/librados/IoCtxImpl.cc
@@ -832,6 +832,14 @@ int librados::IoCtxImpl::remove(const object_t& oid)
   return operate(oid, &op, NULL);
 }
 
+int librados::IoCtxImpl::remove(const object_t& oid, int flags)
+{
+  ::ObjectOperation op;
+  prepare_assert_ops(&op);
+  op.remove();
+  return operate(oid, &op, NULL, flags);
+}
+
 int librados::IoCtxImpl::trunc(const object_t& oid, uint64_t size)
 {
   ::ObjectOperation op;

diff --git a/src/librados/IoCtxImpl.h b/src/librados/IoCtxImpl.h
@@ -127,6 +127,7 @@ struct librados::IoCtxImpl {
   int sparse_read(const object_t& oid, std::map<uint64_t,uint64_t>& m,
 		  bufferlist& bl, size_t len, uint64_t off);
   int remove(const object_t& oid);
+  int remove(const object_t& oid, int flags);
   int stat(const object_t& oid, uint64_t *psize, time_t *pmtime);
   int trunc(const object_t& oid, uint64_t size);
 

diff --git a/src/librados/librados.cc b/src/librados/librados.cc
@@ -1111,6 +1111,12 @@ int librados::IoCtx::remove(const std::string& oid)
   return io_ctx_impl->remove(obj);
 }
 
+int librados::IoCtx::remove(const std::string& oid, int flags)
+{
+  object_t obj(oid);
+  return io_ctx_impl->remove(obj, flags);
+}
+
 int librados::IoCtx::trunc(const std::string& oid, uint64_t size)
 {
   object_t obj(oid);

diff --git a/src/messages/MOSDOp.h b/src/messages/MOSDOp.h
@@ -154,6 +154,7 @@ class MOSDOp : public Message {
 
   // flags
   int get_flags() const { return flags; }
+  bool has_flag(__u32 flag) { return flags & flag; };
 
   bool wants_ack() const { return flags & CEPH_OSD_FLAG_ACK; }
   bool wants_ondisk() const { return flags & CEPH_OSD_FLAG_ONDISK; }

diff --git a/src/osd/ReplicatedPG.cc b/src/osd/ReplicatedPG.cc
@@ -1365,6 +1365,13 @@ void ReplicatedPG::do_op(OpRequestRef& op)
 {
   MOSDOp *m = static_cast<MOSDOp*>(op->get_req());
   assert(m->get_type() == CEPH_MSG_OSD_OP);
+
+  if (m->has_flag(CEPH_OSD_FLAG_PARALLELEXEC)) {
+    // not implemented.
+    osd->reply_op_error(op, -EINVAL);
+    return;
+  }
+
   if (op->includes_pg_op()) {
     if (pg_op_must_wait(m)) {
       wait_for_all_missing(op);
@@ -7478,12 +7485,6 @@ void ReplicatedPG::issue_repop(RepGather *repop)
 {
   OpContext *ctx = repop->ctx;
   const hobject_t& soid = ctx->obs->oi.soid;
-  if (ctx->op &&
-    ((static_cast<MOSDOp *>(
-	ctx->op->get_req()))->get_flags() & CEPH_OSD_FLAG_PARALLELEXEC)) {
-    // replicate original op for parallel execution on replica
-    assert(0 == "broken implementation, do not use");
-  }
   dout(7) << "issue_repop rep_tid " << repop->rep_tid
           << " o " << soid
           << dendl;

diff --git a/src/test/librados/misc.cc b/src/test/librados/misc.cc
@@ -357,6 +357,24 @@ TEST_F(LibRadosMiscPP, ExecPP) {
   ASSERT_NE(all_features, 0);
 }
 
+void set_completion_complete(rados_completion_t cb, void *arg)
+{
+  bool *my_aio_complete = (bool*)arg;
+  *my_aio_complete = true;
+}
+
+TEST_F(LibRadosMiscPP, BadFlagsPP) {
+  unsigned badflags = CEPH_OSD_FLAG_PARALLELEXEC;
+  {
+    bufferlist bl;
+    bl.append("data");
+    ASSERT_EQ(0, ioctx.write("badfoo", bl, bl.length(), 0));
+  }
+  {
+    ASSERT_EQ(-EINVAL, ioctx.remove("badfoo", badflags));
+  }
+}
+
 TEST_F(LibRadosMiscPP, Operate1PP) {
   ObjectWriteOperation o;
   {
@@ -450,12 +468,6 @@ TEST_F(LibRadosMiscPP, BigObjectPP) {
 #endif
 }
 
-void set_completion_complete(rados_completion_t cb, void *arg)
-{
-  bool *my_aio_complete = (bool*)arg;
-  *my_aio_complete = true;
-}
-
 TEST_F(LibRadosMiscPP, AioOperatePP) {
   bool my_aio_complete = false;
   AioCompletion *my_completion = cluster.aio_create_completion(