From: JINMEI Tatuya <jinmei@isc.org>
Date: Wed, 22 Aug 2012 01:48:01 +0000 (-0700)
Subject: [2107] make NSEC3Data::create() exception safe.
X-Git-Tag: trac2351_base~109^2~1^2~29
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=e21efca5f30a849289a3daa44d2f9b3526fd3e86;p=thirdparty%2Fkea.git

[2107] make NSEC3Data::create() exception safe.
---

diff --git a/src/lib/datasrc/memory/tests/zone_data_unittest.cc b/src/lib/datasrc/memory/tests/zone_data_unittest.cc
index a6bbff8d7f..3d1100044e 100644
--- a/src/lib/datasrc/memory/tests/zone_data_unittest.cc
+++ b/src/lib/datasrc/memory/tests/zone_data_unittest.cc
@@ -40,6 +40,43 @@ using namespace isc::testutils;
 
 namespace {
 
+class NSEC3DataTest : public ::testing::Test {
+protected:
+    NSEC3DataTest() : param_rdata_("1 0 12 aabbccdd")
+    {}
+    void TearDown() {
+        // detect any memory leak in the test memory segment
+        EXPECT_TRUE(mem_sgmt_.allMemoryDeallocated());
+    }
+
+    MemorySegmentTest mem_sgmt_;
+    NSEC3Data* nsec3_data_;
+    const generic::NSEC3PARAM param_rdata_;
+};
+
+TEST_F(NSEC3DataTest, create) {
+    nsec3_data_ = NSEC3Data::create(mem_sgmt_, param_rdata_);
+    EXPECT_EQ(0, nsec3_data_->getNSEC3Tree()->getNodeCount());
+    EXPECT_EQ(1, nsec3_data_->hashalg);
+    EXPECT_EQ(0, nsec3_data_->flags);
+    EXPECT_EQ(12, nsec3_data_->iterations);
+    EXPECT_EQ(param_rdata_.getSalt().size(), nsec3_data_->getSaltLen());
+    EXPECT_EQ(0, memcmp(&param_rdata_.getSalt()[0], nsec3_data_->getSaltData(),
+                        param_rdata_.getSalt().size()));
+    NSEC3Data::destroy(mem_sgmt_, nsec3_data_, RRClass::IN());
+}
+
+TEST_F(NSEC3DataTest, throwOnCreate) {
+    // Note: below, we use our knowledge of how memory allocation happens
+    // within the NSEC3Data.
+
+    // Creating internal NSEC3 tree will succeed, but allocation of NSEC3Data
+    // will fail due to bad_alloc.  It shouldn't cause memory leak
+    // (that would be caught in TearDown()).
+    mem_sgmt_.setThrowCount(2);
+    EXPECT_THROW(NSEC3Data::create(mem_sgmt_, param_rdata_), std::bad_alloc);
+}
+
 class ZoneDataTest : public ::testing::Test {
 protected:
     ZoneDataTest() : zname_("example.com"),
@@ -105,30 +142,4 @@ TEST_F(ZoneDataTest, addRdataSets) {
 
     // TearDown() will confirm there's no leak on destroy
 }
-
-class NSEC3DataTest : public ::testing::Test {
-protected:
-    NSEC3DataTest() : param_rdata_("1 0 12 aabbccdd")
-    {}
-    void TearDown() {
-        // detect any memory leak in the test memory segment
-        EXPECT_TRUE(mem_sgmt_.allMemoryDeallocated());
-    }
-
-    MemorySegmentTest mem_sgmt_;
-    NSEC3Data* nsec3_data_;
-    const generic::NSEC3PARAM param_rdata_;
-};
-
-TEST_F(NSEC3DataTest, create) {
-    nsec3_data_ = NSEC3Data::create(mem_sgmt_, param_rdata_);
-    EXPECT_EQ(0, nsec3_data_->getNSEC3Tree()->getNodeCount());
-    EXPECT_EQ(1, nsec3_data_->hashalg);
-    EXPECT_EQ(0, nsec3_data_->flags);
-    EXPECT_EQ(12, nsec3_data_->iterations);
-    EXPECT_EQ(param_rdata_.getSalt().size(), nsec3_data_->getSaltLen());
-    EXPECT_EQ(0, memcmp(&param_rdata_.getSalt()[0], nsec3_data_->getSaltData(),
-                        param_rdata_.getSalt().size()));
-    NSEC3Data::destroy(mem_sgmt_, nsec3_data_, RRClass::IN());
-}
 }
diff --git a/src/lib/datasrc/memory/zone_data.cc b/src/lib/datasrc/memory/zone_data.cc
index 3e38959ab2..3c6340993a 100644
--- a/src/lib/datasrc/memory/zone_data.cc
+++ b/src/lib/datasrc/memory/zone_data.cc
@@ -58,14 +58,21 @@ NSEC3Data*
 NSEC3Data::create(util::MemorySegment& mem_sgmt,
                   const generic::NSEC3PARAM& rdata)
 {
-    ZoneTree* tree = ZoneTree::create(mem_sgmt, true);
+    // NSEC3Data allocation can throw.  To avoid leaking the tree, we manage
+    // it in the holder.
+    // Note: we won't add any RdataSet, so we use the NO-OP deleter
+    // (with an assertion check for that).
+    typedef boost::function<void(RdataSet*)> RdataSetDeleterType;
+    detail::SegmentObjectHolder<ZoneTree, RdataSetDeleterType> holder(
+        mem_sgmt, ZoneTree::create(mem_sgmt, true),
+        boost::bind(nullDeleter, _1));
 
     const size_t salt_len = rdata.getSalt().size();
 
     void* p = mem_sgmt.allocate(sizeof(NSEC3Data) + salt_len + 1);
     NSEC3Data* const param_data =
-        new(p) NSEC3Data(tree, rdata.getHashalg(), rdata.getFlags(),
-                         rdata.getIterations());
+        new(p) NSEC3Data(holder.release(), rdata.getHashalg(),
+                         rdata.getFlags(), rdata.getIterations());
     uint8_t* dp = param_data->getSaltBuf();
     *dp++ =  salt_len;
     memcpy(dp, &rdata.getSalt().at(0), salt_len); // use at for safety
@@ -86,10 +93,8 @@ NSEC3Data::destroy(util::MemorySegment& mem_sgmt, NSEC3Data* data,
 
 ZoneData*
 ZoneData::create(util::MemorySegment& mem_sgmt, const Name& zone_origin) {
-    // ZoneTree::insert() and ZoneData allocation can throw.  To avoid
-    // leaking the tree, we manage it in the holder.
-    // Note: we won't add any RdataSet, so we use the NO-OP deleter
-    // (with an assertion check for that).
+    // ZoneTree::insert() and ZoneData allocation can throw.  See also
+    // NSEC3Data::create().
     typedef boost::function<void(RdataSet*)> RdataSetDeleterType;
     detail::SegmentObjectHolder<ZoneTree, RdataSetDeleterType> holder(
         mem_sgmt, ZoneTree::create(mem_sgmt, true),