Fix a bug where deleting a directory could leave over some blocks.

Details: Before, we allowed removing non-empty directories. Seems 'rm -rf' is trying to do that. Now, we return the correct error code ENOTEMPTY in this case, which causes that 'rm -rf' deletes the entries first.

ChangeLog.txt

@@ -1,7 +1,8 @@
 Version 0.9.1 (unreleased)
 ---------------
-* Report file system usage statistics to the operating system (e.g. amount of space used). This information can be queried using the 'df' tool on linux.
-* Use stronger scrypt parameters when generating the config file key from the user password. This makes it a bit more secure, but also takes a bit longer to load a file system.
+* Report file system usage statistics to the operating system (e.g. amount of space used). This information can be queried using the 'df' tool on linux. See https://github.com/cryfs/cryfs/commit/68acc27e88ff5209ca55ddb4e91f5a449d77fb54
+* Use stronger scrypt parameters when generating the config file key from the user password. This makes it a bit more secure, but also takes a bit longer to load a file system. See https://github.com/cryfs/cryfs/commit/7f1493ab9210319cab008e71d4ee8f4d7d920f39
+* Fix a bug where deleting a non-empty directory could leave some blocks over.
 
 Version 0.9.0
 ---------------

src/cryfs/filesystem/CryDir.cpp

@@ -79,4 +79,16 @@ void CryDir::createSymlink(const string &name, const bf::path &target, uid_t uid
   blob->AddChildSymlink(name, child->key(), uid, gid);
 }
 
+void CryDir::remove() {
+  device()->callFsActionCallbacks();
+  {
+    auto blob = LoadBlob();
+    if (0 != blob->NumChildren()) {
+      throw FuseErrnoException(ENOTEMPTY);
+    }
+  }
+  //TODO removeNode() calls CryDevice::RemoveBlob, which loads the blob again. So we're loading it twice. Should be optimized.
+  removeNode();
+}
+
 }

src/cryfs/filesystem/CryDir.h

@@ -23,6 +23,8 @@ class CryDir final: public fspp::Dir, CryNode {
 
   fspp::Dir::EntryType getType() const override;
 
+  void remove() override;
+
 private:
   cpputils::unique_ref<parallelaccessfsblobstore::DirBlobRef> LoadBlob() const;
 

src/cryfs/filesystem/CryFile.cpp

@@ -51,4 +51,9 @@ fspp::Dir::EntryType CryFile::getType() const {
   return fspp::Dir::EntryType::FILE;
 }
 
+void CryFile::remove() {
+  device()->callFsActionCallbacks();
+  removeNode();
+}
+
 }

src/cryfs/filesystem/CryFile.h

@@ -17,6 +17,7 @@ class CryFile final: public fspp::File, CryNode {
   cpputils::unique_ref<fspp::OpenFile> open(int flags) const override;
   void truncate(off_t size) const override;
   fspp::Dir::EntryType getType() const override;
+  void remove() override;
 
 private:
   cpputils::unique_ref<parallelaccessfsblobstore::FileBlobRef> LoadBlob() const;

src/cryfs/filesystem/CryNode.cpp

@@ -74,8 +74,7 @@ void CryNode::utimens(timespec lastAccessTime, timespec lastModificationTime) {
   (*_parent)->utimensChild(_key, lastAccessTime, lastModificationTime);
 }
 
-void CryNode::remove() {
-  device()->callFsActionCallbacks();
+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 root direcory.

src/cryfs/filesystem/CryNode.h

@@ -19,7 +19,6 @@ class CryNode: public virtual fspp::Node {
   void chown(uid_t uid, gid_t gid) override;
   void rename(const boost::filesystem::path &to) override;
   void utimens(timespec lastAccessTime, timespec lastModificationTime) override;
-  void remove() override;
 
 protected:
   CryNode();
@@ -31,6 +30,8 @@ class CryNode: public virtual fspp::Node {
 
   virtual fspp::Dir::EntryType getType() const = 0;
 
+  void removeNode();
+
 private:
   CryDevice *_device;
   boost::optional<cpputils::unique_ref<parallelaccessfsblobstore::DirBlobRef>> _parent;

src/cryfs/filesystem/CrySymlink.cpp

@@ -50,4 +50,9 @@ bf::path CrySymlink::target() const {
   return blob->target();
 }
 
+void CrySymlink::remove() {
+  device()->callFsActionCallbacks();
+  removeNode();
+}
+
 }

src/cryfs/filesystem/CrySymlink.h

@@ -18,6 +18,8 @@ class CrySymlink final: public fspp::Symlink, CryNode {
 
   fspp::Dir::EntryType getType() const override;
 
+  void remove() override;
+
 private:
   cpputils::unique_ref<parallelaccessfsblobstore::SymlinkBlobRef> LoadBlob() const;
 

src/cryfs/filesystem/cachingfsblobstore/DirBlobRef.h

@@ -26,6 +26,10 @@ class DirBlobRef final: public FsBlobRef {
         return _base->GetChild(key);
     }
 
+    size_t NumChildren() const {
+        return _base->NumChildren();
+    }
+
     void RemoveChild(const blockstore::Key &key) {
         return _base->RemoveChild(key);
     }

src/cryfs/filesystem/fsblobstore/DirBlob.cpp

@@ -169,5 +169,9 @@ cpputils::unique_ref<blobstore::Blob> DirBlob::releaseBaseBlob() {
   return FsBlob::releaseBaseBlob();
 }
 
+size_t DirBlob::NumChildren() const {
+  return _entries.size();
+}
+
 }
 }

src/cryfs/filesystem/fsblobstore/DirBlob.h

@@ -28,6 +28,9 @@ namespace cryfs {
 
             void AppendChildrenTo(std::vector<fspp::Dir::Entry> *result) const;
 
+            //TODO Test NumChildren()
+            size_t NumChildren() const;
+
             boost::optional<const DirEntry&> GetChild(const std::string &name) const;
 
             boost::optional<const DirEntry&> GetChild(const blockstore::Key &key) const;

src/cryfs/filesystem/parallelaccessfsblobstore/DirBlobRef.h

@@ -22,6 +22,10 @@ class DirBlobRef final: public FsBlobRef {
         return _base->GetChild(key);
     }
 
+    size_t NumChildren() const {
+        return _base->NumChildren();
+    }
+
     void RemoveChild(const blockstore::Key &key) {
         return _base->RemoveChild(key);
     }