From 6e4be125c7342da34f0ef85f5aa73b52270a9371 Mon Sep 17 00:00:00 2001 From: g2flyer Date: Mon, 6 May 2024 15:23:34 -0700 Subject: [PATCH 1/6] Fix dentry of open files after rename Signed-off-by: g2flyer --- libos/src/sys/libos_file.c | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/libos/src/sys/libos_file.c b/libos/src/sys/libos_file.c index f4050813d3..e0f84fd908 100644 --- a/libos/src/sys/libos_file.c +++ b/libos/src/sys/libos_file.c @@ -347,8 +347,27 @@ static int do_rename(struct libos_dentry* old_dent, struct libos_dentry* new_den if (new_dent->inode) put_inode(new_dent->inode); + new_dent->inode = old_dent->inode; old_dent->inode = NULL; + // 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); + + 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; + lock(&handle->lock); + handle->dentry = new_dent; + put_dentry(old_dent); + get_dentry(new_dent); + unlock(&handle->lock); + } + rwlock_write_unlock(&handle_map->lock); return 0; } From 7d5317d83094520c6877ce0c67b4792663b38ecf Mon Sep 17 00:00:00 2001 From: g2flyer Date: Tue, 7 May 2024 12:01:03 -0700 Subject: [PATCH 2/6] amend! Fix dentry of open files after rename [LibOS] Fix dentry of open files after rename Signed-off-by: g2flyer --- libos/src/sys/libos_file.c | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/libos/src/sys/libos_file.c b/libos/src/sys/libos_file.c index e0f84fd908..b12375093b 100644 --- a/libos/src/sys/libos_file.c +++ b/libos/src/sys/libos_file.c @@ -350,24 +350,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; } From 70307d433e3ed54901d0b3d1bdcec5f69aec6b39 Mon Sep 17 00:00:00 2001 From: g2flyer Date: Wed, 8 May 2024 10:44:56 -0700 Subject: [PATCH 3/6] fixup! Fix dentry of open files after rename Signed-off-by: g2flyer --- libos/src/sys/libos_file.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libos/src/sys/libos_file.c b/libos/src/sys/libos_file.c index b12375093b..e7881be5c7 100644 --- a/libos/src/sys/libos_file.c +++ b/libos/src/sys/libos_file.c @@ -353,7 +353,7 @@ static int do_rename(struct libos_dentry* old_dent, struct libos_dentry* new_den /* also update dentry of any potentially open fd pointing to old_dent */ struct libos_handle_map* handle_map = get_thread_handle_map(NULL); - assert(handle_libos_call != NULL); + assert(handle_map != NULL); rwlock_read_lock(&handle_map->lock); for (uint32_t i = 0; handle_map->fd_top != FD_NULL && i <= handle_map->fd_top; i++) { From 3f159450bce974a6ef9d247243041909b3d424d7 Mon Sep 17 00:00:00 2001 From: g2flyer Date: Wed, 8 May 2024 18:23:20 -0700 Subject: [PATCH 4/6] fixup! Fix dentry of open files after rename Signed-off-by: g2flyer --- libos/include/libos_handle.h | 3 +++ libos/src/bookkeep/libos_handle.c | 20 ++++++++++++++------ libos/src/fs/libos_namei.c | 2 ++ libos/src/sys/libos_file.c | 1 + 4 files changed, 20 insertions(+), 6 deletions(-) diff --git a/libos/include/libos_handle.h b/libos/include/libos_handle.h index 4ce281f4e0..2dedb77d4c 100644 --- a/libos/include/libos_handle.h +++ b/libos/include/libos_handle.h @@ -165,6 +165,9 @@ struct libos_handle { refcount_t ref_count; struct libos_fs* fs; + /* dentry can change due to rename, so to secure read-access, aquire g_dcache_lock and/or + * handle->lock; to protect updates (unless during creation and deletion where unique use is + * guaranteed), acquire first g_dcache_lock and then handle-Lock */ struct libos_dentry* dentry; /* diff --git a/libos/src/bookkeep/libos_handle.c b/libos/src/bookkeep/libos_handle.c index daa7ee8b2d..10b52fa9eb 100644 --- a/libos/src/bookkeep/libos_handle.c +++ b/libos/src/bookkeep/libos_handle.c @@ -312,7 +312,9 @@ static int clear_posix_locks(struct libos_handle* handle) { .end = FS_LOCK_EOF, .pid = g_process.pid, }; + lock(&handle->lock); int ret = file_lock_set(handle->dentry, &file_lock, /*block=*/false); + unlock(&handle->lock); if (ret < 0) { log_warning("error releasing locks: %s", unix_strerror(ret)); return ret; @@ -477,13 +479,17 @@ int set_new_fd_handle_above_fd(uint32_t fd, struct libos_handle* hdl, int fd_fla } static inline __attribute__((unused)) const char* __handle_name(struct libos_handle* hdl) { + /* This function seems unused, so probably could be dropped? */ + const char* ret = "(unknown)"; + lock(&hdl->lock); if (hdl->uri) - return hdl->uri; + ret = hdl->uri; if (hdl->dentry && hdl->dentry->name[0] != '\0') - return hdl->dentry->name; + ret = hdl->dentry->name; if (hdl->fs) - return hdl->fs->name; - return "(unknown)"; + ret = hdl->fs->name; + unlock(&hdl->lock); + return ret; } void get_handle(struct libos_handle* hdl) { @@ -499,6 +505,8 @@ static void destroy_handle(struct libos_handle* hdl) { static int clear_flock_locks(struct libos_handle* hdl) { /* Clear flock (BSD) locks for a file. We are required to do that when the handle is closed. */ + int ret = 0; + lock(&hdl->lock); if (hdl && hdl->dentry && hdl->created_by_process) { assert(hdl->ref_count == 0); struct libos_file_lock file_lock = { @@ -509,10 +517,10 @@ static int clear_flock_locks(struct libos_handle* hdl) { int ret = file_lock_set(hdl->dentry, &file_lock, /*block=*/false); if (ret < 0) { log_warning("error releasing locks: %s", unix_strerror(ret)); - return ret; } } - return 0; + unlock(&hdl->lock); + return ret; } void put_handle(struct libos_handle* hdl) { diff --git a/libos/src/fs/libos_namei.c b/libos/src/fs/libos_namei.c index f23f2de8da..54898fd642 100644 --- a/libos/src/fs/libos_namei.c +++ b/libos/src/fs/libos_namei.c @@ -741,8 +741,10 @@ int get_dirfd_dentry(int dirfd, struct libos_dentry** dir) { return -ENOTDIR; } + lock(&hdl->lock); /* while hdl->is_dir is immutable, hdl->dentry can change due to rename */ get_dentry(hdl->dentry); *dir = hdl->dentry; + unlock(&hdl->lock); put_handle(hdl); return 0; } diff --git a/libos/src/sys/libos_file.c b/libos/src/sys/libos_file.c index e7881be5c7..f89c7c043f 100644 --- a/libos/src/sys/libos_file.c +++ b/libos/src/sys/libos_file.c @@ -363,6 +363,7 @@ static int do_rename(struct libos_dentry* old_dent, struct libos_dentry* new_den struct libos_handle* handle = fd_handle->handle; /* TODO (MST): systematically go through all for other uses of handle->dentry as they also * would require locking */ + assert(locked(&g_dcache_lock)); lock(&handle->lock); if ((handle->dentry == old_dent) && (handle->inode == new_dent->inode)) { handle->dentry = new_dent; From 216dcd5d577c861c1c5b94fba942fea28799b2b2 Mon Sep 17 00:00:00 2001 From: g2flyer Date: Fri, 10 May 2024 12:01:27 -0700 Subject: [PATCH 5/6] fixup! Fix dentry of open files after rename Signed-off-by: g2flyer --- libos/include/libos_handle.h | 4 ++-- libos/src/fs/libos_fs_pseudo.c | 6 +++++- libos/src/fs/proc/thread.c | 8 ++++++++ libos/src/libos_rtld.c | 2 ++ libos/src/sys/libos_fcntl.c | 23 ++++++++++++++++++----- libos/src/sys/libos_file.c | 3 +-- libos/src/sys/libos_getcwd.c | 2 ++ libos/src/sys/libos_open.c | 5 +++-- libos/src/sys/libos_stat.c | 4 ++++ 9 files changed, 45 insertions(+), 12 deletions(-) diff --git a/libos/include/libos_handle.h b/libos/include/libos_handle.h index 2dedb77d4c..938a8e309b 100644 --- a/libos/include/libos_handle.h +++ b/libos/include/libos_handle.h @@ -165,9 +165,9 @@ struct libos_handle { refcount_t ref_count; struct libos_fs* fs; - /* dentry can change due to rename, so to secure read-access, aquire g_dcache_lock and/or + /* dentry can change due to rename, so to secure read-access, acquire g_dcache_lock and/or * handle->lock; to protect updates (unless during creation and deletion where unique use is - * guaranteed), acquire first g_dcache_lock and then handle-Lock */ + * guaranteed), acquire first g_dcache_lock and then handle->lock */ struct libos_dentry* dentry; /* diff --git a/libos/src/fs/libos_fs_pseudo.c b/libos/src/fs/libos_fs_pseudo.c index eab8129e0f..e6ccec8d4c 100644 --- a/libos/src/fs/libos_fs_pseudo.c +++ b/libos/src/fs/libos_fs_pseudo.c @@ -266,7 +266,11 @@ static int pseudo_stat(struct libos_dentry* dent, struct stat* buf) { } static int pseudo_hstat(struct libos_handle* handle, struct stat* buf) { - return pseudo_istat(handle->dentry, handle->inode, buf); + lock(&handle->lock); + struct libos_dentry* dent = handle->dentry; + unlock(&handle->lock); + + return pseudo_istat(dent, handle->inode, buf); } 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 c3da147c48..dd2bc8c581 100644 --- a/libos/src/fs/proc/thread.c +++ b/libos/src/fs/proc/thread.c @@ -29,7 +29,9 @@ int proc_thread_follow_link(struct libos_dentry* dent, char** out_target) { dent = g_process.cwd; get_dentry(dent); } 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); } @@ -91,11 +93,13 @@ int proc_thread_maps_load(struct libos_dentry* dent, char** out_data, size_t* ou retry_emit_vma: if (vma->file) { int dev_major = 0, dev_minor = 0; + lock(&vma->file->lock); unsigned long ino = vma->file->dentry ? dentry_ino(vma->file->dentry) : 0; char* path = NULL; if (vma->file->dentry) dentry_abs_path(vma->file->dentry, &path, /*size=*/NULL); + unlock(&vma->file->lock); EMIT(ADDR_FMT(start), start); EMIT("-"); @@ -310,6 +314,7 @@ int proc_thread_fd_follow_link(struct libos_dentry* dent, char** out_target) { int ret; struct libos_handle* hdl = handle_map->map[fd]->handle; + lock(&hdl->lock); if (hdl->dentry) { ret = dentry_abs_path(hdl->dentry, out_target, /*size=*/NULL); } else { @@ -318,6 +323,7 @@ int proc_thread_fd_follow_link(struct libos_dentry* dent, char** out_target) { *out_target = describe_handle(hdl); ret = *out_target ? 0 : -ENOMEM; } + unlock(&hdl->lock); rwlock_read_unlock(&handle_map->lock); @@ -457,9 +463,11 @@ int proc_thread_stat_load(struct libos_dentry* dent, char** out_data, size_t* ou char comm[16] = {0}; lock(&g_process.fs_lock); + lock(&g_process.exec->lock); size_t name_length = g_process.exec->dentry->name_len; memcpy(comm, g_process.exec->dentry->name, name_length > sizeof(comm) - 1 ? sizeof(comm) - 1 : name_length); + unlock(&g_process.exec->lock); unlock(&g_process.fs_lock); size_t virtual_mem_size = get_total_memory_usage(); diff --git a/libos/src/libos_rtld.c b/libos/src/libos_rtld.c index 5115d44ce5..a4f405c78e 100644 --- a/libos/src/libos_rtld.c +++ b/libos/src/libos_rtld.c @@ -589,10 +589,12 @@ static int load_and_check_shebang(struct libos_handle* file, const char* pathnam ret = read_partial_fragment(file, shebang, sizeof(shebang), /*offset=*/0, &shebang_len); if (ret < 0 || shebang_len < 2 || shebang[0] != '#' || shebang[1] != '!') { char* path = NULL; + lock(&file->lock); if (file->dentry) { /* this may fail, but we are already inside a more serious error handler */ dentry_abs_path(file->dentry, &path, /*size=*/NULL); } + unlock(&file->lock); log_debug("Failed to read shebang line from %s", path ? path : "(unknown)"); free(path); return -ENOEXEC; diff --git a/libos/src/sys/libos_fcntl.c b/libos/src/sys/libos_fcntl.c index 24578155e2..ffa0725aa5 100644 --- a/libos/src/sys/libos_fcntl.c +++ b/libos/src/sys/libos_fcntl.c @@ -199,7 +199,11 @@ long libos_syscall_fcntl(int fd, int cmd, unsigned long arg) { break; } - if (!hdl->dentry) { + 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; @@ -221,7 +225,7 @@ long libos_syscall_fcntl(int fd, int cmd, unsigned long arg) { if (ret < 0) break; - ret = file_lock_set(hdl->dentry, &file_lock, /*wait=*/cmd == F_SETLKW); + ret = file_lock_set(dent, &file_lock, /*wait=*/cmd == F_SETLKW); break; } @@ -234,7 +238,11 @@ long libos_syscall_fcntl(int fd, int cmd, unsigned long arg) { break; } - if (!hdl->dentry) { + lock(&hdl->lock); + struct libos_dentry* dent = hdl->dentry; + unlock(&hdl->lock); + + if (!dent) { ret = -EINVAL; break; } @@ -250,7 +258,7 @@ long libos_syscall_fcntl(int fd, int cmd, unsigned long arg) { } struct libos_file_lock file_lock2; - ret = file_lock_get(hdl->dentry, &file_lock, &file_lock2); + ret = file_lock_get(dent, &file_lock, &file_lock2); if (ret < 0) break; @@ -342,7 +350,12 @@ long libos_syscall_flock(unsigned int fd, unsigned int cmd) { .type = lock_type, .handle_id = hdl->id, }; - ret = file_lock_set(hdl->dentry, &file_lock, !(cmd & LOCK_NB)); + + lock(&hdl->lock); + struct libos_dentry* dent = hdl->dentry; + 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_file.c b/libos/src/sys/libos_file.c index f89c7c043f..70c6a8f0a6 100644 --- a/libos/src/sys/libos_file.c +++ b/libos/src/sys/libos_file.c @@ -361,8 +361,7 @@ static int do_rename(struct libos_dentry* old_dent, struct libos_dentry* new_den if (!HANDLE_ALLOCATED(fd_handle)) continue; struct libos_handle* handle = fd_handle->handle; - /* TODO (MST): systematically go through all for other uses of handle->dentry as they also - * would require locking */ + /* see comment in libos_handle.h on loocking strategy protecting handle->lock */ assert(locked(&g_dcache_lock)); lock(&handle->lock); if ((handle->dentry == old_dent) && (handle->inode == new_dent->inode)) { diff --git a/libos/src/sys/libos_getcwd.c b/libos/src/sys/libos_getcwd.c index 2cd64fca7f..de5ef5cf02 100644 --- a/libos/src/sys/libos_getcwd.c +++ b/libos/src/sys/libos_getcwd.c @@ -82,7 +82,9 @@ long libos_syscall_fchdir(int fd) { if (!hdl) return -EBADF; + lock(&hdl->lock); struct libos_dentry* dent = hdl->dentry; + unlock(&hdl->lock); if (!dent) { log_debug("FD=%d has no path in the filesystem", fd); diff --git a/libos/src/sys/libos_open.c b/libos/src/sys/libos_open.c index 83fde52c47..ce3afad908 100644 --- a/libos/src/sys/libos_open.c +++ b/libos/src/sys/libos_open.c @@ -373,12 +373,12 @@ static ssize_t do_getdents(int fd, uint8_t* buf, size_t buf_size, bool is_getden goto out_no_unlock; } + lock(&g_dcache_lock); if (!hdl->dentry->inode) { ret = -ENOENT; - goto out_no_unlock; + goto out_unlock_only_dcache_lock; } - lock(&g_dcache_lock); maybe_lock_pos_handle(hdl); lock(&hdl->lock); @@ -467,6 +467,7 @@ static ssize_t do_getdents(int fd, uint8_t* buf, size_t buf_size, bool is_getden out: unlock(&hdl->lock); maybe_unlock_pos_handle(hdl); +out_unlock_only_dcache_lock: unlock(&g_dcache_lock); out_no_unlock: put_handle(hdl); diff --git a/libos/src/sys/libos_stat.c b/libos/src/sys/libos_stat.c index a73d6c0063..c32d2cec13 100644 --- a/libos/src/sys/libos_stat.c +++ b/libos/src/sys/libos_stat.c @@ -44,8 +44,10 @@ static int do_hstat(struct libos_handle* hdl, struct stat* stat) { return ret; /* Update `st_ino` from dentry */ + lock(&hdl->lock); if (hdl->dentry) stat->st_ino = dentry_ino(hdl->dentry); + unlock(&hdl->lock); return 0; } @@ -210,7 +212,9 @@ long libos_syscall_fstatfs(int fd, struct statfs* buf) { if (!hdl) return -EBADF; + lock(&hdl->lock); struct libos_mount* mount = hdl->dentry ? hdl->dentry->mount : NULL; + unlock(&hdl->lock); put_handle(hdl); return __do_statfs(mount, buf); } From b980834edb351d5a8005d05c544f4973a46a9d67 Mon Sep 17 00:00:00 2001 From: g2flyer Date: Tue, 14 May 2024 15:13:27 -0700 Subject: [PATCH 6/6] 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) {