Skip to content

Commit

Permalink
[LibOS] Fix ENOENT error in fchmod on unlinked file
Browse files Browse the repository at this point in the history
Signed-off-by: Sonali Saha <sonali.saha@intel.com>
  • Loading branch information
sahason authored and dimakuv committed Sep 20, 2023
1 parent f0047d1 commit ac8fc77
Show file tree
Hide file tree
Showing 6 changed files with 73 additions and 18 deletions.
12 changes: 12 additions & 0 deletions libos/include/libos_fs.h
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,18 @@ struct libos_fs_ops {
/* checkpoint/migrate the file system */
ssize_t (*checkpoint)(void** checkpoint, void* mount_data);
int (*migrate)(void* checkpoint, void** mount_data);

/*
* \brief Change file permissions.
*
* \param hdl File handle.
* \param perm New permissions for the file.
*
* Changes the permissions of a file associated with a given file handle.
*
* On success, the caller should update `hdl->inode->perm`.
*/
int (*fchmod)(struct libos_handle* hdl, mode_t perm);
};

/* Limit for the number of dentry children. This is mostly to prevent overflow if (untrusted) host
Expand Down
14 changes: 14 additions & 0 deletions libos/src/fs/chroot/encrypted.c
Original file line number Diff line number Diff line change
Expand Up @@ -368,6 +368,19 @@ static int chroot_encrypted_chmod(struct libos_dentry* dent, mode_t perm) {
return ret;
}

static int chroot_encrypted_fchmod(struct libos_handle* hdl, mode_t perm) {
assert(hdl->inode);

struct libos_encrypted_file* enc = hdl->inode->data;
mode_t host_perm = HOST_PERM(perm);
PAL_STREAM_ATTR attr = {.share_flags = host_perm};
int ret = PalStreamAttributesSetByHandle(enc->pal_handle, &attr);
if (ret < 0)
return pal_to_unix_errno(ret);

return 0;
}

static int chroot_encrypted_flush(struct libos_handle* hdl) {
assert(hdl->type == TYPE_CHROOT_ENCRYPTED);
if (hdl->inode->type != S_IFREG)
Expand Down Expand Up @@ -484,6 +497,7 @@ struct libos_fs_ops chroot_encrypted_fs_ops = {
.migrate = &chroot_encrypted_migrate,
.mmap = &generic_emulated_mmap,
.msync = &generic_emulated_msync,
.fchmod = &chroot_encrypted_fchmod,
};

struct libos_d_ops chroot_encrypted_d_ops = {
Expand Down
13 changes: 13 additions & 0 deletions libos/src/fs/chroot/fs.c
Original file line number Diff line number Diff line change
Expand Up @@ -467,6 +467,18 @@ static int chroot_chmod(struct libos_dentry* dent, mode_t perm) {
return 0;
}

static int chroot_fchmod(struct libos_handle* hdl, mode_t perm) {
int ret;

mode_t host_perm = HOST_PERM(perm);
PAL_STREAM_ATTR attr = {.share_flags = host_perm};
ret = PalStreamAttributesSetByHandle(hdl->pal_handle, &attr);
if (ret < 0)
return pal_to_unix_errno(ret);

return 0;
}

struct libos_fs_ops chroot_fs_ops = {
.mount = &chroot_mount,
.flush = &chroot_flush,
Expand All @@ -480,6 +492,7 @@ struct libos_fs_ops chroot_fs_ops = {
.hstat = &generic_inode_hstat,
.truncate = &generic_truncate,
.poll = &generic_inode_poll,
.fchmod = &chroot_fchmod,
};

struct libos_d_ops chroot_d_ops = {
Expand Down
7 changes: 7 additions & 0 deletions libos/src/fs/tmpfs/fs.c
Original file line number Diff line number Diff line change
Expand Up @@ -210,6 +210,12 @@ static int tmpfs_chmod(struct libos_dentry* dent, mode_t perm) {
return 0;
}

static int tmpfs_fchmod(struct libos_handle* hdl, mode_t perm) {
__UNUSED(hdl);
__UNUSED(perm);
return 0;
}

static ssize_t tmpfs_read(struct libos_handle* hdl, void* buf, size_t size, file_off_t* pos) {
ssize_t ret;

Expand Down Expand Up @@ -301,6 +307,7 @@ struct libos_fs_ops tmp_fs_ops = {
.poll = &generic_inode_poll,
.mmap = &generic_emulated_mmap,
.msync = &generic_emulated_msync,
.fchmod = &tmpfs_fchmod,
};

struct libos_d_ops tmp_d_ops = {
Expand Down
25 changes: 7 additions & 18 deletions libos/src/sys/libos_file.c
Original file line number Diff line number Diff line change
Expand Up @@ -212,35 +212,24 @@ long libos_syscall_fchmod(int fd, mode_t mode) {
/* This isn't documented, but that's what Linux does. */
mode_t perm = mode & 07777;

struct libos_dentry* dent = hdl->dentry;
int ret = 0;

lock(&g_dcache_lock);
if (!dent) {
ret = -EINVAL;
goto out;
}

if (!dent->inode) {
/* TODO: the `chmod` callback should take a handle, not dentry; otherwise we're not able to
* chmod an unlinked file */
if (!hdl->inode) {
ret = -ENOENT;
goto out;
}

struct libos_fs* fs = dent->inode->fs;
if (fs && fs->d_ops && fs->d_ops->chmod) {
ret = fs->d_ops->chmod(dent, perm);
struct libos_fs* fs = hdl->inode->fs;
if (fs && fs->fs_ops && fs->fs_ops->fchmod) {
ret = fs->fs_ops->fchmod(hdl, perm);
if (ret < 0)
goto out;
}

lock(&dent->inode->lock);
dent->inode->perm = perm;
unlock(&dent->inode->lock);
lock(&hdl->inode->lock);
hdl->inode->perm = perm;
unlock(&hdl->inode->lock);

out:
unlock(&g_dcache_lock);
put_handle(hdl);
return ret;
}
Expand Down
20 changes: 20 additions & 0 deletions libos/test/regression/rename_unlink.c
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@
* Tests for renaming and deleting files. Mostly focus on cases where a file is still open.
*/

#define _DEFAULT_SOURCE /* fchmod */

#include <assert.h>
#include <err.h>
#include <errno.h>
Expand Down Expand Up @@ -224,6 +226,23 @@ static void test_unlink_and_write(const char* path) {
err(1, "close unlinked %s", path);
}

static void test_unlink_fchmod(const char* path) {
printf("%s...\n", __func__);

int fd = create_file(path, /*message=*/NULL, /*len=*/0);

if (unlink(path) == -1)
err(1, "unlink");

should_not_exist(path);

if (fchmod(fd, (mode_t)0644) == -1)
err(1, "fchmod");

if (close(fd) == -1)
err(1, "close unlinked %s", path);
}

int main(int argc, char* argv[]) {
setbuf(stdout, NULL);
setbuf(stderr, NULL);
Expand All @@ -239,6 +258,7 @@ int main(int argc, char* argv[]) {
test_rename_open_file(path1, path2);
test_unlink_and_recreate(path1);
test_unlink_and_write(path1);
test_unlink_fchmod(path1);
printf("TEST OK\n");
return 0;
}

0 comments on commit ac8fc77

Please sign in to comment.