Skip to content

Commit

Permalink
fixup! Fix dentry of open files after rename
Browse files Browse the repository at this point in the history
Signed-off-by: g2flyer <[email protected]>
  • Loading branch information
g2flyer committed Jun 12, 2024
1 parent 3f15945 commit 216dcd5
Show file tree
Hide file tree
Showing 9 changed files with 45 additions and 12 deletions.
4 changes: 2 additions & 2 deletions libos/include/libos_handle.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;

/*
Expand Down
6 changes: 5 additions & 1 deletion libos/src/fs/libos_fs_pseudo.c
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
8 changes: 8 additions & 0 deletions libos/src/fs/proc/thread.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down Expand Up @@ -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("-");
Expand Down Expand Up @@ -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 {
Expand All @@ -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);

Expand Down Expand Up @@ -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();

Expand Down
2 changes: 2 additions & 0 deletions libos/src/libos_rtld.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
23 changes: 18 additions & 5 deletions libos/src/sys/libos_fcntl.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
}

Expand All @@ -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;
}
Expand All @@ -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;

Expand Down Expand Up @@ -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;
Expand Down
3 changes: 1 addition & 2 deletions libos/src/sys/libos_file.c
Original file line number Diff line number Diff line change
Expand Up @@ -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)) {
Expand Down
2 changes: 2 additions & 0 deletions libos/src/sys/libos_getcwd.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
5 changes: 3 additions & 2 deletions libos/src/sys/libos_open.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down Expand Up @@ -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);
Expand Down
4 changes: 4 additions & 0 deletions libos/src/sys/libos_stat.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down Expand Up @@ -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);
}
Expand Down

0 comments on commit 216dcd5

Please sign in to comment.