diff --git a/src/cryfs/filesystem/fsblobstore/DirBlob.cpp b/src/cryfs/filesystem/fsblobstore/DirBlob.cpp index 53b134c4..d2edc5bd 100644 --- a/src/cryfs/filesystem/fsblobstore/DirBlob.cpp +++ b/src/cryfs/filesystem/fsblobstore/DirBlob.cpp @@ -29,18 +29,18 @@ namespace fsblobstore { constexpr off_t DirBlob::DIR_LSTAT_SIZE; DirBlob::DirBlob(FsBlobStore *fsBlobStore, unique_ref blob, std::function getLstatSize) : - FsBlob(std::move(blob)), _fsBlobStore(fsBlobStore), _getLstatSize(getLstatSize), _entries(), _mutex(), _changed(false) { + FsBlob(std::move(blob)), _fsBlobStore(fsBlobStore), _getLstatSize(getLstatSize), _getLstatSizeMutex(), _entries(), _entriesAndChangedMutex(), _changed(false) { ASSERT(baseBlob().blobType() == FsBlobView::BlobType::DIR, "Loaded blob is not a directory"); _readEntriesFromBlob(); } DirBlob::~DirBlob() { - std::unique_lock lock(_mutex); + std::unique_lock lock(_entriesAndChangedMutex); _writeEntriesToBlob(); } void DirBlob::flush() { - std::unique_lock lock(_mutex); + std::unique_lock lock(_entriesAndChangedMutex); _writeEntriesToBlob(); baseBlob().flush(); } @@ -66,17 +66,17 @@ void DirBlob::_readEntriesFromBlob() { } void DirBlob::AddChildDir(const std::string &name, const Key &blobKey, mode_t mode, uid_t uid, gid_t gid, timespec lastAccessTime, timespec lastModificationTime) { - std::unique_lock lock(_mutex); + std::unique_lock lock(_entriesAndChangedMutex); _addChild(name, blobKey, fspp::Dir::EntryType::DIR, mode, uid, gid, lastAccessTime, lastModificationTime); } void DirBlob::AddChildFile(const std::string &name, const Key &blobKey, mode_t mode, uid_t uid, gid_t gid, timespec lastAccessTime, timespec lastModificationTime) { - std::unique_lock lock(_mutex); + std::unique_lock lock(_entriesAndChangedMutex); _addChild(name, blobKey, fspp::Dir::EntryType::FILE, mode, uid, gid, lastAccessTime, lastModificationTime); } void DirBlob::AddChildSymlink(const std::string &name, const blockstore::Key &blobKey, uid_t uid, gid_t gid, timespec lastAccessTime, timespec lastModificationTime) { - std::unique_lock lock(_mutex); + std::unique_lock lock(_entriesAndChangedMutex); _addChild(name, blobKey, fspp::Dir::EntryType::SYMLINK, S_IFLNK | S_IRUSR | S_IWUSR | S_IXUSR | S_IRGRP | S_IWGRP | S_IXGRP | S_IROTH | S_IWOTH | S_IXOTH, uid, gid, lastAccessTime, lastModificationTime); } @@ -89,41 +89,41 @@ void DirBlob::_addChild(const std::string &name, const Key &blobKey, void DirBlob::AddOrOverwriteChild(const std::string &name, const Key &blobKey, fspp::Dir::EntryType entryType, mode_t mode, uid_t uid, gid_t gid, timespec lastAccessTime, timespec lastModificationTime, std::function onOverwritten) { - std::unique_lock lock(_mutex); + std::unique_lock lock(_entriesAndChangedMutex); _entries.addOrOverwrite(name, blobKey, entryType, mode, uid, gid, lastAccessTime, lastModificationTime, onOverwritten); _changed = true; } void DirBlob::RenameChild(const blockstore::Key &key, const std::string &newName, std::function onOverwritten) { - std::unique_lock lock(_mutex); + std::unique_lock lock(_entriesAndChangedMutex); _entries.rename(key, newName, onOverwritten); _changed = true; } boost::optional DirBlob::GetChild(const string &name) const { - std::unique_lock lock(_mutex); + std::unique_lock lock(_entriesAndChangedMutex); return _entries.get(name); } boost::optional DirBlob::GetChild(const Key &key) const { - std::unique_lock lock(_mutex); + std::unique_lock lock(_entriesAndChangedMutex); return _entries.get(key); } void DirBlob::RemoveChild(const string &name) { - std::unique_lock lock(_mutex); + std::unique_lock lock(_entriesAndChangedMutex); _entries.remove(name); _changed = true; } void DirBlob::RemoveChild(const Key &key) { - std::unique_lock lock(_mutex); + std::unique_lock lock(_entriesAndChangedMutex); _entries.remove(key); _changed = true; } void DirBlob::AppendChildrenTo(vector *result) const { - std::unique_lock lock(_mutex); + std::unique_lock lock(_entriesAndChangedMutex); result->reserve(result->size() + _entries.size()); for (const auto &entry : _entries) { result->emplace_back(entry.type(), entry.name()); @@ -135,7 +135,17 @@ off_t DirBlob::lstat_size() const { } void DirBlob::statChild(const Key &key, struct ::stat *result) const { - result->st_size = _getLstatSize(key); + std::unique_lock lock(_getLstatSizeMutex); + auto lstatSizeGetter = _getLstatSize; + + // The following unlock is important to avoid deadlock. + // ParallelAccessFsBlobStore::load() causes a call to DirBlob::setLstatSizeGetter, + // so their lock ordering first locks the ParallelAccessStore::_mutex, then the DirBlob::_getLstatSizeMutex. + // this requires us to free DirBlob::_getLstatSizeMutex before calling into lstatSizeGetter(), because + // lstatSizeGetter can call ParallelAccessFsBlobStore::load(). + lock.unlock(); + + result->st_size = lstatSizeGetter(key); statChildWithSizeAlreadySet(key, result); } @@ -159,43 +169,43 @@ void DirBlob::statChildWithSizeAlreadySet(const Key &key, struct ::stat *result) } void DirBlob::updateAccessTimestampForChild(const Key &key) { - std::unique_lock lock(_mutex); + std::unique_lock lock(_entriesAndChangedMutex); _entries.updateAccessTimestampForChild(key); _changed = true; } void DirBlob::updateModificationTimestampForChild(const Key &key) { - std::unique_lock lock(_mutex); + std::unique_lock lock(_entriesAndChangedMutex); _entries.updateModificationTimestampForChild(key); _changed = true; } void DirBlob::chmodChild(const Key &key, mode_t mode) { - std::unique_lock lock(_mutex); + std::unique_lock lock(_entriesAndChangedMutex); _entries.setMode(key, mode); _changed = true; } void DirBlob::chownChild(const Key &key, uid_t uid, gid_t gid) { - std::unique_lock lock(_mutex); + std::unique_lock lock(_entriesAndChangedMutex); if(_entries.setUidGid(key, uid, gid)) { _changed = true; } } void DirBlob::utimensChild(const Key &key, timespec lastAccessTime, timespec lastModificationTime) { - std::unique_lock lock(_mutex); + std::unique_lock lock(_entriesAndChangedMutex); _entries.setAccessTimes(key, lastAccessTime, lastModificationTime); _changed = true; } void DirBlob::setLstatSizeGetter(std::function getLstatSize) { - std::unique_lock lock(_mutex); + std::unique_lock lock(_getLstatSizeMutex); _getLstatSize = getLstatSize; } cpputils::unique_ref DirBlob::releaseBaseBlob() { - std::unique_lock lock(_mutex); + std::unique_lock lock(_entriesAndChangedMutex); _writeEntriesToBlob(); return FsBlob::releaseBaseBlob(); } diff --git a/src/cryfs/filesystem/fsblobstore/DirBlob.h b/src/cryfs/filesystem/fsblobstore/DirBlob.h index 4b4e71af..3202e41e 100644 --- a/src/cryfs/filesystem/fsblobstore/DirBlob.h +++ b/src/cryfs/filesystem/fsblobstore/DirBlob.h @@ -82,8 +82,9 @@ namespace cryfs { FsBlobStore *_fsBlobStore; std::function _getLstatSize; + mutable std::mutex _getLstatSizeMutex; DirEntryList _entries; - mutable std::mutex _mutex; + mutable std::mutex _entriesAndChangedMutex; bool _changed; DISALLOW_COPY_AND_ASSIGN(DirBlob);