fixup! Fix dentry of open files after rename

Signed-off-by: g2flyer <[email protected]>
gramineproject · May 15, 2024 · 6a0f5b3 · 6a0f5b3
1 parent 18fa749
commit 6a0f5b3
Show file tree

Hide file tree

Showing 6 changed files with 42 additions and 32 deletions.
diff --git a/libos/include/libos_process.h b/libos/include/libos_process.h
@@ -62,7 +62,7 @@ struct libos_process {
     LISTP_TYPE(libos_child_process) zombies;
 
     struct libos_lock children_lock;
-    struct libos_lock fs_lock;
+    struct libos_lock fs_lock; /* Note: has lower priority than g_dcache_lock. */
 
     /* Complete command line for the process, as reported by /proc/[pid]/cmdline; currently filled
      * once during initialization and not able to be modified.

diff --git a/libos/src/fs/libos_fs_pseudo.c b/libos/src/fs/libos_fs_pseudo.c
@@ -266,11 +266,16 @@ static int pseudo_stat(struct libos_dentry* dent, struct stat* buf) {
 }
 
 static int pseudo_hstat(struct libos_handle* handle, struct stat* buf) {
-    lock(&handle->lock);
-    struct libos_dentry* dent = handle->dentry;
-    unlock(&handle->lock);
-
-    return pseudo_istat(dent, handle->inode, buf);
+    /* some callers, e.g., do_hstat in libos_stat.c, already hold lock but others do not */
+    bool hold_lock = locked(&handle->lock);
+    if (!hold_lock)
+        lock(&handle->lock);
+    int ret = -ENOENT;
+    if (handle->dentry)
+        ret = pseudo_istat(handle->dentry, handle->inode, buf);
+    if (!hold_lock)
+        unlock(&handle->lock);
+    return ret;
 }
 
 static int pseudo_unlink(struct libos_dentry* dent) {

diff --git a/libos/src/fs/proc/thread.c b/libos/src/fs/proc/thread.c
@@ -31,9 +31,9 @@ int proc_thread_follow_link(struct libos_dentry* dent, char** out_target) {
     } else if (strcmp(name, "exe") == 0) {
         lock(&g_process.exec->lock);
         dent = g_process.exec->dentry;
-        unlock(&g_process.exec->lock);
         if (dent)
             get_dentry(dent);
+        unlock(&g_process.exec->lock);
     }
 
     unlock(&g_process.fs_lock);

diff --git a/libos/src/sys/libos_fcntl.c b/libos/src/sys/libos_fcntl.c
@@ -201,31 +201,32 @@ long libos_syscall_fcntl(int fd, int cmd, unsigned long arg) {
 
             lock(&hdl->lock);
             struct libos_dentry* dent = hdl->dentry;
-            unlock(&hdl->lock);
 
             if (!dent) {
                 /* TODO: Linux allows locks on pipes etc. Our locks work only for "normal" files
                  * that have a dentry. */
                 ret = -EINVAL;
-                break;
+                goto out_setlkw_unlock;
             }
 
             if (fl->l_type == F_RDLCK && !(hdl->acc_mode & MAY_READ)) {
                 ret = -EINVAL;
-                break;
+                goto out_setlkw_unlock;
             }
 
             if (fl->l_type == F_WRLCK && !(hdl->acc_mode & MAY_WRITE)) {
                 ret = -EINVAL;
-                break;
+                goto out_setlkw_unlock;
             }
 
             struct libos_file_lock file_lock;
             ret = flock_to_file_lock(fl, hdl, &file_lock);
             if (ret < 0)
-                break;
+                goto out_setlkw_unlock;
 
             ret = file_lock_set(dent, &file_lock, /*wait=*/cmd == F_SETLKW);
+        out_setlkw_unlock:
+            unlock(&hdl->lock);
             break;
         }
 
@@ -240,11 +241,10 @@ long libos_syscall_fcntl(int fd, int cmd, unsigned long arg) {
 
             lock(&hdl->lock);
             struct libos_dentry* dent = hdl->dentry;
-            unlock(&hdl->lock);
 
             if (!dent) {
                 ret = -EINVAL;
-                break;
+                goto out_getlkw_unlock;
             }
 
             struct libos_file_lock file_lock;
@@ -254,13 +254,13 @@ long libos_syscall_fcntl(int fd, int cmd, unsigned long arg) {
 
             if (file_lock.type == F_UNLCK) {
                 ret = -EINVAL;
-                break;
+                goto out_getlkw_unlock;
             }
 
             struct libos_file_lock file_lock2;
             ret = file_lock_get(dent, &file_lock, &file_lock2);
             if (ret < 0)
-                break;
+                goto out_getlkw_unlock;
 
             fl->l_type = file_lock2.type;
             if (file_lock2.type != F_UNLCK) {
@@ -275,6 +275,8 @@ long libos_syscall_fcntl(int fd, int cmd, unsigned long arg) {
                 fl->l_pid = file_lock2.pid;
             }
             ret = 0;
+        out_getlkw_unlock:
+            unlock(&hdl->lock);
             break;
         }
 
@@ -350,12 +352,10 @@ long libos_syscall_flock(unsigned int fd, unsigned int cmd) {
         .type = lock_type,
         .handle_id = hdl->id,
     };
-    
+
     lock(&hdl->lock);
-    struct libos_dentry* dent = hdl->dentry;
+    ret = file_lock_set(hdl->dentry, &file_lock, !(cmd & LOCK_NB));
     unlock(&hdl->lock);
-
-    ret = file_lock_set(dent, &file_lock, !(cmd & LOCK_NB));
 out:
     put_handle(hdl);
     return ret;

diff --git a/libos/src/sys/libos_getcwd.c b/libos/src/sys/libos_getcwd.c
@@ -82,21 +82,26 @@ long libos_syscall_fchdir(int fd) {
     if (!hdl)
         return -EBADF;
 
-    lock(&hdl->lock);
+    int ret = 0;
+    lock(&g_dcache_lock);
+    /* Note we have to protect hdl->dentry and hdl->dentry->inode, so hdl->lock is not enough and we
+     * have to lock g_dcache_lock (which also protectes hdl->dentry). Also note that
+     * g_process.fs_lock has lower priority than g_dcache_lock, see e.g., do_path_lookupat. */
     struct libos_dentry* dent = hdl->dentry;
-    unlock(&hdl->lock);
 
     if (!dent) {
         log_debug("FD=%d has no path in the filesystem", fd);
-        return -ENOTDIR;
+        ret = -ENOTDIR;
+        goto out_unlock_dcache;
     }
     if (!dent->inode || dent->inode->type != S_IFDIR) {
         char* path = NULL;
         dentry_abs_path(dent, &path, /*size=*/NULL);
         log_debug("%s is not a directory", path);
         free(path);
         put_handle(hdl);
-        return -ENOTDIR;
+        ret = -ENOTDIR;
+        goto out_unlock_dcache;
     }
 
     lock(&g_process.fs_lock);
@@ -105,5 +110,7 @@ long libos_syscall_fchdir(int fd) {
     g_process.cwd = dent;
     unlock(&g_process.fs_lock);
     put_handle(hdl);
-    return 0;
+out_unlock_dcache:
+    unlock(&g_dcache_lock);
+    return ret;
 }
diff --git a/libos/src/sys/libos_stat.c b/libos/src/sys/libos_stat.c
@@ -39,17 +39,15 @@ static int do_hstat(struct libos_handle* hdl, struct stat* stat) {
     if (!fs || !fs->fs_ops || !fs->fs_ops->hstat)
         return -EACCES;
 
-    int ret = fs->fs_ops->hstat(hdl, stat);
-    if (ret < 0)
-        return ret;
-
-    /* Update `st_ino` from dentry */
     lock(&hdl->lock);
-    if (hdl->dentry)
+    int ret = fs->fs_ops->hstat(hdl, stat);
+    if (ret >= 0 && hdl->dentry) {
+        /* Update `st_ino` from dentry */
         stat->st_ino = dentry_ino(hdl->dentry);
+    }
     unlock(&hdl->lock);
 
-    return 0;
+    return ret;
 }
 
 long libos_syscall_stat(const char* file, struct stat* stat) {