amend! Fix dentry of open files after rename

[LibOS] Fix dentry of open files after rename

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

libos/src/sys/libos_file.c

@@ -358,24 +358,28 @@ static int do_rename(struct libos_dentry* old_dent, struct libos_dentry* new_den
 
     new_dent->inode = old_dent->inode;
     old_dent->inode = NULL;
-    // also update dentry of any potentially open fd pointing to old_dent
+
+    /* also update dentry of any potentially open fd pointing to old_dent */
     struct libos_handle_map* handle_map = get_thread_handle_map(NULL);
-    rwlock_write_lock(&handle_map->lock);
+    assert(handle_libos_call != NULL);
+    rwlock_read_lock(&handle_map->lock);
 
     for (uint32_t i = 0; handle_map->fd_top != FD_NULL && i <= handle_map->fd_top; i++) {
         struct libos_fd_handle* fd_handle = handle_map->map[i];
         if (!HANDLE_ALLOCATED(fd_handle))
             continue;
         struct libos_handle* handle = fd_handle->handle;
-        if ((handle->dentry != old_dent) || (handle->inode != new_dent->inode))
-            continue;
+        /* TODO (MST): systematically go through all for other uses of handle->dentry as they also
+         * would require locking */
         lock(&handle->lock);
-        handle->dentry = new_dent;
-        put_dentry(old_dent);
-        get_dentry(new_dent);
+        if ((handle->dentry == old_dent) && (handle->inode == new_dent->inode)) {
+            handle->dentry = new_dent;
+            put_dentry(old_dent);
+            get_dentry(new_dent);
+        }
         unlock(&handle->lock);
     }
-    rwlock_write_unlock(&handle_map->lock);
+    rwlock_read_unlock(&handle_map->lock);
     return 0;
 }
 

libos/test/regression/rename_unlink.c

@@ -1,6 +1,7 @@
 /* SPDX-License-Identifier: LGPL-3.0-or-later */
-/* Copyright (C) 2021 Intel Corporation
+/* Copyright (C) 2024 Intel Corporation
  *                    Paweł Marczewski <[email protected]>
+ *                    Michael Steiner <[email protected]>
  */
 
 /*
@@ -156,7 +157,6 @@ 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 */
@@ -182,9 +182,8 @@ 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);
+        err(1, "create %s", path1);
 
     if (rename(path1, path2) != 0)
         err(1, "rename");
@@ -220,17 +219,20 @@ static void test_rename_follow(const char* path1, const char* path2) {
         err(1, "unlink %s", path2);
 }
 
-// NOTE: below will _not_ run correctly when directly executed unless you run as root.
-// But it should run properly in gramine when executed as normal user.
+/* NOTE: below will _not_ run correctly when directly executed unless you run as root.
+ * But it should run properly in Gramine when executed as normal user. */
 static void test_rename_fchown_fchmod(const char* path1, const char* path2) {
     printf("%s...\n", __func__);
 
     int fd = create_file(path1, message1, message1_len);
+    if (fd < 0)
+        err(1, "create %s", path1);
 
-    if (fchown(fd, 1, 1))
+    if (fchown(fd, /*owner=*/1, /*group=*/1) != 0) /* dummy owner/group just for testing */
         err(1, "fchown before rename");
-    if (fchmod(fd, S_IRWXU | S_IRWXG) != 0)  // Note: no other!
+    if (fchmod(fd, S_IRWXU | S_IRWXG) != 0) /* note: no "other users" mode bits */
         err(1, "fchmod before rename");
+
     struct stat st;
     if (stat(path1, &st) != 0)
         err(1, "Failed to stat file %s", path1);
@@ -239,19 +241,17 @@ static void test_rename_fchown_fchmod(const char* path1, const char* path2) {
     if (st.st_mode & S_IRWXO)
         err(1, "wrong permissions of file %s", path1);
 
-    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 (fchown(fd, 2, 2))
+    if (fchown(fd, /*owner=*/2, /*group=*/2) != 0) /* different dummy owner/group */
         err(1, "fchown after rename");
-    if (fchmod(fd, S_IRWXU | S_IRWXG | S_IRWXO) != 0)  // Note: with other now!
+    if (fchmod(fd, S_IRWXU | S_IRWXG | S_IRWXO) != 0) /* note: now with "other users" mode bits */
         err(1, "fchmod after rename");
+
     if (stat(path2, &st) != 0)
         err(1, "Failed to stat (renamed) file %s", path2);
     if (st.st_uid != 2 || st.st_gid != 2)