Make RAMFS not to free file data when file is still opened

Even though it is valid to delete a file its data (i-node) should not be deleted until all file descriptors are closed. This patch fixes the bug in file deletion logic in RAMFS to make sure that file node does not get deleted until all file descriptors are closed. Fixes #1035 Signed-off-by: Waldemar Kozaczuk <[email protected]>
cloudius-systems · Jun 24, 2019 · 2e45b33 · 2e45b33
1 parent 1e22a86
commit 2e45b33
Show file tree

Hide file tree

Showing 3 changed files with 68 additions and 3 deletions.
diff --git a/fs/ramfs/ramfs.h b/fs/ramfs/ramfs.h
@@ -59,6 +59,8 @@ struct ramfs_node {
     struct timespec rn_mtime;
     int rn_mode;
     bool rn_owns_buf;
+    int rn_ref_count;
+    bool rn_removed;
 };
 
 struct ramfs_node *ramfs_allocate_node(const char *name, int type);

diff --git a/fs/ramfs/ramfs_vnops.cc b/fs/ramfs/ramfs_vnops.cc
@@ -95,13 +95,19 @@ ramfs_allocate_node(const char *name, int type)
 
     set_times_to_now(&(np->rn_ctime), &(np->rn_atime), &(np->rn_mtime));
     np->rn_owns_buf = true;
+    np->rn_ref_count = 0;
+    np->rn_removed = false;
 
     return np;
 }
 
 void
 ramfs_free_node(struct ramfs_node *np)
 {
+    if (!np->rn_removed || np->rn_ref_count > 0) {
+	    return;
+    }
+
     if (np->rn_buf != NULL && np->rn_owns_buf)
         free(np->rn_buf);
 
@@ -159,7 +165,11 @@ ramfs_remove_node(struct ramfs_node *dnp, struct ramfs_node *np)
         }
         prev->rn_next = np->rn_next;
     }
-    ramfs_free_node(np);
+
+    np->rn_removed = true;
+    if (np->rn_ref_count <= 0) {
+        ramfs_free_node(np);
+    }
 
     set_times_to_now(&(dnp->rn_mtime), &(dnp->rn_ctime));
 
@@ -522,6 +532,9 @@ ramfs_rename(struct vnode *dvp1, struct vnode *vp1, char *name1,
             np->rn_buf = old_np->rn_buf;
             np->rn_size = old_np->rn_size;
             np->rn_bufsize = old_np->rn_bufsize;
+            np->rn_owns_buf = old_np->rn_owns_buf;
+            np->rn_ref_count = old_np->rn_ref_count;
+            np->rn_removed = old_np->rn_removed;
             old_np->rn_buf = NULL;
         }
         /* Remove source file */
@@ -629,8 +642,27 @@ ramfs_setattr(struct vnode *vnode, struct vattr *attr) {
     return 0;
 }
 
-#define ramfs_open      ((vnop_open_t)vop_nullop)
-#define ramfs_close     ((vnop_close_t)vop_nullop)
+int ramfs_open(struct file *fp)
+{
+    struct vnode *vp = file_dentry(fp)->d_vnode;
+    struct ramfs_node *np = (ramfs_node *) vp->v_data;
+    np->rn_ref_count++;
+    return 0;
+}
+
+int ramfs_close(struct vnode *dvp, struct file *file)
+{
+    struct vnode *vp = file_dentry(file)->d_vnode;
+    struct ramfs_node *np = (ramfs_node *) vp->v_data;
+    np->rn_ref_count--;
+
+    if (np->rn_removed && np->rn_ref_count <= 0) {
+        ramfs_free_node(np);
+    }
+
+    return 0;
+}
+
 #define ramfs_seek      ((vnop_seek_t)vop_nullop)
 #define ramfs_ioctl     ((vnop_ioctl_t)vop_einval)
 #define ramfs_fsync     ((vnop_fsync_t)vop_nullop)

diff --git a/tests/tst-mmap-file.cc b/tests/tst-mmap-file.cc
@@ -14,6 +14,7 @@
 #include <fcntl.h>
 #include <stdio.h>
 #include <errno.h>
+#include <assert.h>
 
 static int tests = 0, fails = 0;
 
@@ -81,6 +82,34 @@ static int check_mapping(void *addr, size_t size, unsigned flags, int fd,
     return 0;
 }
 
+static int test_mmap_with_file_removed()
+{
+    char file_template[30];
+    snprintf(file_template, sizeof(file_template) / sizeof(char), "%s/mmap-file-deleted.XXXXXX", "/tmp");
+
+    auto fd1 = mkstemp(file_template);
+    assert(fd1 != -1);
+
+    assert(unlink(file_template) >= 0);
+    size_t buf_size = 131072;
+    assert(ftruncate(fd1, buf_size) >= 0);
+
+    auto buf = (char *) mmap(NULL, buf_size, PROT_READ|PROT_WRITE, MAP_PRIVATE, fd1, 0);
+    assert(buf != MAP_FAILED);
+
+    auto fd2 = open(file_template, O_RDWR);
+    close(fd2);
+
+    auto urandom = fopen("/dev/urandom", "rb");
+    assert(urandom != NULL);
+    assert(fread(buf, 1, buf_size, urandom) == buf_size);
+
+    close(fd1);
+    report(munmap(buf,buf_size) == 0, "test_mmap_with_file_removed succeeded");
+
+    return 0;
+}
+
 int main(int argc, char *argv[])
 {
     auto fd = open("/tmp/mmap-file-test", O_CREAT|O_TRUNC|O_RDWR, 0666);
@@ -143,6 +172,8 @@ int main(int argc, char *argv[])
     report(munmap(b, 4096) == 0, "munmap temporary mapping");
     report(close(fd) == 0, "close again");
 
+    test_mmap_with_file_removed();
+
     // TODO: map an append-only file with prot asking for PROT_WRITE, mmap should return EACCES.
     // TODO: map a file under a fs mounted with the flag NO_EXEC and prot asked for PROT_EXEC (expect EPERM).