Merge pull request #6639 from xiexingguo/xxg-wip-13822

librados: potential null pointer access in list_(n)objects Reviewed-by: Kefu Chai <kchai@redhat.com>
ceph · Dec 31, 2015 · da58110 · da58110
2 parents 5dd1cb0 + 1908d32
commit da58110
Show file tree

Hide file tree

Showing 4 changed files with 71 additions and 2 deletions.
diff --git a/src/osdc/Objecter.cc b/src/osdc/Objecter.cc
@@ -3372,13 +3372,18 @@ void Objecter::list_nobjects(NListContext *list_context, Context *onfinish)
     // release the listing context's budget once all
     // OPs (in the session) are finished
     put_nlist_context_budget(list_context);
-
     onfinish->complete(0);
     return;
   }
 
   rwlock.get_read();
   const pg_pool_t *pool = osdmap->get_pg_pool(list_context->pool_id);
+  if (!pool) { // pool is gone
+    rwlock.unlock();
+    put_nlist_context_budget(list_context);
+    onfinish->complete(-ENOENT);
+    return;
+  }
   int pg_num = pool->get_pg_num();
   rwlock.unlock();
 
@@ -3523,13 +3528,18 @@ void Objecter::list_objects(ListContext *list_context, Context *onfinish)
     // release the listing context's budget once all
     // OPs (in the session) are finished
     put_list_context_budget(list_context);
-
     onfinish->complete(0);
     return;
   }
 
   rwlock.get_read();
   const pg_pool_t *pool = osdmap->get_pg_pool(list_context->pool_id);
+  if (!pool) { // pool is gone
+    rwlock.unlock();
+    put_list_context_budget(list_context);
+    onfinish->complete(-ENOENT);
+    return;
+  }
   int pg_num = pool->get_pg_num();
   rwlock.unlock();
 

diff --git a/src/test/librados/TestCase.h b/src/test/librados/TestCase.h
@@ -206,4 +206,17 @@ class RadosTestECPP : public RadosTestPP {
   std::string nspace;
   uint64_t alignment;
 };
+
+/**
+ * Test case without creating a temporary pool in advance.
+ * This is necessary for scenarios such that we need to
+ * manually create a pool, start some long-runing tasks and
+ * then the related pool is suddenly gone.
+ */
+class RadosTestNP: public ::testing::Test {
+public:
+  RadosTestNP() {}
+  virtual ~RadosTestNP() {}
+};
+
 #endif
diff --git a/src/test/librados/list.cc b/src/test/librados/list.cc
@@ -19,6 +19,7 @@ typedef RadosTestNS LibRadosList;
 typedef RadosTestPPNS LibRadosListPP;
 typedef RadosTestECNS LibRadosListEC;
 typedef RadosTestECPPNS LibRadosListECPP;
+typedef RadosTestNP LibRadosListNP;
 
 TEST_F(LibRadosList, ListObjects) {
   char buf[128];
@@ -883,5 +884,27 @@ TEST_F(LibRadosListPP, EnumerateObjectsSplitPP) {
   ASSERT_EQ(n_objects, saw_obj.size());
 }
 
+TEST_F(LibRadosListNP, ListObjectsError) {
+  std::string pool_name;
+  rados_t cluster;
+  rados_ioctx_t ioctx;
+  pool_name = get_temp_pool_name();
+  ASSERT_EQ("", create_one_pool(pool_name, &cluster));
+  ASSERT_EQ(0, rados_ioctx_create(cluster, pool_name.c_str(), &ioctx));
+  char buf[128];
+  memset(buf, 0xcc, sizeof(buf));
+  rados_ioctx_set_namespace(ioctx, "");
+  ASSERT_EQ(0, rados_write(ioctx, "foo", buf, sizeof(buf), 0));
+  ASSERT_EQ(0, rados_pool_delete(cluster, pool_name.c_str()));
+
+  rados_list_ctx_t ctx;
+  ASSERT_EQ(0, rados_objects_list_open(ioctx, &ctx));
+  const char *entry;
+  ASSERT_EQ(-ENOENT, rados_objects_list_next(ctx, &entry, NULL));
+  rados_objects_list_close(ctx);
+  rados_ioctx_destroy(ioctx);
+  rados_shutdown(cluster);
+}
+
 #pragma GCC diagnostic pop
 #pragma GCC diagnostic warning "-Wpragmas"
diff --git a/src/test/librados/nlist.cc b/src/test/librados/nlist.cc
@@ -16,6 +16,7 @@ typedef RadosTestNS LibRadosList;
 typedef RadosTestPPNS LibRadosListPP;
 typedef RadosTestECNS LibRadosListEC;
 typedef RadosTestECPPNS LibRadosListECPP;
+typedef RadosTestNP LibRadosListNP;
 
 TEST_F(LibRadosList, ListObjects) {
   char buf[128];
@@ -730,3 +731,25 @@ TEST_F(LibRadosListPP, ListObjectsFilterPP) {
   ASSERT_TRUE(foundit);
 }
 
+TEST_F(LibRadosListNP, ListObjectsError) {
+  std::string pool_name;
+  rados_t cluster;
+  rados_ioctx_t ioctx;
+  pool_name = get_temp_pool_name();
+  ASSERT_EQ("", create_one_pool(pool_name, &cluster));
+  ASSERT_EQ(0, rados_ioctx_create(cluster, pool_name.c_str(), &ioctx));
+  char buf[128];
+  memset(buf, 0xcc, sizeof(buf));
+  rados_ioctx_set_namespace(ioctx, "");
+  ASSERT_EQ(0, rados_write(ioctx, "foo", buf, sizeof(buf), 0));
+  ASSERT_EQ(0, rados_pool_delete(cluster, pool_name.c_str()));
+
+  rados_list_ctx_t ctx;
+  ASSERT_EQ(0, rados_nobjects_list_open(ioctx, &ctx));
+  const char *entry;
+  ASSERT_EQ(-ENOENT, rados_nobjects_list_next(ctx, &entry, NULL, NULL));
+  rados_nobjects_list_close(ctx);
+  rados_ioctx_destroy(ioctx);
+  rados_shutdown(cluster);
+}
+