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
https://github.com/vgough/encfs/issues/214 .
This commit is contained in:
Jakob Unterwurzacher 2017-07-08 00:49:51 +02:00 committed by rfjakob
parent 319f7b4525
commit c8ff1f94e8
4 changed files with 22 additions and 0 deletions

View File

@ -120,10 +120,13 @@ void EncFS_Context::eraseNode(const char *path, FileNode *pl) {
FileMap::iterator it = openFiles.find(std::string(path)); FileMap::iterator it = openFiles.find(std::string(path));
rAssert(it != openFiles.end()); rAssert(it != openFiles.end());
auto fn = it->second.front();
it->second.pop_front(); it->second.pop_front();
// if no more references to this file, remove the record all together // if no more references to this file, remove the record all together
if (it->second.empty()) { if (it->second.empty()) {
fn->canary = CANARY_RELEASED;
openFiles.erase(it); openFiles.erase(it);
} }
} }

View File

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

View File

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

View File

@ -125,6 +125,16 @@ static int withFileNode(const char *opName, const char *path,
auto do_op = [&FSRoot, opName, &op](FileNode *fnode) { auto do_op = [&FSRoot, opName, &op](FileNode *fnode) {
rAssert(fnode != nullptr); 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(); VLOG(1) << "op: " << opName << " : " << fnode->cipherName();
// check that we're not recursing into the mount point itself // check that we're not recursing into the mount point itself