Skip to content

Commit

Permalink
[LibOS,Pal] Fix operations on files without permissions
Browse files Browse the repository at this point in the history
This commit fixes the problem in which the user is not allowed to
manipulate (unlink, chmod) files without read permissions. Because PAL
requires us to open such files, we override the host mode in order to be
able to open them.

Signed-off-by: Paweł Marczewski <pawel@invisiblethingslab.com>
  • Loading branch information
pwmarcz authored and mkow committed Sep 6, 2021
1 parent 9ecc538 commit 8efe371
Show file tree
Hide file tree
Showing 4 changed files with 20 additions and 31 deletions.
22 changes: 18 additions & 4 deletions LibOS/shim/src/fs/chroot/fs.c
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@

#include "pal.h"
#include "pal_error.h"
#include "perm.h"
#include "shim_flags_conv.h"
#include "shim_fs.h"
#include "shim_handle.h"
Expand All @@ -33,6 +34,17 @@

#define KEEP_URI_PREFIX 0

/*
* Always add a read permission to files created on host, because PAL requires opening the file even
* for operations such as `unlink` or `chmod`.
*
* The updated file permissions will not be visible to the process creating the file or updating its
* permissions, e.g. if a process creates a write-only file, Gramine's `stat` will still report it
* as write-only. However, other Gramine processes accessing that file afterwards will see the
* updated permissions.
*/
#define HOST_PERM(perm) ((perm) | PERM_r________)

static int chroot_mount(const char* uri, void** mount_data) {
__UNUSED(mount_data);
if (!(strstartswith(uri, URI_PREFIX_FILE) || strstartswith(uri, URI_PREFIX_DEV)))
Expand Down Expand Up @@ -215,7 +227,7 @@ static int chroot_temp_open(struct shim_dentry* dent, mode_t type, PAL_HANDLE* o

/* Open a PAL handle, and associate it with a LibOS handle (if provided). */
static int chroot_do_open(struct shim_handle* hdl, struct shim_dentry* dent, mode_t type,
int flags, mode_t mode) {
int flags, mode_t perm) {
assert(locked(&dent->lock));

int ret;
Expand All @@ -229,7 +241,8 @@ static int chroot_do_open(struct shim_handle* hdl, struct shim_dentry* dent, mod
int access = LINUX_OPEN_FLAGS_TO_PAL_ACCESS(flags);
int create = LINUX_OPEN_FLAGS_TO_PAL_CREATE(flags);
int options = LINUX_OPEN_FLAGS_TO_PAL_OPTIONS(flags);
ret = DkStreamOpen(uri, access, mode, create, options, &palhdl);
mode_t host_perm = HOST_PERM(perm);
ret = DkStreamOpen(uri, access, host_perm, create, options, &palhdl);
if (ret < 0) {
ret = pal_to_unix_errno(ret);
goto out;
Expand Down Expand Up @@ -266,7 +279,7 @@ static int chroot_open(struct shim_handle* hdl, struct shim_dentry* dent, int fl
goto out;
}

ret = chroot_do_open(hdl, dent, dent->type, flags, /*mode=*/0);
ret = chroot_do_open(hdl, dent, dent->type, flags, /*perm=*/0);
if (ret < 0)
goto out;

Expand Down Expand Up @@ -681,7 +694,8 @@ static int chroot_chmod(struct shim_dentry* dent, mode_t perm) {
if (ret < 0)
goto out;

PAL_STREAM_ATTR attr = {.share_flags = perm};
mode_t host_perm = HOST_PERM(perm);
PAL_STREAM_ATTR attr = {.share_flags = host_perm};
ret = DkStreamAttributesSetByHandle(palhdl, &attr);
DkObjectClose(palhdl);
if (ret < 0) {
Expand Down
25 changes: 0 additions & 25 deletions LibOS/shim/test/ltp/ltp.cfg
Original file line number Diff line number Diff line change
Expand Up @@ -219,22 +219,6 @@ skip = yes
[dirtyc0w]
skip = yes

# Subtest 4 tries to unlink a file which is not readable. See
# https://github.com/oscarlab/graphene/issues/1248.
[dup07]
must-pass =
1
2
3

# Subtest 4 tries to unlink a file which is not readable. See
# https://github.com/oscarlab/graphene/issues/1248.
[dup202]
must-pass =
1
2
3

# complex test, not all of its checking is implemented in Graphene
[epoll01]
skip = yes
Expand Down Expand Up @@ -428,15 +412,6 @@ skip = yes
[fcntl26_64]
skip = yes

# 2. unlink in /tmp fails with EACCES
[fcntl28]
must-pass =
1

[fcntl28_64]
must-pass =
1

# no F_GETPIPE_SZ
[fcntl30]
skip = yes
Expand Down
2 changes: 1 addition & 1 deletion Pal/src/host/Linux-SGX/db_files.c
Original file line number Diff line number Diff line change
Expand Up @@ -777,7 +777,7 @@ static int file_attrquerybyhdl(PAL_HANDLE handle, PAL_STREAM_ATTR* attr) {

static int file_attrsetbyhdl(PAL_HANDLE handle, PAL_STREAM_ATTR* attr) {
int fd = handle->file.fd;
int ret = ocall_fchmod(fd, attr->share_flags | PERM_rw_______);
int ret = ocall_fchmod(fd, attr->share_flags);
if (ret < 0)
return unix_to_pal_error(ret);

Expand Down
2 changes: 1 addition & 1 deletion Pal/src/host/Linux/db_files.c
Original file line number Diff line number Diff line change
Expand Up @@ -248,7 +248,7 @@ static int file_attrquerybyhdl(PAL_HANDLE handle, PAL_STREAM_ATTR* attr) {
static int file_attrsetbyhdl(PAL_HANDLE handle, PAL_STREAM_ATTR* attr) {
int fd = handle->generic.fds[0], ret;

ret = DO_SYSCALL(fchmod, fd, attr->share_flags | PERM_rw_______);
ret = DO_SYSCALL(fchmod, fd, attr->share_flags);
if (ret < 0)
return unix_to_pal_error(ret);

Expand Down

0 comments on commit 8efe371

Please sign in to comment.