fixup! Single-process-lifetime rollback protection for protected file…

…s (WIP)

Signed-off-by: g2flyer <[email protected]>

libos/include/libos_fs_encrypted.h

@@ -190,7 +190,7 @@ int encrypted_file_get(struct libos_encrypted_file* enc);
  *
  * This decreases `use_count`, and closes the file if it reaches 0.
  */
-void encrypted_file_put(struct libos_encrypted_file* enc);
+void encrypted_file_put(struct libos_encrypted_file* enc, bool fs_reachable);
 
 /*
  * \brief Flush pending writes to an encrypted file.

libos/src/fs/chroot/encrypted.c

@@ -200,7 +200,7 @@ static int chroot_encrypted_lookup(struct libos_dentry* dent) {
             }
         } else {
             ret = encrypted_file_get_size(enc, &size);
-            encrypted_file_put(enc);
+            encrypted_file_put(enc, true);
 
             if (ret < 0) {
                 encrypted_file_destroy(enc);
@@ -390,7 +390,7 @@ static int chroot_encrypted_rename(struct libos_dentry* old, struct libos_dentry
         goto out;
 
     ret = encrypted_file_rename(enc, new_uri);
-    encrypted_file_put(enc);
+    encrypted_file_put(enc, true);
 out:
     unlock(&old->inode->lock);
     free(new_uri);
@@ -476,7 +476,7 @@ static int chroot_encrypted_close(struct libos_handle* hdl) {
     assert(enc);
 
     lock(&hdl->inode->lock);
-    encrypted_file_put(enc);
+    encrypted_file_put(enc, hdl->inode == hdl->dentry->inode);
     unlock(&hdl->inode->lock);
 
     return 0;

libos/src/fs/libos_fs_encrypted.c

@@ -284,12 +284,13 @@ int parse_pf_key(const char* key_str, pf_key_t* pf_key) {
     return 0;
 }
 
-static void encrypted_file_internal_close(struct libos_encrypted_file* enc) {
+static void encrypted_file_internal_close(struct libos_encrypted_file* enc, bool fs_reachable) {
     assert(enc->pf);
 
     pf_mac_t closing_root_gmac;
     pf_status_t pfs = pf_close(enc->pf, &closing_root_gmac);
-    log_debug("file '%s' closed with MAC=" MAC_PRINTF_PATTERN, enc->norm_path,
+    log_debug("%sreachable file '%s' closed with MAC=" MAC_PRINTF_PATTERN,
+              (fs_reachable ? "" : "un"), enc->norm_path,
               MAC_PRINTF_ARGS(closing_root_gmac));  // TODO (MST): remove me eventually?
     lock(&(enc->volume->files_state_map_lock));
     struct libos_encrypted_volume_state_map* file_state = NULL;
@@ -300,9 +301,7 @@ static void encrypted_file_internal_close(struct libos_encrypted_file* enc) {
         file_state->state = PF_FILE_ERROR;
         pf_set_corrupted(enc->pf);
     } else {
-        // TODO (MST): Below also has to rule out that our file is stale, i.e., somebody has renamed
-        // a file to our own original file name
-        if (file_state->state != PF_FILE_DELETED) {
+        if (file_state->state != PF_FILE_DELETED && fs_reachable) {
             // TODO (MST): omit below if read-only file?
             memcpy(file_state->last_seen_root_gmac, closing_root_gmac, sizeof(pf_mac_t));
             file_state->state = PF_FILE_CLOSED;
@@ -617,12 +616,12 @@ int encrypted_file_get(struct libos_encrypted_file* enc) {
     return 0;
 }
 
-void encrypted_file_put(struct libos_encrypted_file* enc) {
+void encrypted_file_put(struct libos_encrypted_file* enc, bool fs_reachable) {
     assert(enc->use_count > 0);
     assert(enc->pf);
     enc->use_count--;
     if (enc->use_count == 0) {
-        encrypted_file_internal_close(enc);
+        encrypted_file_internal_close(enc, fs_reachable);
     }
 }
 

libos/test/regression/rename_unlink.c

@@ -156,6 +156,7 @@ static void test_rename_replace(const char* path1, const char* path2) {
         err(1, "rename");
 
     should_not_exist(path1);
+
     should_exist(path2, message1_len);
 
     /* We expect `fd` to still point to old data, even though we replaced the file under its path */
@@ -177,6 +178,48 @@ static void test_rename_replace(const char* path1, const char* path2) {
         err(1, "unlink %s", path2);
 }
 
+static void test_rename_follow(const char* path1, const char* path2) {
+    printf("%s...\n", __func__);
+
+    int fd = create_file(path1, message1, message1_len);
+
+    if (fd < 0)
+        err(1, "open %s", path1);
+
+    if (rename(path1, path2) != 0)
+        err(1, "rename");
+
+    should_not_exist(path1);
+    should_exist(path2, message1_len);
+
+    if (lseek(fd, 0, SEEK_SET) != 0)
+        err(1, "lseek");
+
+    ssize_t n = posix_fd_write(fd, message2, message2_len);
+    if (n < 0)
+        errx(1, "posix_fd_write failed");
+    if ((size_t)n != message2_len)
+        errx(1, "wrote less bytes than expected");
+
+    should_contain("file opened before it's renamed", fd, message2, message2_len);
+
+    if (close(fd) != 0)
+        err(1, "close %s", path2);
+
+    fd = open(path2, O_RDONLY, 0);
+    if (fd < 0)
+        err(1, "open %s", path2);
+
+    /* We expect `fd` to point to new data, even though we changed data via old fd after rename */
+    should_contain("file opened after it's renamed", fd, message2, message2_len);
+
+    if (close(fd) != 0)
+        err(1, "close %s", path2);
+
+    if (unlink(path2) != 0)
+        err(1, "unlink %s", path2);
+}
+
 static void test_rename_open_file(const char* path1, const char* path2) {
     printf("%s...\n", __func__);
 
@@ -271,6 +314,7 @@ int main(int argc, char* argv[]) {
     test_rename_same_file(path1);
     test_simple_rename(path1, path2);
     test_rename_replace(path1, path2);
+    test_rename_follow(path1, path2);
     test_rename_open_file(path1, path2);
     test_unlink_and_recreate(path1);
     test_unlink_and_write(path1);