Fix fstat (a bug in the fstat implementation caused problems with some text editors (e.g. nano) falsely thinking a file changed since they opened it).

This commit is contained in:
Sebastian Messmer 2016-03-16 16:57:46 +00:00
parent e4ff7e17f1
commit 0cdfb8ba2e
12 changed files with 47 additions and 21 deletions

View File

@ -6,6 +6,7 @@ Version 0.9.3 (unreleased)
* You can disable the automatic update check by setting CRYFS_NO_UPDATE_CHECK=true in your environment.
* Building CryFS from the GitHub tarball (i.e. when there is no .git directory present) works.
* The ciphertext block size is configurable. You can use the "--blocksize-bytes" command line argument. If not specified, CryFS will ask you for a block size when creating a file system.
* Fix fstat (a bug in the fstat implementation caused problems with some text editors (e.g. nano) falsely thinking a file changed since they opened it).
Version 0.9.2
---------------

View File

@ -41,8 +41,9 @@ unique_ref<fspp::OpenFile> CryDir::createAndOpenFile(const string &name, mode_t
device()->callFsActionCallbacks();
auto child = device()->CreateFileBlob();
auto now = fsblobstore::time::now();
LoadBlob()->AddChildFile(name, child->key(), mode, uid, gid, now, now);
return make_unique_ref<CryOpenFile>(device(), std::move(child));
auto dirBlob = LoadBlob();
dirBlob->AddChildFile(name, child->key(), mode, uid, gid, now, now);
return make_unique_ref<CryOpenFile>(device(), cpputils::to_unique_ptr(std::move(dirBlob)), std::move(child));
}
void CryDir::createDir(const string &name, mode_t mode, uid_t uid, gid_t gid) {

View File

@ -37,7 +37,7 @@ unique_ref<parallelaccessfsblobstore::FileBlobRef> CryFile::LoadBlob() const {
unique_ref<fspp::OpenFile> CryFile::open(int flags) const {
device()->callFsActionCallbacks();
auto blob = LoadBlob();
return make_unique_ref<CryOpenFile>(device(), std::move(blob));
return make_unique_ref<CryOpenFile>(device(), parent(), std::move(blob));
}
void CryFile::truncate(off_t size) const {

View File

@ -17,6 +17,7 @@ using cpputils::dynamic_pointer_move;
using cpputils::unique_ref;
using boost::optional;
using boost::none;
using std::shared_ptr;
using cryfs::parallelaccessfsblobstore::FsBlobRef;
using cryfs::parallelaccessfsblobstore::DirBlobRef;
@ -28,8 +29,11 @@ namespace cryfs {
CryNode::CryNode(CryDevice *device, optional<unique_ref<DirBlobRef>> parent, const Key &key)
: _device(device),
_parent(std::move(parent)),
_parent(none),
_key(key) {
if (parent != none) {
_parent = cpputils::to_unique_ptr(std::move(*parent));
}
}
CryNode::~CryNode() {
@ -42,10 +46,15 @@ void CryNode::access(int mask) const {
throw FuseErrnoException(ENOTSUP);
}
shared_ptr<const DirBlobRef> CryNode::parent() const {
ASSERT(_parent != none, "We are the root directory and can't get the parent of the root directory");
return *_parent;
}
void CryNode::rename(const bf::path &to) {
device()->callFsActionCallbacks();
if (_parent == none) {
//We are the base direcory.
//We are the root direcory.
//TODO What should we do?
throw FuseErrnoException(EIO);
}
@ -70,7 +79,7 @@ void CryNode::rename(const bf::path &to) {
void CryNode::utimens(timespec lastAccessTime, timespec lastModificationTime) {
device()->callFsActionCallbacks();
if (_parent == none) {
//We are the base direcory.
//We are the root direcory.
//TODO What should we do?
throw FuseErrnoException(EIO);
}
@ -80,7 +89,7 @@ void CryNode::utimens(timespec lastAccessTime, timespec lastModificationTime) {
void CryNode::removeNode() {
//TODO Instead of all these if-else and having _parent being an optional, we could also introduce a CryRootDir which inherits from fspp::Dir.
if (_parent == none) {
//We are the base direcory.
//We are the root direcory.
//TODO What should we do?
throw FuseErrnoException(EIO);
}
@ -103,7 +112,7 @@ unique_ref<FsBlobRef> CryNode::LoadBlob() const {
void CryNode::stat(struct ::stat *result) const {
device()->callFsActionCallbacks();
if(_parent == none) {
//We are the base directory.
//We are the root directory.
//TODO What should we do?
result->st_uid = getuid();
result->st_gid = getgid();
@ -130,7 +139,7 @@ void CryNode::stat(struct ::stat *result) const {
void CryNode::chmod(mode_t mode) {
device()->callFsActionCallbacks();
if (_parent == none) {
//We are the base direcory.
//We are the root direcory.
//TODO What should we do?
throw FuseErrnoException(EIO);
}
@ -140,7 +149,7 @@ void CryNode::chmod(mode_t mode) {
void CryNode::chown(uid_t uid, gid_t gid) {
device()->callFsActionCallbacks();
if (_parent == none) {
//We are the base direcory.
//We are the root direcory.
//TODO What should we do?
throw FuseErrnoException(EIO);
}

View File

@ -27,6 +27,7 @@ protected:
CryDevice *device();
const CryDevice *device() const;
cpputils::unique_ref<parallelaccessfsblobstore::FsBlobRef> LoadBlob() const;
std::shared_ptr<const parallelaccessfsblobstore::DirBlobRef> parent() const;
virtual fspp::Dir::EntryType getType() const = 0;
@ -34,7 +35,7 @@ protected:
private:
CryDevice *_device;
boost::optional<cpputils::unique_ref<parallelaccessfsblobstore::DirBlobRef>> _parent;
boost::optional<std::shared_ptr<parallelaccessfsblobstore::DirBlobRef>> _parent;
blockstore::Key _key;
DISALLOW_COPY_AND_ASSIGN(CryNode);

View File

@ -8,8 +8,10 @@
namespace bf = boost::filesystem;
using std::shared_ptr;
using cpputils::unique_ref;
using cryfs::parallelaccessfsblobstore::FileBlobRef;
using cryfs::parallelaccessfsblobstore::DirBlobRef;
//TODO Get rid of this in favor of a exception hierarchy
using fspp::fuse::CHECK_RETVAL;
@ -17,8 +19,8 @@ using fspp::fuse::FuseErrnoException;
namespace cryfs {
CryOpenFile::CryOpenFile(const CryDevice *device, unique_ref<FileBlobRef> fileBlob)
: _device(device), _fileBlob(std::move(fileBlob)) {
CryOpenFile::CryOpenFile(const CryDevice *device, shared_ptr<const DirBlobRef> parent, unique_ref<FileBlobRef> fileBlob)
: _device(device), _parent(parent), _fileBlob(std::move(fileBlob)) {
}
CryOpenFile::~CryOpenFile() {
@ -32,9 +34,8 @@ void CryOpenFile::flush() {
void CryOpenFile::stat(struct ::stat *result) const {
_device->callFsActionCallbacks();
result->st_mode = S_IFREG | S_IRUSR | S_IXUSR | S_IWUSR;
_parent->statChildExceptSize(_fileBlob->key(), result);
result->st_size = _fileBlob->size();
return;
}
void CryOpenFile::truncate(off_t size) const {

View File

@ -4,13 +4,14 @@
#include <fspp/fs_interface/OpenFile.h>
#include "parallelaccessfsblobstore/FileBlobRef.h"
#include "parallelaccessfsblobstore/DirBlobRef.h"
namespace cryfs {
class CryDevice;
class CryOpenFile final: public fspp::OpenFile {
public:
explicit CryOpenFile(const CryDevice *device, cpputils::unique_ref<parallelaccessfsblobstore::FileBlobRef> fileBlob);
explicit CryOpenFile(const CryDevice *device, std::shared_ptr<const parallelaccessfsblobstore::DirBlobRef> parent, cpputils::unique_ref<parallelaccessfsblobstore::FileBlobRef> fileBlob);
~CryOpenFile();
void stat(struct ::stat *result) const override;
@ -23,6 +24,7 @@ public:
private:
const CryDevice *_device;
std::shared_ptr<const parallelaccessfsblobstore::DirBlobRef> _parent;
cpputils::unique_ref<parallelaccessfsblobstore::FileBlobRef> _fileBlob;
DISALLOW_COPY_AND_ASSIGN(CryOpenFile);

View File

@ -47,6 +47,10 @@ public:
return _base->statChild(key, result);
}
void statChildExceptSize(const blockstore::Key &key, struct ::stat *result) const {
return _base->statChildExceptSize(key, result);
}
void chmodChild(const blockstore::Key &key, mode_t mode) {
return _base->chmodChild(key, mode);
}

View File

@ -112,13 +112,16 @@ off_t DirBlob::lstat_size() const {
}
void DirBlob::statChild(const Key &key, struct ::stat *result) const {
statChildExceptSize(key, result);
result->st_size = _getLstatSize(key);
}
void DirBlob::statChildExceptSize(const Key &key, struct ::stat *result) const {
auto childOpt = GetChild(key);
if (childOpt == boost::none) {
throw fspp::fuse::FuseErrnoException(ENOENT);
}
const auto &child = *childOpt;
//TODO Loading the blob for only getting the size of the file/symlink is not very performant.
// Furthermore, this is the only reason why DirBlob needs a pointer to _fsBlobStore, which is ugly
result->st_mode = child.mode();
result->st_uid = child.uid();
result->st_gid = child.gid();
@ -133,7 +136,6 @@ void DirBlob::statChild(const Key &key, struct ::stat *result) const {
result->st_mtim = child.lastModificationTime();
result->st_ctim = child.lastMetadataChangeTime();
#endif
result->st_size = _getLstatSize(key);
//TODO Move ceilDivision to general utils which can be used by cryfs as well
result->st_blocks = blobstore::onblocks::utils::ceilDivision(result->st_size, (off_t)512);
result->st_blksize = _fsBlobStore->blocksizeBytes();

View File

@ -52,6 +52,8 @@ namespace cryfs {
void statChild(const blockstore::Key &key, struct ::stat *result) const;
void statChildExceptSize(const blockstore::Key &key, struct ::stat *result) const;
void chmodChild(const blockstore::Key &key, mode_t mode);
void chownChild(const blockstore::Key &key, uid_t uid, gid_t gid);

View File

@ -43,6 +43,10 @@ public:
return _base->statChild(key, result);
}
void statChildExceptSize(const blockstore::Key &key, struct ::stat *result) const {
return _base->statChildExceptSize(key, result);
}
void chmodChild(const blockstore::Key &key, mode_t mode) {
return _base->chmodChild(key, mode);
}

View File

@ -25,8 +25,7 @@ public:
file.stat(&st1);
callback(st1);
file.open(O_RDONLY)->stat(&st2);
//TODO Enable this line
//callback(st2);
callback(st2);
}
void EXPECT_SIZE(uint64_t expectedSize, const fspp::File &file) {