withFileNode: move canary check to its own function

The do_op closure is complicated enough. Get the canary check out
of the way to not make it even more complicated.

Also, use an explicitely atomic type for the canary.
This commit is contained in:
Jakob Unterwurzacher 2017-07-22 20:53:11 +02:00
parent d2ee96d2bd
commit 9a4ea2007f
3 changed files with 26 additions and 11 deletions

View File

@ -114,6 +114,8 @@ FileNode *EncFS_Context::putNode(const char *path,
return list.front().get(); return list.front().get();
} }
// eraseNode is called by encfs_release in response to the RELEASE
// FUSE-command we get from the kernel.
void EncFS_Context::eraseNode(const char *path, FileNode *pl) { void EncFS_Context::eraseNode(const char *path, FileNode *pl) {
Lock lock(contextMutex); Lock lock(contextMutex);

View File

@ -27,6 +27,7 @@
#include <stdint.h> #include <stdint.h>
#include <string> #include <string>
#include <sys/types.h> #include <sys/types.h>
#include <atomic>
#include "CipherKey.h" #include "CipherKey.h"
#include "FSConfig.h" #include "FSConfig.h"
@ -49,7 +50,9 @@ class FileNode {
const char *cipherName); const char *cipherName);
~FileNode(); ~FileNode();
uint32_t canary; // Use an atomic type. The canary is accessed without holding any
// locks.
std::atomic<std::uint32_t> canary;
const char *plaintextName() const; const char *plaintextName() const;
const char *cipherName() const; const char *cipherName() const;

View File

@ -111,6 +111,25 @@ static int withCipherPath(const char *opName, const char *path,
return res; return res;
} }
static void checkCanary(std::shared_ptr<FileNode> fnode) {
if(fnode->canary == CANARY_OK) {
return;
}
if(fnode->canary == CANARY_RELEASED) {
// "fnode" may have been released after it was retrieved by
// lookupFuseFh. This is not an error. std::shared_ptr will release
// the memory only when all operations on the FileNode have been
// completed.
return;
}
if(fnode->canary == CANARY_DESTROYED) {
RLOG(ERROR) << "canary=CANARY_DESTROYED. FileNode accessed after it was destroyed.";
} else {
RLOG(ERROR) << "canary=0x" << std::hex << fnode->canary << ". Memory corruption?";
}
throw Error("dead canary");
}
// helper function -- apply a functor to a node // helper function -- apply a functor to a node
static int withFileNode(const char *opName, const char *path, static int withFileNode(const char *opName, const char *path,
struct fuse_file_info *fi, struct fuse_file_info *fi,
@ -125,16 +144,7 @@ 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) { checkCanary(fnode);
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