From 6a0f5b32d7e28b05e4a70f63a4cbfa59abc77987 Mon Sep 17 00:00:00 2001 From: g2flyer Date: Tue, 14 May 2024 15:13:27 -0700 Subject: [PATCH] fixup! Fix dentry of open files after rename Signed-off-by: g2flyer --- libos/include/libos_process.h | 2 +- libos/src/fs/libos_fs_pseudo.c | 15 ++++++++++----- libos/src/fs/proc/thread.c | 2 +- libos/src/sys/libos_fcntl.c | 26 +++++++++++++------------- libos/src/sys/libos_getcwd.c | 17 ++++++++++++----- libos/src/sys/libos_stat.c | 12 +++++------- 6 files changed, 42 insertions(+), 32 deletions(-) diff --git a/libos/include/libos_process.h b/libos/include/libos_process.h index 6478d00370..e2637d484d 100644 --- 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 index e6ccec8d4c..d551d8e042 100644 --- 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 index dd2bc8c581..b22828fbb9 100644 --- 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 index ffa0725aa5..0fa6b7b297 100644 --- 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 index de5ef5cf02..5ef309f6d2 100644 --- a/libos/src/sys/libos_getcwd.c +++ b/libos/src/sys/libos_getcwd.c @@ -82,13 +82,17 @@ 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; @@ -96,7 +100,8 @@ long libos_syscall_fchdir(int fd) { 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 index c32d2cec13..190d4cc09d 100644 --- 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) {