Add a canary value to FileNode

Adds a uint32 value to FileNode that is initialized to an arbitrary value (CANARY_OK) in the constructor, and reset when the reference is dropped (CANARY_RELEASED, CANARY_DESTROYED). The canary is checked on each withFileNode call. Makes it much easier to trigger the bug seen in #214 .
vgough · Jul 19, 2017 · c8ff1f9 · c8ff1f9 · benrubson · Jul 20, 2017
1 parent 319f7b4
commit c8ff1f9
Show file tree

Hide file tree

Showing 4 changed files with 22 additions and 0 deletions.
diff --git a/encfs/Context.cpp b/encfs/Context.cpp
@@ -120,10 +120,13 @@ void EncFS_Context::eraseNode(const char *path, FileNode *pl) {
   FileMap::iterator it = openFiles.find(std::string(path));
   rAssert(it != openFiles.end());
 
+  auto fn = it->second.front();
+
   it->second.pop_front();
 
   // if no more references to this file, remove the record all together
   if (it->second.empty()) {
+    fn->canary = CANARY_RELEASED;
     openFiles.erase(it);
   }
 }

diff --git a/encfs/FileNode.cpp b/encfs/FileNode.cpp
@@ -58,6 +58,8 @@ FileNode::FileNode(DirNode *parent_, const FSConfigPtr &cfg,
 
   Lock _lock(mutex);
 
+  this->canary = CANARY_OK;
+
   this->_pname = plaintextName_;
   this->_cname = cipherName_;
   this->parent = parent_;
@@ -76,6 +78,7 @@ FileNode::~FileNode() {
   // FileNode mutex should be locked before the destructor is called
   // pthread_mutex_lock( &mutex );
 
+  canary = CANARY_DESTROYED;
   _pname.assign(_pname.length(), '\0');
   _cname.assign(_cname.length(), '\0');
   io.reset();

diff --git a/encfs/FileNode.h b/encfs/FileNode.h
@@ -33,6 +33,10 @@
 #include "FileUtils.h"
 #include "encfs.h"
 
+#define CANARY_OK 0x46040975
+#define CANARY_RELEASED 0x70c5610d
+#define CANARY_DESTROYED 0x52cdad90
+
 namespace encfs {
 
 class Cipher;
@@ -45,6 +49,8 @@ class FileNode {
            const char *cipherName);
   ~FileNode();
 
+  uint32_t canary;
+
   const char *plaintextName() const;
   const char *cipherName() const;
 

diff --git a/encfs/encfs.cpp b/encfs/encfs.cpp
@@ -125,6 +125,16 @@ static int withFileNode(const char *opName, const char *path,
 
     auto do_op = [&FSRoot, opName, &op](FileNode *fnode) {
       rAssert(fnode != nullptr);
+      if(fnode->canary != CANARY_OK) {
+        if(fnode->canary == CANARY_RELEASED) {
+          RLOG(ERROR) << "canary=CANARY_RELEASED. File node accessed after it was released.";
+        } else if(fnode->canary == CANARY_DESTROYED) {
+          RLOG(ERROR) << "canary=CANARY_DESTROYED. File node accessed after it was destroyed.";
+        } else {
+          RLOG(ERROR) << "canary=0x" << std::hex << fnode->canary << ". Corruption?";
+        }
+        throw Error("dead canary");
+      }
       VLOG(1) << "op: " << opName << " : " << fnode->cipherName();
 
       // check that we're not recursing into the mount point itself