ensure returned Component refers to valid memory

* we have seen some core dumps in logctl when using
  the mmap'ed levels inside a Component.  Add extra
  checks to ensure we stay inside the actual file.

3 files changed, 18 insertions, 6 deletions

diff --git a/vespalog/src/vespa/log/component.h b/vespalog/src/vespa/log/component.h
index 9dd364af456..bb124a04b11 100644
--- a/vespalog/src/vespa/log/component.h
+++ b/vespalog/src/vespa/log/component.h
@@ -15,8 +15,8 @@ public:
     bool matches(const char *pattern);
     void modifyLevels(char *levels);
     void display();
+    const char *endPointer() const { return _charLevels + Logger::NUM_LOGLEVELS*sizeof(int); }
     explicit Component(char *);
 };
 
 } // end namespace ns_log
-
diff --git a/vespalog/src/vespa/log/control-file.cpp b/vespalog/src/vespa/log/control-file.cpp
index 32f5a850ccd..70833847905 100644
--- a/vespalog/src/vespa/log/control-file.cpp
+++ b/vespalog/src/vespa/log/control-file.cpp
@@ -26,6 +26,7 @@ ControlFile::ControlFile(const char *file, Mode mode)
                    O_NOCTTY | (  (mode == READONLY) ? O_RDONLY
                                : (mode == READWRITE) ? O_RDWR
                                : (O_RDWR | O_CREAT))),
+      _fileSize(0),
       _mode(mode),
       _fileName(strdup(file)),
       _prefix(0),
@@ -182,6 +183,7 @@ ControlFile::extendMapping()
         return false;
     }
     _mappedSize = size;
+    _fileSize = fileLen;
 
     return true;
 }
@@ -267,6 +269,7 @@ ControlFile::getLevels(const char *name)
             _fileName, wlen, len, strerror(errno));
         return reinterpret_cast<unsigned int *>(inheritLevels);
     } else {
+        _fileSize += wlen;
     }
 
     if (fileLength + wlen > _mappedSize) {
@@ -421,11 +424,18 @@ Component *
 ComponentIterator::next()
 {
     Component *ret = NULL;
-    if (_next && _next[0]) {
-        ret = new Component(_next);
-        _next = strchr(_next, '\n');
-        if (_next) {
-            ++_next;
+    if (_next && _cf->insideFile(_next) && _next[0]) {
+        char *nn = strchr(_next, '\n');
+        if (nn && _cf->insideFile(nn)) {
+            ret = new Component(_next);
+            _next = ++nn;
+            if (nn != ret->endPointer()) {
+                delete ret;
+                ret = NULL;
+                _next = NULL;
+            }
+        } else {
+            _next = NULL;
         }
     }
     return ret;
diff --git a/vespalog/src/vespa/log/control-file.h b/vespalog/src/vespa/log/control-file.h
index d0fb211e202..886f38791ba 100644
--- a/vespalog/src/vespa/log/control-file.h
+++ b/vespalog/src/vespa/log/control-file.h
@@ -17,6 +17,7 @@ public:
     enum Mode { READONLY, READWRITE, CREATE };
 private:
     Lock _fileBacking;
+    int _fileSize;
     enum Mode _mode;
     char *_fileName;
     void ensureHeader();
@@ -56,6 +57,7 @@ public:
     ~ControlFile();
     unsigned int *getLevels(const char *name);
     void ensureComponent(const char *pattern);
+    bool insideFile(char *p) { return (p >= _mapBase) && (p < _mapBase + _fileSize); }
 
     static unsigned int *defaultLevels();