diff --git a/libos/include/libos_handle.h b/libos/include/libos_handle.h index 4ce281f4e0..938a8e309b 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, 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 */ struct libos_dentry* dentry; /* 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/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_fs_pseudo.c b/libos/src/fs/libos_fs_pseudo.c index eab8129e0f..d551d8e042 100644 --- a/libos/src/fs/libos_fs_pseudo.c +++ b/libos/src/fs/libos_fs_pseudo.c @@ -266,7 +266,16 @@ 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); + /* 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/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/fs/proc/thread.c b/libos/src/fs/proc/thread.c index c3da147c48..b22828fbb9 100644 --- a/libos/src/fs/proc/thread.c +++ b/libos/src/fs/proc/thread.c @@ -29,9 +29,11 @@ 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; if (dent) get_dentry(dent); + unlock(&g_process.exec->lock); } unlock(&g_process.fs_lock); @@ -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..0fa6b7b297 100644 --- a/libos/src/sys/libos_fcntl.c +++ b/libos/src/sys/libos_fcntl.c @@ -199,29 +199,34 @@ long libos_syscall_fcntl(int fd, int cmd, unsigned long arg) { break; } - if (!hdl->dentry) { + lock(&hdl->lock); + struct libos_dentry* dent = hdl->dentry; + + 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(hdl->dentry, &file_lock, /*wait=*/cmd == F_SETLKW); + ret = file_lock_set(dent, &file_lock, /*wait=*/cmd == F_SETLKW); + out_setlkw_unlock: + unlock(&hdl->lock); break; } @@ -234,9 +239,12 @@ long libos_syscall_fcntl(int fd, int cmd, unsigned long arg) { break; } - if (!hdl->dentry) { + lock(&hdl->lock); + struct libos_dentry* dent = hdl->dentry; + + if (!dent) { ret = -EINVAL; - break; + goto out_getlkw_unlock; } struct libos_file_lock file_lock; @@ -246,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(hdl->dentry, &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) { @@ -267,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; } @@ -342,7 +352,10 @@ long libos_syscall_flock(unsigned int fd, unsigned int cmd) { .type = lock_type, .handle_id = hdl->id, }; + + lock(&hdl->lock); ret = file_lock_set(hdl->dentry, &file_lock, !(cmd & LOCK_NB)); + unlock(&hdl->lock); out: put_handle(hdl); return ret; diff --git a/libos/src/sys/libos_file.c b/libos/src/sys/libos_file.c index f4050813d3..70c6a8f0a6 100644 --- a/libos/src/sys/libos_file.c +++ b/libos/src/sys/libos_file.c @@ -347,8 +347,31 @@ 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); + 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++) { + struct libos_fd_handle* fd_handle = handle_map->map[i]; + if (!HANDLE_ALLOCATED(fd_handle)) + continue; + struct libos_handle* handle = fd_handle->handle; + /* 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)) { + handle->dentry = new_dent; + put_dentry(old_dent); + get_dentry(new_dent); + } + unlock(&handle->lock); + } + rwlock_read_unlock(&handle_map->lock); return 0; } diff --git a/libos/src/sys/libos_getcwd.c b/libos/src/sys/libos_getcwd.c index 2cd64fca7f..5ef309f6d2 100644 --- a/libos/src/sys/libos_getcwd.c +++ b/libos/src/sys/libos_getcwd.c @@ -82,11 +82,17 @@ long libos_syscall_fchdir(int fd) { if (!hdl) return -EBADF; + 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; 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; @@ -94,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); @@ -103,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_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..190d4cc09d 100644 --- a/libos/src/sys/libos_stat.c +++ b/libos/src/sys/libos_stat.c @@ -39,15 +39,15 @@ static int do_hstat(struct libos_handle* hdl, struct stat* stat) { if (!fs || !fs->fs_ops || !fs->fs_ops->hstat) return -EACCES; + lock(&hdl->lock); int ret = fs->fs_ops->hstat(hdl, stat); - if (ret < 0) - return ret; - - /* Update `st_ino` from dentry */ - if (hdl->dentry) + 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) { @@ -210,7 +210,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); }