[2831] check prohibited operations on read-only segments more reliably

src/lib/util/memory_segment_mapped.cc

@@ -34,18 +34,20 @@ namespace util {
 
 struct MemorySegmentMapped::Impl {
     Impl(const std::string& filename, size_t initial_size) :
-        filename_(filename),
+        read_only_(false), filename_(filename),
         base_sgmt_(new managed_mapped_file(open_or_create, filename.c_str(),
                                            initial_size))
     {}
 
     Impl(const std::string& filename, bool read_only) :
-        filename_(filename),
+        read_only_(read_only), filename_(filename),
         base_sgmt_(read_only ?
                    new managed_mapped_file(open_read_only, filename.c_str()) :
                    new managed_mapped_file(open_only, filename.c_str()))
     {}
 
+    const bool read_only_;
+
     // mapped file; remember it in case we need to grow it.
     const std::string filename_;
 
@@ -88,6 +90,10 @@ MemorySegmentMapped::~MemorySegmentMapped() {
 
 void*
 MemorySegmentMapped::allocate(size_t size) {
+    if (impl_->read_only_) {
+        isc_throw(InvalidOperation, "allocate attempt on read-only segment");
+    }
+
     // We explicitly check the free memory size; it appears
     // managed_mapped_file::allocate() could incorrectly return a seemingly
     // valid pointer for some very large requested size.
@@ -157,6 +163,10 @@ MemorySegmentMapped::getNamedAddress(const char* name) {
 
 void
 MemorySegmentMapped::setNamedAddress(const char* name, void* addr) {
+    if (impl_->read_only_) {
+        isc_throw(InvalidOperation, "setNamedAddress on read-only segment");
+    }
+
     if (addr && !impl_->base_sgmt_->belongs_to_segment(addr)) {
         isc_throw(MemorySegmentError, "out of segment address: " << addr);
     }
@@ -167,11 +177,19 @@ MemorySegmentMapped::setNamedAddress(const char* name, void* addr) {
 
 bool
 MemorySegmentMapped::clearNamedAddress(const char* name) {
+    if (impl_->read_only_) {
+        isc_throw(InvalidOperation, "clearNamedAddress on read-only segment");
+    }
+
     return (impl_->base_sgmt_->destroy<offset_ptr<void> >(name));
 }
 
 void
 MemorySegmentMapped::shrinkToFit() {
+    if (impl_->read_only_) {
+        isc_throw(InvalidOperation, "shrinkToFit on read-only segment");
+    }
+
     // It appears an assertion failure is triggered within Boost if the size
     // is too small.  To work this around we'll make it no-op if the size is
     // already reasonably small.

src/lib/util/memory_segment_mapped.h

@@ -16,6 +16,7 @@
 #define MEMORY_SEGMENT_MAPPED_H
 
 #include <util/memory_segment.h>
+#include <exceptions/exceptions.h>
 
 #include <boost/noncopyable.hpp>
 

src/lib/util/tests/memory_segment_mapped_unittest.cc

@@ -13,6 +13,7 @@
 // PERFORMANCE OF THIS SOFTWARE.
 
 #include <util/memory_segment_mapped.h>
+#include <exceptions/exceptions.h>
 
 #include <gtest/gtest.h>
 
@@ -40,7 +41,7 @@ protected:
 
     ~MemorySegmentMappedTest() {
         segment_.reset();
-        //boost::interprocess::file_mapping::remove(mapped_file);
+        boost::interprocess::file_mapping::remove(mapped_file);
     }
 
     // For initialization and for tests after the segment possibly becomes
@@ -235,27 +236,6 @@ TEST_F(MemorySegmentMappedTest, namedAddress) {
     EXPECT_TRUE(segment_->allMemoryDeallocated());
 }
 
-TEST_F(MemorySegmentMappedTest, violateReadOnly) {
-    // If the segment is opened in the read only mode, modification attempt
-    // will result in crash.
-    EXPECT_DEATH_IF_SUPPORTED({
-            MemorySegmentMapped segment_ro(mapped_file);
-            segment_ro.allocate(16);
-        }, "");
-    EXPECT_DEATH_IF_SUPPORTED({
-            MemorySegmentMapped segment_ro(mapped_file);
-            segment_ro.setNamedAddress("test", 0);
-        }, "");
-    EXPECT_DEATH_IF_SUPPORTED({
-            void* ptr = segment_->allocate(sizeof(uint32_t));
-            segment_->setNamedAddress("test address", ptr);
-            MemorySegmentMapped segment_ro(mapped_file);
-            EXPECT_TRUE(segment_ro.getNamedAddress("test address"));
-            *static_cast<uint32_t*>(
-                segment_ro.getNamedAddress("test address")) = 0;
-        }, "");
-}
-
 TEST_F(MemorySegmentMappedTest, nullDeallocate) {
     // NULL deallocation is a no-op.
     EXPECT_NO_THROW(segment_->deallocate(0, 1024));
@@ -279,4 +259,33 @@ TEST_F(MemorySegmentMappedTest, shrink) {
     segment_->deallocate(p, sizeof(uint32_t));
 }
 
+TEST_F(MemorySegmentMappedTest, violateReadOnly) {
+    // If the segment is opened in the read only mode, modification attempts
+    // are prohibited.  when detectable it's result in an exception;
+    // an attempt of not directly through the segment class will result in
+    // crash.
+    EXPECT_THROW(MemorySegmentMapped(mapped_file).allocate(16),
+                 isc::InvalidOperation);
+    // allocation that would otherwise require growing the segment; permission
+    // check should be performed before that.
+    EXPECT_THROW(MemorySegmentMapped(mapped_file).
+                 allocate(DEFAULT_INITIAL_SIZE * 2),
+                 isc::InvalidOperation);
+    EXPECT_THROW(MemorySegmentMapped(mapped_file).setNamedAddress("test", 0),
+                 isc::InvalidOperation);
+    EXPECT_THROW(MemorySegmentMapped(mapped_file).clearNamedAddress("test"),
+                 isc::InvalidOperation);
+    EXPECT_THROW(MemorySegmentMapped(mapped_file).shrinkToFit(),
+                 isc::InvalidOperation);
+
+    EXPECT_DEATH_IF_SUPPORTED({
+            void* ptr = segment_->allocate(sizeof(uint32_t));
+            segment_->setNamedAddress("test address", ptr);
+            MemorySegmentMapped segment_ro(mapped_file);
+            EXPECT_TRUE(segment_ro.getNamedAddress("test address"));
+            *static_cast<uint32_t*>(
+                segment_ro.getNamedAddress("test address")) = 0;
+        }, "");
+}
+
 }