From 216dcd5d577c861c1c5b94fba942fea28799b2b2 Mon Sep 17 00:00:00 2001 From: g2flyer Date: Fri, 10 May 2024 12:01:27 -0700 Subject: [PATCH] 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); }