diff --git a/LibOS/shim/include/shim_fs.h b/LibOS/shim/include/shim_fs.h index db105896fd..d1c0062df4 100644 --- a/LibOS/shim/include/shim_fs.h +++ b/LibOS/shim/include/shim_fs.h @@ -85,39 +85,23 @@ struct shim_fs_ops { int (*migrate)(void* checkpoint, void** mount_data); }; -#define DENTRY_VALID 0x0001 /* this dentry is verified to be valid */ -#define DENTRY_NEGATIVE 0x0002 /* recently deleted or inaccessible */ -#define DENTRY_LOCKED 0x0200 /* locked by mountpoints at children */ -/* These flags are not used */ -//#define DENTRY_REACHABLE 0x0400 /* permission checked to be reachable */ -//#define DENTRY_UNREACHABLE 0x0800 /* permission checked to be unreachable */ -#define DENTRY_LISTED 0x1000 /* children in directory listed */ - -// Catch memory corruption issues by checking for invalid state values -#define DENTRY_INVALID_FLAGS (~0x7FFF) - -#define DCACHE_HASH_SIZE 1024 -#define DCACHE_HASH(hash) ((hash) & (DCACHE_HASH_SIZE - 1)) - /* Limit for the number of dentry children. This is mostly to prevent overflow if (untrusted) host * pretends to have many files in a directory. */ #define DENTRY_MAX_CHILDREN 1000000 struct fs_lock_info; +/* + * Describes a single path within a mounted filesystem. If `inode` is set, it is the file at given + * path. + * + * A dentry is called *positive* if `inode` is set, *negative* otherwise. + */ DEFINE_LIST(shim_dentry); DEFINE_LISTP(shim_dentry); struct shim_dentry { - /* - * TODO: Concurrent access to dentry objects is currently inconsistent and broken. The dentry - * tree structure, i.e. children/siblings, is protected by `g_dcache_lock`, but many fields are - * accessed without any locks. - * - * The end goal is to move some fields to `shim_inode`, and use `g_dcache_lock` for all - * remaining mutable fields. - */ - - int state; /* flags for managing state */ + /* Inode associated with this dentry, or NULL. Protected by `g_dcache_lock`. */ + struct shim_inode* inode; /* File name, maximum of NAME_MAX characters. By convention, the root has an empty name. Does * not change. Length is kept for performance reasons. */ @@ -127,32 +111,20 @@ struct shim_dentry { /* Mounted filesystem this dentry belongs to. Does not change. */ struct shim_mount* mount; - /* Filesystem to use for operations on this file: this is usually `mount->fs`, but can be - * different in case of special files (such as named pipes or sockets). */ - struct shim_fs* fs; - /* Parent of this dentry, but only within the same mount. If you need the dentry one level up, * regardless of mounts (i.e. `..`), you should use `dentry_up()` instead. Does not change. */ struct shim_dentry* parent; + /* The following fields are protected by `g_dcache_lock`. */ size_t nchildren; LISTP_TYPE(shim_dentry) children; /* These children and siblings link */ LIST_TYPE(shim_dentry) siblings; /* Filesystem mounted under this dentry. If set, this dentry is a mountpoint: filesystem - * operations should use `attached_mount->root` instead of this dentry. */ + * operations should use `attached_mount->root` instead of this dentry. Protected by + * `g_dcache_lock`. */ struct shim_mount* attached_mount; - /* file type: S_IFREG, S_IFDIR, S_IFLNK etc. */ - mode_t type; - - /* file permissions: PERM_rwxrwxrwx, etc. */ - mode_t perm; - - /* Inode associated with this dentry. Currently optional, and only for the use of underlying - * filesystem (see `shim_inode` below). Protected by `g_dcache_lock`. */ - struct shim_inode* inode; - /* File lock information, stored only in the main process. Managed by `shim_fs_lock.c`. */ struct fs_lock* fs_lock; @@ -167,9 +139,6 @@ struct shim_dentry { /* * Describes a single file in Gramine filesystem. * - * The migration to inodes is underway. Currently, the underlying filesystems may use fields in this - * structure, but should also write to corresponding fields in dentry. - * * The fields in this structure are protected by `lock`, with the exception of fields that do not * change (`type`, `mount`, `fs`). */ @@ -204,18 +173,17 @@ struct shim_inode { typedef int (*readdir_callback_t)(const char* name, void* arg); +/* TODO: Some of these operations could be simplified if they take an `inode` parameter. */ struct shim_d_ops { /* * \brief Look up a file. * - * \param dent Dentry, not valid. + * \param dent Dentry, negative. * * Queries the underlying filesystem for a path described by a dentry (`dent->name` and - * `dent->parent`). On success, prepares the dentry for use by the filesystem. Always sets the - * `type` and `perm` fields. + * `dent->parent`). On success, creates an inode and attaches it to the dentry. * - * The caller should hold `g_dcache_lock`. On success, the caller should mark the dentry as - * valid and non-negative. + * The caller should hold `g_dcache_lock`. */ int (*lookup)(struct shim_dentry* dent); @@ -223,7 +191,7 @@ struct shim_d_ops { * \brief Open an existing file. * * \param hdl A newly created handle. - * \param dent Dentry, valid and non-negative. + * \param dent Dentry, positive. * \param flags Open flags, including access mode (O_RDONLY / O_WRONLY / O_RDWR). * * Opens a file, and if successful, prepares the handle for use by the filesystem. Always sets @@ -238,49 +206,49 @@ struct shim_d_ops { * \brief Create and open a new regular file. * * \param hdl A newly created handle. - * \param dent Dentry, not valid (file to be created). + * \param dent Dentry, negative (file to be created). * \param flags Open flags, including access mode (O_RDONLY / O_WRONLY / O_RDWR). * \param perm Permissions of the new file. * - * Creates and opens a new regular file at path described by `dent`. On success, prepares the - * handle and the dentry for use by the filesystem (as in `open` and `lookup`). + * Creates and opens a new regular file at path described by `dent`. On success, creates an + * inode and attaches it to the dentry (as in `lookup`) and prepares the handle for use by the + * filesystem (as in `open`). * * The caller should hold `g_dcache_lock`. On success, the caller should finish preparing the - * handle and the dentry (as in `open` and `lookup`). + * handle (as in `open`). */ int (*creat)(struct shim_handle* hdl, struct shim_dentry* dent, int flags, mode_t perm); /* * \brief Create a directory. * - * \param dent dentry, not valid (directory to be created). + * \param dent Dentry, negative (directory to be created). * \param perm Permissions of the new directory. * - * Creates a new directory at path described by `dent`. On success, prepares the dentry for use - * by the filesystem (as in `lookup`). + * Creates a new directory at path described by `dent`. On success, creates an inode and + * attaches it to the dentry (as in `lookup`). * - * The caller should hold `g_dcache_lock`. On success, the caller should finish preparing the - * dentry (as in `lookup`). + * The caller should hold `g_dcache_lock`. */ int (*mkdir)(struct shim_dentry* dent, mode_t perm); /* * \brief Unlink a file. * - * \param dent Dentry, valid and non-negative, must have a parent. + * \param dent Dentry, positive, must have a parent. * * Unlinks a file described by `dent`. Note that there might be handles for that file; if * possible, they should still work. * - * The caller should hold `g_dcache_lock`. On success, the caller should update the dentry to be - * negative. + * The caller should hold `g_dcache_lock`. On success, the caller should detach the inode from + * the dentry. */ int (*unlink)(struct shim_dentry* dent); /* * \brief Get file status. * - * \param dent Dentry, valid and non-negative. + * \param dent Dentry, positive. * \param buf Status buffer to fill. * * Fills `buf` with information about a file. Omits `st_ino` (which is later filled by the @@ -293,7 +261,7 @@ struct shim_d_ops { /* * \brief Extract the target of a symbolic link. * - * \param dent Dentry, valid and non-negative, describing a symlink. + * \param dent Dentry, positive, describing a symlink. * \param out_target On success, contains link target. * * Determines the target of a symbolic link, and sets `*out_target` to an allocated string. @@ -305,20 +273,21 @@ struct shim_d_ops { /* * \brief Change file permissions. * - * \param dent Dentry, valid and non-negative. + * \param dent Dentry, positive. * \param perm New permissions for the file. * * Changes the permissions for a file. * - * The caller should hold `g_dcache_lock`. On success, the caller should update `dent->perm`. + * The caller should hold `g_dcache_lock`. On success, the caller should update + * `dent->inode->perm`. */ int (*chmod)(struct shim_dentry* dent, mode_t perm); /* * \brief Rename a file. * - * \param old Source dentry, valid and non-negative. - * \param new Target dentry, valid, can be negative or non-negative. + * \param old Source dentry, positive. + * \param new Target dentry, can be negative or positive. * * Moves a file described by `old` to the path described by `new`. Updates the fields of `new` * (same as `lookup`). @@ -334,7 +303,7 @@ struct shim_d_ops { /*! * \brief List all files in the directory. * - * \param dent The dentry, must be valid, non-negative and describing a directory. + * \param dent The dentry, must be positive and describing a directory. * \param callback The callback to invoke on each file name. * \param arg Argument to pass to the callback. * @@ -509,8 +478,7 @@ void dump_dcache(struct shim_dentry* dent); * * The caller should hold `g_dcache_lock`. * - * `dentry` should be a valid dentry, but can be negative (in which case the function will return - * -ENOENT). + * `dentry` can be negative (in which case the function will return -ENOENT). */ int check_permissions(struct shim_dentry* dent, mode_t mask); @@ -547,7 +515,8 @@ int check_permissions(struct shim_dentry* dent, mode_t mask); * On success, returns 0, and puts the retrieved dentry in `*found`. The reference count of the * dentry will be increased by one. * - * The retrieved dentry is always valid, and can only be negative if LOOKUP_CREATE is set. + * If LOOKUP_CREATE is set, the retrieved dentry can be negative. Otherwise, it is guaranteed to be + * positive. * * On failure, returns a negative error code, and sets `*found` to NULL. * @@ -639,8 +608,7 @@ int open_namei(struct shim_handle* hdl, struct shim_dentry* start, const char* p * \param dent The dentry to open. * \param flags Unix open flags. * - * The dentry has to already correspond to a file (i.e. has to be valid and non-negative). The - * caller has to hold `g_dcache_lock`. + * The dentry has to be positive. The caller has to hold `g_dcache_lock`. * * The `flags` parameter will be passed to the underlying filesystem's `open` function. If O_TRUNC * flag is specified, the filesystem's `truncate` function will also be called. @@ -679,25 +647,6 @@ void get_dentry(struct shim_dentry* dent); /* Decrement the reference count on dent */ void put_dentry(struct shim_dentry* dent); -/*! - * \brief Reset dentry state related to a file. - * - * \param dent The dentry (should be either invalid or negative). - * - * Resets the following dentry fields: `state`, `fs`, `type`, `perm`, `inode`. Ensures that there is - * no leftover state from a file previously associated with the dentry, and the dentry can be used - * for a new file. - * - * The caller should hold `g_dcache_lock`. - * - * Should be called before initializing the dentry for a new file (e.g. `lookup`, `create`, - * `mkdir`). - * - * TODO: This function should not be necessary after the inode migration, as most of the fields - * listed above will be removed. - */ -void reset_dentry(struct shim_dentry* dent); - /*! * \brief Get the dentry one level up. * @@ -719,8 +668,8 @@ struct shim_dentry* dentry_up(struct shim_dentry* dent); * This function checks if a dentry is unused, and deletes it if that's true. The caller must hold * `g_dcache_lock`. * - * A dentry is unused if it has no external references and does not correspond to a real file - * (i.e. is invalid or negative). Such dentries can remain after failed lookups or file deletion. + * A dentry is unused if it has no external references and is negative. Such dentries can remain + * after failed lookups or file deletion. * * The function should be called when processing a list of children, after you're done with a given * dentry. It guarantees that the amortized cost of processing such dentries is constant, i.e. they diff --git a/LibOS/shim/src/bookkeep/shim_handle.c b/LibOS/shim/src/bookkeep/shim_handle.c index aecfa9a6dc..a0253a111d 100644 --- a/LibOS/shim/src/bookkeep/shim_handle.c +++ b/LibOS/shim/src/bookkeep/shim_handle.c @@ -47,12 +47,12 @@ int open_executable(struct shim_handle* hdl, const char* path) { goto out; } - if (dent->type != S_IFREG) { + if (dent->inode->type != S_IFREG) { ret = -EACCES; goto out; } - if (!(dent->perm & S_IXUSR)) { + if (!(dent->inode->perm & S_IXUSR)) { ret = -EACCES; goto out; } @@ -718,16 +718,17 @@ BEGIN_CP_FUNC(handle) { if (hdl->uri) DO_CP_MEMBER(str, hdl, new_hdl, uri); + if (hdl->is_dir) { + /* + * We don't checkpoint children dentries of a directory dentry, so the child process + * will need to list the directory again. However, we keep `dir_info.pos` unchanged + * so that `getdents/getdents64` will resume from the same place. + */ + new_hdl->dir_info.dents = NULL; + new_hdl->dir_info.count = 0; + } + if (hdl->dentry) { - if (hdl->dentry->type == S_IFDIR) { - /* - * We don't checkpoint children dentries of a directory dentry, so the child process - * will need to list the directory again. However, we keep `dir_info.pos` unchanged - * so that `getdents/getdents64` will resume from the same place. - */ - new_hdl->dir_info.dents = NULL; - new_hdl->dir_info.count = 0; - } DO_CP_MEMBER(dentry, hdl, new_hdl, dentry); } diff --git a/LibOS/shim/src/fs/chroot/fs.c b/LibOS/shim/src/fs/chroot/fs.c index 8f9d73329a..f3e93ac291 100644 --- a/LibOS/shim/src/fs/chroot/fs.c +++ b/LibOS/shim/src/fs/chroot/fs.c @@ -55,8 +55,9 @@ static const char* strip_prefix(const char* uri) { /* * Calculate the URI for a dentry. The URI scheme is determined by file type (`type` field). It - * needs to be passed separately (instead of using `dent->type`) because the dentry might not have - * this information yet: we might be creating a new file, or looking up a file we don't know yet. + * needs to be passed separately (instead of using `dent->inode->type`) because the dentry might not + * have inode associated yet: we might be creating a new file, or looking up a file we don't know + * yet. * * If `type` is KEEP_URI_PREFIX, we keep the URI prefix from mount URI. */ @@ -133,9 +134,6 @@ static int chroot_setup_dentry(struct shim_dentry* dent, mode_t type, mode_t per assert(locked(&g_dcache_lock)); assert(!dent->inode); - dent->type = type; - dent->perm = perm; - struct shim_inode* inode = get_new_inode(dent->mount, type, perm); if (!inode) return -ENOMEM; @@ -259,13 +257,7 @@ static int chroot_open(struct shim_handle* hdl, struct shim_dentry* dent, int fl assert(locked(&g_dcache_lock)); assert(dent->inode); - int ret = chroot_do_open(hdl, dent, dent->type, flags, /*perm=*/0); - if (ret < 0) - return ret; - - hdl->inode = dent->inode; - get_inode(dent->inode); - return 0; + return chroot_do_open(hdl, dent, dent->inode->type, flags, /*perm=*/0); } static int chroot_creat(struct shim_handle* hdl, struct shim_dentry* dent, int flags, mode_t perm) { @@ -280,13 +272,7 @@ static int chroot_creat(struct shim_handle* hdl, struct shim_dentry* dent, int f if (ret < 0) return ret; - ret = chroot_setup_dentry(dent, type, perm, /*size=*/0); - if (ret < 0) - return ret; - - hdl->inode = dent->inode; - get_inode(dent->inode); - return 0; + return chroot_setup_dentry(dent, type, perm, /*size=*/0); } static int chroot_mkdir(struct shim_dentry* dent, mode_t perm) { @@ -488,7 +474,7 @@ static int chroot_unlink(struct shim_dentry* dent) { int ret; PAL_HANDLE palhdl; - ret = chroot_temp_open(dent, dent->type, &palhdl); + ret = chroot_temp_open(dent, dent->inode->type, &palhdl); if (ret < 0) return ret; @@ -497,9 +483,6 @@ static int chroot_unlink(struct shim_dentry* dent) { if (ret < 0) return pal_to_unix_errno(ret); - struct shim_inode* inode = dent->inode; - dent->inode = NULL; - put_inode(inode); return 0; } @@ -510,12 +493,12 @@ static int chroot_rename(struct shim_dentry* old, struct shim_dentry* new) { int ret; char* new_uri = NULL; - ret = chroot_dentry_uri(new, old->type, &new_uri); + ret = chroot_dentry_uri(new, old->inode->type, &new_uri); if (ret < 0) goto out; PAL_HANDLE palhdl; - ret = chroot_temp_open(old, old->type, &palhdl); + ret = chroot_temp_open(old, old->inode->type, &palhdl); if (ret < 0) goto out; @@ -525,17 +508,6 @@ static int chroot_rename(struct shim_dentry* old, struct shim_dentry* new) { ret = pal_to_unix_errno(ret); goto out; } - - reset_dentry(new); - - /* No need to adjust refcount of `old->inode`: we add a reference from `new` and remove the one - * from `old`. */ - new->inode = old->inode; - new->type = old->type; - new->perm = old->perm; - new->state |= DENTRY_VALID; - - old->inode = NULL; ret = 0; out: @@ -552,7 +524,7 @@ static int chroot_chmod(struct shim_dentry* dent, mode_t perm) { lock(&dent->inode->lock); PAL_HANDLE palhdl; - ret = chroot_temp_open(dent, dent->type, &palhdl); + ret = chroot_temp_open(dent, dent->inode->type, &palhdl); if (ret < 0) goto out; @@ -565,7 +537,6 @@ static int chroot_chmod(struct shim_dentry* dent, mode_t perm) { goto out; } - /* `dent->perm` already updated by caller */ dent->inode->perm = perm; ret = 0; diff --git a/LibOS/shim/src/fs/pipe/fs.c b/LibOS/shim/src/fs/pipe/fs.c index c8616202ee..5681a37651 100644 --- a/LibOS/shim/src/fs/pipe/fs.c +++ b/LibOS/shim/src/fs/pipe/fs.c @@ -76,12 +76,9 @@ int fifo_setup_dentry(struct shim_dentry* dent, mode_t perm, int fd_read, int fd fifo_data->fd_read = fd_read; fifo_data->fd_write = fd_write; - dent->fs = &fifo_builtin_fs; inode->fs = &fifo_builtin_fs; inode->data = fifo_data; - dent->type = S_IFIFO; - dent->perm = perm; dent->inode = inode; return 0; diff --git a/LibOS/shim/src/fs/shim_dcache.c b/LibOS/shim/src/fs/shim_dcache.c index e6f4d53de0..59c3fcb521 100644 --- a/LibOS/shim/src/fs/shim_dcache.c +++ b/LibOS/shim/src/fs/shim_dcache.c @@ -63,23 +63,24 @@ int init_dcache(void) { return 0; } + /* + * Prepare `g_dentry_root`. Note that this dentry is special: + * + * - It has an extra reference, so that it's never deallocated + * - It doesn't have `mount` + * - It's always negative (doesn't have `inode`) + * + * Most functions do not need to handle `g_dentry_root`, because it's used only as a mountpoint + * for the root filesystem. For instance, a lookup of "/" will not retrieve `g_dentry_root`, but + * the root dentry of the filesystem mounted there. + */ g_dentry_root = alloc_dentry(); if (!g_dentry_root) { return -ENOMEM; } - /* The root is special; we assume it won't change or be freed, so we artificially increase its - * refcount by 1. */ get_dentry(g_dentry_root); - /* Initialize the root to a valid state, as a low-level lookup - * will fail. */ - g_dentry_root->state |= DENTRY_VALID; - - /* The root should be a directory too */ - g_dentry_root->perm = PERM_rwx______; - g_dentry_root->type = S_IFDIR; - char* name = strdup(""); if (!name) { free_dentry(g_dentry_root); @@ -152,7 +153,7 @@ void dentry_gc(struct shim_dentry* dent) { if (REF_GET(dent->ref_count) != 1) return; - if ((dent->state & DENTRY_VALID) && !(dent->state & DENTRY_NEGATIVE)) + if (dent->inode) return; LISTP_DEL_INIT(dent, &dent->parent->children, siblings); @@ -161,20 +162,6 @@ void dentry_gc(struct shim_dentry* dent) { put_dentry(dent); } -void reset_dentry(struct shim_dentry* dent) { - assert(locked(&g_dcache_lock)); - - dent->state = 0; - dent->fs = dent->mount ? dent->mount->fs : NULL; - dent->type = 0; - dent->perm = 0; - - if (dent->inode) { - put_inode(dent->inode); - dent->inode = NULL; - } -} - struct shim_dentry* get_new_dentry(struct shim_mount* mount, struct shim_dentry* parent, const char* name, size_t name_len) { assert(locked(&g_dcache_lock)); @@ -201,7 +188,6 @@ struct shim_dentry* get_new_dentry(struct shim_mount* mount, struct shim_dentry* if (mount) { get_mount(mount); dent->mount = mount; - dent->fs = mount->fs; } if (parent) { @@ -434,44 +420,41 @@ static void dump_dentry_mode(struct print_buf* buf, mode_t type, mode_t perm) { buf_putc(buf, ' '); } -#define DUMP_FLAG(flag, s, empty) buf_puts(&buf, (dent->state & (flag)) ? (s) : (empty)) - static void dump_dentry(struct shim_dentry* dent, unsigned int level) { assert(locked(&g_dcache_lock)); struct print_buf buf = INIT_PRINT_BUF(dump_dentry_write_all); - buf_printf(&buf, "[%6.6s ", dent->fs ? dent->fs->name : ""); + buf_printf(&buf, "[%6.6s ", dent->inode ? dent->inode->fs->name : ""); - DUMP_FLAG(DENTRY_VALID, "V", "."); - DUMP_FLAG(DENTRY_LISTED, "L", "."); buf_printf(&buf, "%3d] ", (int)REF_GET(dent->ref_count)); - dump_dentry_mode(&buf, dent->type, dent->perm); - - if (dent->attached_mount) { - buf_puts(&buf, "M"); - } else if (!dent->parent) { - buf_puts(&buf, "*"); + if (dent->inode) { + dump_dentry_mode(&buf, dent->inode->type, dent->inode->perm); } else { - buf_puts(&buf, " "); + buf_puts(&buf, "------ ---- "); } - DUMP_FLAG(DENTRY_NEGATIVE, "-", " "); + + buf_puts(&buf, dent->attached_mount ? "M" : " "); for (unsigned int i = 0; i < level; i++) buf_puts(&buf, " "); buf_puts(&buf, dent->name); - switch (dent->type) { - case S_IFDIR: buf_puts(&buf, "/"); break; - case S_IFLNK: buf_puts(&buf, " -> "); break; - default: break; + + if (dent->inode) { + switch (dent->inode->type) { + case S_IFDIR: buf_puts(&buf, "/"); break; + case S_IFLNK: buf_puts(&buf, " -> "); break; + default: break; + } } + if (!dent->parent && dent->mount) { buf_printf(&buf, " (%s \"%s\")", dent->mount->fs->name, dent->mount->uri); } - buf_flush(&buf); + buf_flush(&buf); if (dent->attached_mount) { struct shim_dentry* root = dent->attached_mount->root; @@ -546,9 +529,6 @@ BEGIN_CP_FUNC(dentry) { INIT_LIST_HEAD(new_dent, siblings); REF_SET(new_dent->ref_count, 0); - /* we don't checkpoint children dentries, so need to list directory again */ - new_dent->state &= ~DENTRY_LISTED; - /* `fs_lock` is used only by process leader. */ new_dent->fs_lock = NULL; @@ -557,9 +537,6 @@ BEGIN_CP_FUNC(dentry) { if (new_dent->mount) DO_CP_MEMBER(mount, dent, new_dent, mount); - if (new_dent->fs) - DO_CP_MEMBER(fs, dent, new_dent, fs); - if (dent->parent) DO_CP_MEMBER(dentry, dent, new_dent, parent); @@ -587,7 +564,6 @@ BEGIN_RS_FUNC(dentry) { CP_REBASE(dent->children); CP_REBASE(dent->siblings); CP_REBASE(dent->mount); - CP_REBASE(dent->fs); CP_REBASE(dent->parent); CP_REBASE(dent->attached_mount); CP_REBASE(dent->inode); diff --git a/LibOS/shim/src/fs/shim_fs.c b/LibOS/shim/src/fs/shim_fs.c index d89df10f56..7124947a49 100644 --- a/LibOS/shim/src/fs/shim_fs.c +++ b/LibOS/shim/src/fs/shim_fs.c @@ -498,7 +498,7 @@ static int mount_fs_at_dentry(const char* type, const char* uri, const char* mou goto err; } - /* Trigger filesystem lookup for the root dentry, so that it's already valid. If there is a + /* Trigger filesystem lookup for the root dentry, so that it's already positive. If there is a * problem looking up the root, we want the mount operation to fail. */ struct shim_dentry* root; @@ -550,10 +550,18 @@ int mount_fs(const char* type, const char* uri, const char* mount_path) { lock(&g_dcache_lock); - int lookup_flags = LOOKUP_NO_FOLLOW | LOOKUP_MAKE_SYNTHETIC; - if ((ret = path_lookupat(g_dentry_root, mount_path, lookup_flags, &mount_point)) < 0) { - log_error("error looking up mountpoint %s: %d", mount_path, ret); - goto out; + if (!g_dentry_root->attached_mount && !strcmp(mount_path, "/")) { + /* `g_dentry_root` does not belong to any mounted filesystem, so lookup will fail. Use it + * directly. */ + mount_point = g_dentry_root; + get_dentry(g_dentry_root); + } else { + int lookup_flags = LOOKUP_NO_FOLLOW | LOOKUP_MAKE_SYNTHETIC; + ret = path_lookupat(g_dentry_root, mount_path, lookup_flags, &mount_point); + if (ret < 0) { + log_error("error looking up mountpoint %s: %d", mount_path, ret); + goto out; + } } if ((ret = mount_fs_at_dentry(type, uri, mount_path, mount_point)) < 0) { diff --git a/LibOS/shim/src/fs/shim_fs_pseudo.c b/LibOS/shim/src/fs/shim_fs_pseudo.c index 1dd8190247..7805bfdbaa 100644 --- a/LibOS/shim/src/fs/shim_fs_pseudo.c +++ b/LibOS/shim/src/fs/shim_fs_pseudo.c @@ -139,8 +139,6 @@ static int pseudo_open(struct shim_handle* hdl, struct shim_dentry* dent, int fl break; } } - hdl->inode = dent->inode; - get_inode(dent->inode); return 0; } @@ -177,8 +175,6 @@ static int pseudo_lookup(struct shim_dentry* dent) { inode->data = node; - dent->type = type; - dent->perm = node->perm; dent->inode = inode; return 0; } diff --git a/LibOS/shim/src/fs/shim_fs_synthetic.c b/LibOS/shim/src/fs/shim_fs_synthetic.c index 820a44b6ab..e14826c6ad 100644 --- a/LibOS/shim/src/fs/shim_fs_synthetic.c +++ b/LibOS/shim/src/fs/shim_fs_synthetic.c @@ -22,15 +22,11 @@ int synthetic_setup_dentry(struct shim_dentry* dent, mode_t type, mode_t perm) { assert(locked(&g_dcache_lock)); assert(!dent->inode); - dent->type = type; - dent->perm = perm; - struct shim_inode* inode = get_new_inode(dent->mount, type, perm); if (!inode) return -ENOMEM; dent->inode = inode; - dent->fs = &synthetic_builtin_fs; inode->fs = &synthetic_builtin_fs; return 0; @@ -39,11 +35,10 @@ int synthetic_setup_dentry(struct shim_dentry* dent, mode_t type, mode_t perm) { static int synthetic_open(struct shim_handle* hdl, struct shim_dentry* dent, int flags) { assert(locked(&g_dcache_lock)); assert(dent->inode); + __UNUSED(dent); __UNUSED(flags); hdl->type = TYPE_SYNTHETIC; - hdl->inode = dent->inode; - get_inode(dent->inode); return 0; } diff --git a/LibOS/shim/src/fs/shim_fs_util.c b/LibOS/shim/src/fs/shim_fs_util.c index a8589c73bd..613182276f 100644 --- a/LibOS/shim/src/fs/shim_fs_util.c +++ b/LibOS/shim/src/fs/shim_fs_util.c @@ -40,11 +40,12 @@ int generic_seek(file_off_t pos, file_off_t size, file_off_t offset, int origin, int generic_readdir(struct shim_dentry* dent, readdir_callback_t callback, void* arg) { assert(locked(&g_dcache_lock)); - assert(dent->type == S_IFDIR); + assert(dent->inode); + assert(dent->inode->type == S_IFDIR); struct shim_dentry* child; LISTP_FOR_EACH_ENTRY(child, &dent->children, siblings) { - if ((child->state & DENTRY_VALID) && !(child->state & DENTRY_NEGATIVE)) { + if (child->inode) { int ret = callback(child->name, arg); if (ret < 0) return ret; diff --git a/LibOS/shim/src/fs/shim_namei.c b/LibOS/shim/src/fs/shim_namei.c index c3855820c5..8ee6580aa1 100644 --- a/LibOS/shim/src/fs/shim_namei.c +++ b/LibOS/shim/src/fs/shim_namei.c @@ -18,9 +18,8 @@ int check_permissions(struct shim_dentry* dent, mode_t mask) { assert(locked(&g_dcache_lock)); - assert(dent->state & DENTRY_VALID); - if (dent->state & DENTRY_NEGATIVE) + if (!dent->inode) return -ENOENT; /* If we only check if the file exists, at this point we know that */ @@ -28,14 +27,14 @@ int check_permissions(struct shim_dentry* dent, mode_t mask) { return 0; /* Check the "user" part of mode against mask */ - if (((dent->perm >> 6) & mask) == mask) + if (((dent->inode->perm >> 6) & mask) == mask) return 0; return -EACCES; } /* This function works like `lookup_dcache`, but if the dentry is not in cache, it creates a new, - * not yet valid one. */ + * negative one. */ static struct shim_dentry* lookup_dcache_or_create(struct shim_dentry* parent, const char* name, size_t name_len) { assert(locked(&g_dcache_lock)); @@ -47,51 +46,25 @@ static struct shim_dentry* lookup_dcache_or_create(struct shim_dentry* parent, c return dent; } -/*! - * \brief Validate a dentry, if necessary - * - * \param dent dentry to query - * - * \return 0 on success, negative error code otherwise - * - * This function makes sure a dentry is valid. If the dentry is not yet valid (DENTRY_VALID is not - * set), it invokes the lookup operation of the underlying filesystem. On success, it returns 0 and - * marks the dentry as valid. - * - * A negative filesystem lookup (ENOENT) is also considered a success, and the dentry is marked as - * valid and negative. - * - * If the dentry is already valid, this function succeeds without doing anything. - * - * The caller should hold `g_dcache_lock`. - */ -static int validate_dentry(struct shim_dentry* dent) { +/* Performs lookup operation in the underlying filesystem. Treats -ENOENT from lookup operation as + * success (but leaves the dentry negative). */ +static int lookup_dentry(struct shim_dentry* dent) { assert(locked(&g_dcache_lock)); - if (dent->state & DENTRY_VALID) + if (dent->inode) return 0; - reset_dentry(dent); - - /* This is an invalid dentry: either we just created it, or it got left over from a previous - * failed lookup. Perform the lookup. */ - assert(dent->fs); - assert(dent->fs->d_ops); - assert(dent->fs->d_ops->lookup); - int ret = dent->fs->d_ops->lookup(dent); - - if (ret == 0) { - /* Lookup succeeded. */ - dent->state |= DENTRY_VALID; - return 0; - } else if (ret == -ENOENT) { - /* File not found, mark dentry as negative */ - dent->state |= DENTRY_VALID | DENTRY_NEGATIVE; - return 0; - } else { - /* Lookup failed, keep dentry as invalid */ - return ret; + assert(dent->mount); + assert(dent->mount->fs->d_ops); + assert(dent->mount->fs->d_ops->lookup); + int ret = dent->mount->fs->d_ops->lookup(dent); + if (ret < 0) { + assert(!dent->inode); + /* Treat -ENOENT as successful lookup (but leave the dentry negative) */ + return ret == -ENOENT ? 0 : ret; } + assert(dent->inode); + return 0; } static int do_path_lookupat(struct shim_dentry* start, const char* path, int flags, @@ -105,10 +78,11 @@ static int path_lookupat_follow(struct shim_dentry* link, int flags, struct shim assert(locked(&g_dcache_lock)); - assert(link->fs); - assert(link->fs->d_ops); - assert(link->fs->d_ops->follow_link); - ret = link->fs->d_ops->follow_link(link, &target); + assert(link->inode); + struct shim_fs* fs = link->inode->fs; + assert(fs->d_ops); + assert(fs->d_ops->follow_link); + ret = fs->d_ops->follow_link(link, &target); if (ret < 0) goto out; @@ -123,15 +97,15 @@ static int path_lookupat_follow(struct shim_dentry* link, int flags, struct shim } /*! - * \brief Traverse mountpoints and validate dentry + * \brief Traverse mountpoints and look up a dentry * * \param[in,out] dent the dentry * * \return 0 on success, negative error code otherwise * * If `*dent` is a mountpoint, this function converts it to the underlying filesystem root - * (iterating if necessary). All dentries on the way are validated. As a result, on success `*dent` - * is guaranteed to be valid and not a mountpoint. + * (iterating if necessary), then performs a lookup on the last dentry. As in `lookup_dentry`, if + * the file is not found, the function will still return 0 (but `*dent` will be negative). * * The caller should hold a reference to `*dent`. On success, the reference will be converted: the * function will decrease the reference count for the original dentry, and increase it for the new @@ -139,34 +113,24 @@ static int path_lookupat_follow(struct shim_dentry* link, int flags, struct shim * * The caller should hold `g_dcache_lock`. */ -static int traverse_mount_and_validate(struct shim_dentry** dent) { +static int traverse_mount_and_lookup(struct shim_dentry** dent) { assert(locked(&g_dcache_lock)); - int ret; struct shim_dentry* cur_dent = *dent; - get_dentry(cur_dent); - - if ((ret = validate_dentry(cur_dent)) < 0) - goto out; - while (cur_dent->attached_mount) { - get_dentry(cur_dent->attached_mount->root); - put_dentry(cur_dent); cur_dent = cur_dent->attached_mount->root; - if ((ret = validate_dentry(cur_dent)) < 0) - goto out; } + int ret = lookup_dentry(cur_dent); + if (ret < 0) + return ret; + if (cur_dent != *dent) { - put_dentry(*dent); get_dentry(cur_dent); + put_dentry(*dent); *dent = cur_dent; } - ret = 0; - -out: - put_dentry(cur_dent); - return ret; + return 0; } /* State of the lookup algorithm */ @@ -188,16 +152,16 @@ struct lookup { }; /* Process a new dentry in the lookup: follow mounts and symbolic links, then check if the resulting - * dentry is valid. */ + * dentry is positive. */ static int lookup_enter_dentry(struct lookup* lookup) { int ret; bool is_final = (*lookup->name == '\0'); bool has_slash = lookup->has_slash; - if ((ret = traverse_mount_and_validate(&lookup->dent)) < 0) + if ((ret = traverse_mount_and_lookup(&lookup->dent)) < 0) return ret; - if (!(lookup->dent->state & DENTRY_NEGATIVE) && (lookup->dent->type == S_IFLNK)) { + if (lookup->dent->inode && lookup->dent->inode->type == S_IFLNK) { /* Traverse the symbolic link. This applies to all intermediate segments, final segments * ending with slash, and to all final segments if LOOKUP_FOLLOW is set. */ if (!is_final || has_slash || (lookup->flags & LOOKUP_FOLLOW)) { @@ -224,7 +188,7 @@ static int lookup_enter_dentry(struct lookup* lookup) { } } - if (lookup->dent->state & DENTRY_NEGATIVE) { + if (!lookup->dent->inode) { /* * The file does not exist: * @@ -233,18 +197,15 @@ static int lookup_enter_dentry(struct lookup* lookup) { * - otherwise, fail with -ENOENT. */ if (lookup->flags & LOOKUP_MAKE_SYNTHETIC) { - reset_dentry(lookup->dent); ret = synthetic_setup_dentry(lookup->dent, S_IFDIR, PERM_r_xr_xr_x); if (ret < 0) return ret; - - lookup->dent->state |= DENTRY_VALID; } else if (is_final && (lookup->flags & LOOKUP_CREATE)) { /* proceed with a negative dentry */ } else { return -ENOENT; } - } else if (lookup->dent->type != S_IFDIR) { + } else if (lookup->dent->inode->type != S_IFDIR) { /* * The file exists, but is not a directory. We expect a directory (and need to fail with * -ENOTDIR) in the following cases: @@ -409,21 +370,27 @@ static inline int open_flags_to_lookup_flags(int flags) { } static void assoc_handle_with_dentry(struct shim_handle* hdl, struct shim_dentry* dent, int flags) { - hdl->fs = dent->fs; - get_dentry(dent); + assert(locked(&g_dcache_lock)); + assert(dent->inode); + hdl->dentry = dent; + get_dentry(dent); + + hdl->inode = dent->inode; + get_inode(dent->inode); + + hdl->fs = dent->inode->fs; hdl->flags = flags; hdl->acc_mode = ACC_MODE(flags & O_ACCMODE); } int dentry_open(struct shim_handle* hdl, struct shim_dentry* dent, int flags) { assert(locked(&g_dcache_lock)); - assert(dent->state & DENTRY_VALID); - assert(!(dent->state & DENTRY_NEGATIVE)); + assert(dent->inode); assert(!hdl->dentry); int ret = 0; - struct shim_fs* fs = dent->fs; + struct shim_fs* fs = dent->inode->fs; if (!(fs->d_ops && fs->d_ops->open)) { ret = -EINVAL; @@ -436,7 +403,7 @@ int dentry_open(struct shim_handle* hdl, struct shim_dentry* dent, int flags) { assoc_handle_with_dentry(hdl, dent, flags); - if (dent->type == S_IFDIR) { + if (dent->inode->type == S_IFDIR) { /* Initialize directory handle */ hdl->is_dir = true; @@ -445,8 +412,8 @@ int dentry_open(struct shim_handle* hdl, struct shim_dentry* dent, int flags) { /* truncate regular writable file if O_TRUNC is given */ if ((flags & O_TRUNC) && ((flags & O_RDWR) | (flags & O_WRONLY)) - && (dent->type != S_IFDIR) - && (dent->type != S_IFLNK)) { + && (dent->inode->type != S_IFDIR) + && (dent->inode->type != S_IFLNK)) { if (!(fs->fs_ops && fs->fs_ops->truncate)) { ret = -EINVAL; @@ -491,9 +458,7 @@ int open_namei(struct shim_handle* hdl, struct shim_dentry* start, const char* p if (ret < 0) goto err; - assert(dent->state & DENTRY_VALID); - - if (dent->type == S_IFDIR) { + if (dent->inode && dent->inode->type == S_IFDIR) { if (flags & O_WRONLY || flags & O_RDWR || ((flags & O_CREAT) && !(flags & O_DIRECTORY) && !(flags & O_EXCL))) { ret = -EISDIR; @@ -501,7 +466,7 @@ int open_namei(struct shim_handle* hdl, struct shim_dentry* start, const char* p } } - if (dent->type == S_IFLNK) { + if (dent->inode && dent->inode->type == S_IFLNK) { /* * Can happen if user specified O_NOFOLLOW, or O_TRUNC | O_EXCL. Posix requires us to fail * with -ELOOP when trying to open a symlink. @@ -513,7 +478,7 @@ int open_namei(struct shim_handle* hdl, struct shim_dentry* start, const char* p } bool need_open = true; - if (dent->state & DENTRY_NEGATIVE) { + if (!dent->inode) { if (!(flags & O_CREAT)) { ret = -ENOENT; goto err; @@ -521,38 +486,35 @@ int open_namei(struct shim_handle* hdl, struct shim_dentry* start, const char* p /* The root always exists, so if we got here, the dentry should have a parent */ struct shim_dentry* dir = dent->parent; - assert(dir); - assert(dir->fs); - assert(dir->fs->d_ops); + if (!dir->inode) { + ret = -ENOENT; + goto err; + } /* Check the parent permission first */ ret = check_permissions(dir, MAY_WRITE | MAY_EXEC); if (ret < 0) goto err; - reset_dentry(dent); - + struct shim_fs* fs = dir->inode->fs; /* Create directory or file, depending on O_DIRECTORY. Return -EINVAL if the operation is * not supported for this filesystem. */ if (flags & O_DIRECTORY) { - if (!dir->fs->d_ops->mkdir) { + if (!fs->d_ops->mkdir) { ret = -EINVAL; goto err; } - ret = dir->fs->d_ops->mkdir(dent, mode & ~S_IFMT); + ret = fs->d_ops->mkdir(dent, mode & ~S_IFMT); if (ret < 0) goto err; - dent->state |= DENTRY_VALID; - dent->type = S_IFDIR; } else { - if (!dir->fs->d_ops->creat) { + if (!fs->d_ops->creat) { ret = -EINVAL; goto err; } - ret = dir->fs->d_ops->creat(hdl, dent, flags, mode & ~S_IFMT); + ret = fs->d_ops->creat(hdl, dent, flags, mode & ~S_IFMT); if (ret < 0) goto err; - dent->state |= DENTRY_VALID; assoc_handle_with_dentry(hdl, dent, flags); need_open = false; } @@ -620,8 +582,8 @@ static int add_name(const char* name, void* arg) { } /* - * Ensure that a directory has a complete list of dentries, by calling `readdir` and then - * ensuring every name corresponds to a valid dentry. + * Ensure that a directory has a complete list of dentries, by calling `readdir` and then looking up + * every name. * * While `readdir` is callback-based, we don't look up the names inside of callback, but first * finish `readdir`. Otherwise, the two filesystem operations (`readdir` and `lookup`) might @@ -630,36 +592,36 @@ static int add_name(const char* name, void* arg) { static int populate_directory(struct shim_dentry* dent) { assert(locked(&g_dcache_lock)); - if (dent->state & DENTRY_NEGATIVE) + if (!dent->inode) return -ENOENT; - if (!dent->fs || !dent->fs->d_ops || !dent->fs->d_ops->readdir) + struct shim_fs* fs = dent->inode->fs; + if (!fs->d_ops || !fs->d_ops->readdir) return -EINVAL; LISTP_TYPE(temp_dirent) ents = LISTP_INIT; - int ret = dent->fs->d_ops->readdir(dent, &add_name, &ents); + int ret = fs->d_ops->readdir(dent, &add_name, &ents); if (ret < 0) log_error("readdir error: %d", ret); struct shim_dentry* child; LISTP_FOR_EACH_ENTRY(child, &dent->children, siblings) { - struct temp_dirent* ent; - bool removed = true; - LISTP_FOR_EACH_ENTRY(ent, &ents, list) { - if (strcmp(ent->name, child->name) == 0) { - removed = false; - break; + struct shim_inode* inode = child->inode; + /* Check `inode->fs` so that we don't remove files added by Gramine (named pipes, sockets, + * synthetic mountpoints) */ + if (inode && inode->fs == inode->mount->fs) { + struct temp_dirent* ent; + bool removed = true; + LISTP_FOR_EACH_ENTRY(ent, &ents, list) { + if (strcmp(ent->name, child->name) == 0) { + removed = false; + break; + } } - } - - if (removed) { - /* Do not remove dentries added by Gramine (named pipes, sockets, synthetic - * mountpoints). */ - if ((child->state & DENTRY_VALID) && !(child->state & DENTRY_NEGATIVE) && - (child->fs == child->mount->fs)) { - log_debug("Directory no longer present, removing dentry: %s", child->name); - child->state &= ~DENTRY_VALID; - child->state |= DENTRY_NEGATIVE; + if (removed) { + log_debug("File no longer present, detaching inode: %s", child->name); + child->inode = NULL; + put_inode(inode); } } } @@ -674,17 +636,14 @@ static int populate_directory(struct shim_dentry* dent) { goto out; } - ret = traverse_mount_and_validate(&child); + ret = traverse_mount_and_lookup(&child); put_dentry(child); - if (ret < 0) { - if (ret != -EACCES) { - /* Fail on underlying lookup errors, except -EACCES (for which we will just ignore - * the file). The lookup might fail with -EACCES for host symlinks pointing to - * inaccessible target, since the "chroot" filesystem transparently follows symlinks - * instead of reporting them to Gramine. */ - goto out; - } - continue; + if (ret < 0 && ret != -EACCES) { + /* Fail on underlying lookup errors, except -EACCES (for which we will just ignore the + * file). The lookup might fail with -EACCES for host symlinks pointing to inaccessible + * target, since the "chroot" filesystem transparently follows symlinks instead of + * reporting them to Gramine. */ + goto out; } } @@ -732,19 +691,16 @@ int populate_directory_handle(struct shim_handle* hdl) { struct shim_dentry* tmp; struct shim_dentry* dent; LISTP_FOR_EACH_ENTRY_SAFE(dent, tmp, &hdl->dentry->children, siblings) { - if (dent->state & DENTRY_VALID) { - /* Traverse mount */ - struct shim_dentry* cur_dent = dent; - while (cur_dent->attached_mount) { - cur_dent = cur_dent->attached_mount->root; - } + /* Traverse mount */ + struct shim_dentry* cur_dent = dent; + while (cur_dent->attached_mount) { + cur_dent = cur_dent->attached_mount->root; + } - assert(cur_dent->state & DENTRY_VALID); - if (!(cur_dent->state & DENTRY_NEGATIVE)) { - get_dentry(cur_dent); - assert(dirhdl->count < capacity); - dirhdl->dents[dirhdl->count++] = cur_dent; - } + if (cur_dent->inode) { + get_dentry(cur_dent); + assert(dirhdl->count < capacity); + dirhdl->dents[dirhdl->count++] = cur_dent; } dentry_gc(dent); } diff --git a/LibOS/shim/src/fs/socket/fs.c b/LibOS/shim/src/fs/socket/fs.c index 8fb58c5667..6f0621bbfb 100644 --- a/LibOS/shim/src/fs/socket/fs.c +++ b/LibOS/shim/src/fs/socket/fs.c @@ -24,11 +24,8 @@ int unix_socket_setup_dentry(struct shim_dentry* dent, mode_t perm) { if (!inode) return -ENOMEM; - dent->fs = &socket_builtin_fs; inode->fs = &socket_builtin_fs; - dent->type = S_IFSOCK; - dent->perm = perm; dent->inode = inode; return 0; } diff --git a/LibOS/shim/src/fs/tmpfs/fs.c b/LibOS/shim/src/fs/tmpfs/fs.c index 02ba6768b9..90071771ec 100644 --- a/LibOS/shim/src/fs/tmpfs/fs.c +++ b/LibOS/shim/src/fs/tmpfs/fs.c @@ -28,9 +28,6 @@ static int tmpfs_setup_dentry(struct shim_dentry* dent, mode_t type, mode_t perm assert(locked(&g_dcache_lock)); assert(!dent->inode); - dent->type = type; - dent->perm = perm; - struct shim_inode* inode = get_new_inode(dent->mount, type, perm); if (!inode) return -ENOMEM; @@ -136,12 +133,11 @@ static int tmpfs_lookup(struct shim_dentry* dent) { static void tmpfs_do_open(struct shim_handle* hdl, struct shim_dentry* dent, int flags) { assert(locked(&g_dcache_lock)); assert(dent->inode); + __UNUSED(dent); __UNUSED(flags); hdl->type = TYPE_TMPFS; hdl->pos = 0; - hdl->inode = dent->inode; - get_inode(dent->inode); } static int tmpfs_open(struct shim_handle* hdl, struct shim_dentry* dent, int flags) { @@ -156,8 +152,7 @@ static int tmpfs_creat(struct shim_handle* hdl, struct shim_dentry* dent, int fl assert(locked(&g_dcache_lock)); assert(!dent->inode); - mode_t type = S_IFREG; - int ret = tmpfs_setup_dentry(dent, type, perm); + int ret = tmpfs_setup_dentry(dent, S_IFREG, perm); if (ret < 0) return ret; @@ -169,19 +164,18 @@ static int tmpfs_mkdir(struct shim_dentry* dent, mode_t perm) { assert(locked(&g_dcache_lock)); assert(!dent->inode); - mode_t type = S_IFREG; - return tmpfs_setup_dentry(dent, type, perm); + return tmpfs_setup_dentry(dent, S_IFDIR, perm); } static int tmpfs_unlink(struct shim_dentry* dent) { assert(locked(&g_dcache_lock)); assert(dent->inode); - if (dent->type == S_IFDIR) { + if (dent->inode->type == S_IFDIR) { struct shim_dentry* child; bool found = false; LISTP_FOR_EACH_ENTRY(child, &dent->children, siblings) { - if ((child->state & DENTRY_VALID) && !(child->state & DENTRY_NEGATIVE)) { + if (child->inode) { found = true; break; } @@ -189,16 +183,13 @@ static int tmpfs_unlink(struct shim_dentry* dent) { if (found) return -ENOTEMPTY; } - - struct shim_inode* inode = dent->inode; - dent->inode = NULL; - put_inode(inode); return 0; } static int tmpfs_rename(struct shim_dentry* old, struct shim_dentry* new) { assert(locked(&g_dcache_lock)); assert(old->inode); + __UNUSED(new); uint64_t time_us; if (DkSystemTimeQuery(&time_us) < 0) @@ -206,41 +197,16 @@ static int tmpfs_rename(struct shim_dentry* old, struct shim_dentry* new) { /* TODO: this should be done in the syscall handler, not here */ - struct shim_inode* new_inode = new->inode; - if (new_inode) { - new->inode = NULL; - put_inode(new_inode); - } - - struct shim_inode* old_inode = old->inode; - - lock(&old_inode->lock); - - /* No need to adjust refcount of `old->inode`: we add a reference from `new` and remove the one - * from `old`. */ - new->inode = old_inode; - new->type = old->type; - new->perm = old->perm; - - old->inode = NULL; - - old_inode->ctime = time_us / USEC_IN_SEC; - - unlock(&old_inode->lock); + lock(&old->inode->lock); + old->inode->ctime = time_us / USEC_IN_SEC; + unlock(&old->inode->lock); return 0; } static int tmpfs_chmod(struct shim_dentry* dent, mode_t perm) { - assert(locked(&g_dcache_lock)); - assert(dent->inode); - - lock(&dent->inode->lock); - - /* `dent->perm` already updated by caller */ - dent->inode->perm = perm; - - unlock(&dent->inode->lock); + __UNUSED(dent); + __UNUSED(perm); return 0; } diff --git a/LibOS/shim/src/sys/shim_file.c b/LibOS/shim/src/sys/shim_file.c index 2804b0880c..59fa959df4 100644 --- a/LibOS/shim/src/sys/shim_file.c +++ b/LibOS/shim/src/sys/shim_file.c @@ -35,7 +35,7 @@ long shim_do_unlinkat(int dfd, const char* pathname, int flag) { struct shim_dentry* dir = NULL; struct shim_dentry* dent = NULL; - int ret = 0; + int ret; if (*pathname != '/' && (ret = get_dirfd_dentry(dfd, &dir)) < 0) return ret; @@ -51,25 +51,28 @@ long shim_do_unlinkat(int dfd, const char* pathname, int flag) { } if (flag & AT_REMOVEDIR) { - if (dent->type != S_IFDIR) { + if (dent->inode->type != S_IFDIR) { ret = -ENOTDIR; goto out; } } else { - if (dent->type == S_IFDIR) { + if (dent->inode->type == S_IFDIR) { ret = -EISDIR; goto out; } } - if (dent->fs && dent->fs->d_ops && dent->fs->d_ops->unlink) { - ret = dent->fs->d_ops->unlink(dent); + struct shim_fs* fs = dent->inode->fs; + if (fs->d_ops && fs->d_ops->unlink) { + ret = fs->d_ops->unlink(dent); if (ret < 0) { goto out; } } - dent->state |= DENTRY_NEGATIVE; + put_inode(dent->inode); + dent->inode = NULL; + ret = 0; out: unlock(&g_dcache_lock); if (dir) @@ -127,21 +130,24 @@ long shim_do_rmdir(const char* pathname) { goto out; } - if (dent->type != S_IFDIR) { + if (dent->inode->type != S_IFDIR) { ret = -ENOTDIR; goto out; } - if (!dent->fs || !dent->fs->d_ops || !dent->fs->d_ops->unlink) { + struct shim_fs* fs = dent->inode->fs; + if (!fs || !fs->d_ops || !fs->d_ops->unlink) { ret = -EACCES; goto out; } - ret = dent->fs->d_ops->unlink(dent); + ret = fs->d_ops->unlink(dent); if (ret < 0) goto out; - dent->state |= DENTRY_NEGATIVE; + put_inode(dent->inode); + dent->inode = NULL; + ret = 0; out: unlock(&g_dcache_lock); if (dent) @@ -166,7 +172,7 @@ long shim_do_fchmodat(int dfd, const char* filename, mode_t mode) { return -EFAULT; /* This isn't documented, but that's what Linux does. */ - mode &= 07777; + mode_t perm = mode & 07777; struct shim_dentry* dir = NULL; struct shim_dentry* dent = NULL; @@ -180,12 +186,16 @@ long shim_do_fchmodat(int dfd, const char* filename, mode_t mode) { if (ret < 0) goto out; - if (dent->fs && dent->fs->d_ops && dent->fs->d_ops->chmod) { - if ((ret = dent->fs->d_ops->chmod(dent, mode)) < 0) + struct shim_fs* fs = dent->inode->fs; + if (fs && fs->d_ops && fs->d_ops->chmod) { + if ((ret = fs->d_ops->chmod(dent, perm)) < 0) goto out_dent; } - dent->perm = mode; + lock(&dent->inode->lock); + dent->inode->perm = perm; + unlock(&dent->inode->lock); + out_dent: put_dentry(dent); out: @@ -201,7 +211,7 @@ long shim_do_fchmod(int fd, mode_t mode) { return -EBADF; /* This isn't documented, but that's what Linux does. */ - mode &= 07777; + mode_t perm = mode & 07777; struct shim_dentry* dent = hdl->dentry; int ret = 0; @@ -212,21 +222,24 @@ long shim_do_fchmod(int fd, mode_t mode) { goto out; } - assert(dent->state & DENTRY_VALID); - if (dent->state & DENTRY_NEGATIVE) { + if (!dent->inode) { /* TODO: the `chmod` callback should take a handle, not dentry; otherwise we're not able to * chmod an unlinked file */ ret = -ENOENT; goto out; } - if (dent->fs && dent->fs->d_ops && dent->fs->d_ops->chmod) { - ret = dent->fs->d_ops->chmod(dent, mode); + struct shim_fs* fs = dent->inode->fs; + if (fs && fs->d_ops && fs->d_ops->chmod) { + ret = fs->d_ops->chmod(dent, perm); if (ret < 0) goto out; } - dent->perm = mode; + lock(&dent->inode->lock); + dent->inode->perm = perm; + unlock(&dent->inode->lock); + out: unlock(&g_dcache_lock); put_handle(hdl); @@ -280,36 +293,34 @@ long shim_do_fchown(int fd, uid_t uid, gid_t gid) { static int do_rename(struct shim_dentry* old_dent, struct shim_dentry* new_dent) { assert(locked(&g_dcache_lock)); + assert(old_dent->inode); - if ((old_dent->type != S_IFREG) || - (!(new_dent->state & DENTRY_NEGATIVE) && (new_dent->type != S_IFREG))) { + if ((old_dent->inode->type != S_IFREG) || (new_dent->inode && + new_dent->inode->type != S_IFREG)) { /* Current implementation of fs does not allow for renaming anything but regular files */ return -ENOSYS; } - if (old_dent->fs != new_dent->fs) { + if (old_dent->mount != new_dent->mount) { /* Disallow cross mount renames */ return -EXDEV; } - if (!old_dent->fs || !old_dent->fs->d_ops || !old_dent->fs->d_ops->rename) { + struct shim_fs* fs = old_dent->inode->fs; + if (!fs || !fs->d_ops || !fs->d_ops->rename) { return -EPERM; } - if (old_dent->type == S_IFDIR) { - if (!(new_dent->state & DENTRY_NEGATIVE)) { - if (new_dent->type != S_IFDIR) { + if (old_dent->inode->type == S_IFDIR) { + if (new_dent->inode) { + if (new_dent->inode->type != S_IFDIR) { return -ENOTDIR; } if (new_dent->nchildren > 0) { return -ENOTEMPTY; } - } else { - /* destination is a negative dentry and needs to be marked as a directory, since source - * is a directory */ - new_dent->type = S_IFDIR; } - } else if (new_dent->type == S_IFDIR) { + } else if (new_dent->inode && new_dent->inode->type == S_IFDIR) { return -EISDIR; } @@ -319,13 +330,15 @@ static int do_rename(struct shim_dentry* old_dent, struct shim_dentry* new_dent) /* TODO: Add appropriate checks for hardlinks once they get implemented. */ - int ret = old_dent->fs->d_ops->rename(old_dent, new_dent); - if (!ret) { - old_dent->state |= DENTRY_NEGATIVE; - new_dent->state &= ~DENTRY_NEGATIVE; - } + int ret = fs->d_ops->rename(old_dent, new_dent); + if (ret < 0) + return ret; - return ret; + if (new_dent->inode) + put_inode(new_dent->inode); + new_dent->inode = old_dent->inode; + old_dent->inode = NULL; + return 0; } long shim_do_rename(const char* oldpath, const char* newpath) { @@ -354,7 +367,7 @@ long shim_do_renameat(int olddirfd, const char* oldpath, int newdirfd, const cha goto out; } - if (old_dent->state & DENTRY_NEGATIVE) { + if (!old_dent->inode) { ret = -ENOENT; goto out; } diff --git a/LibOS/shim/src/sys/shim_getcwd.c b/LibOS/shim/src/sys/shim_getcwd.c index f5b4fd5fa5..0c875a272f 100644 --- a/LibOS/shim/src/sys/shim_getcwd.c +++ b/LibOS/shim/src/sys/shim_getcwd.c @@ -88,7 +88,7 @@ long shim_do_fchdir(int fd) { struct shim_dentry* dent = hdl->dentry; - if (dent->type != S_IFDIR) { + if (!dent->inode || dent->inode->type != S_IFDIR) { char* path = NULL; dentry_abs_path(dent, &path, /*size=*/NULL); log_debug("%s is not a directory", path); diff --git a/LibOS/shim/src/sys/shim_open.c b/LibOS/shim/src/sys/shim_open.c index cd642b03b3..965a51cb3d 100644 --- a/LibOS/shim/src/sys/shim_open.c +++ b/LibOS/shim/src/sys/shim_open.c @@ -369,7 +369,7 @@ static ssize_t do_getdents(int fd, uint8_t* buf, size_t buf_size, bool is_getden goto out_no_unlock; } - if (hdl->dentry->state & DENTRY_NEGATIVE) { + if (!hdl->dentry->inode) { ret = -ENOENT; goto out_no_unlock; } @@ -385,6 +385,8 @@ static ssize_t do_getdents(int fd, uint8_t* buf, size_t buf_size, bool is_getden assert(hdl->pos >= 0); while ((size_t)hdl->pos < dirhdl->count) { struct shim_dentry* dent = dirhdl->dents[hdl->pos]; + assert(dent->inode); + const char* name; size_t name_len; @@ -400,7 +402,7 @@ static ssize_t do_getdents(int fd, uint8_t* buf, size_t buf_size, bool is_getden } uint64_t d_ino = dentry_ino(dent); - char d_type = get_dirent_type(dent->type); + char d_type = get_dirent_type(dent->inode->type); size_t ent_size; diff --git a/LibOS/shim/src/sys/shim_pipe.c b/LibOS/shim/src/sys/shim_pipe.c index 2978868c2d..5e17c7818c 100644 --- a/LibOS/shim/src/sys/shim_pipe.c +++ b/LibOS/shim/src/sys/shim_pipe.c @@ -306,7 +306,7 @@ long shim_do_mknodat(int dirfd, const char* pathname, mode_t mode, dev_t dev) { goto out; } - if (!(dent->state & DENTRY_NEGATIVE)) { + if (dent->inode) { ret = -EEXIST; goto out; } @@ -365,11 +365,9 @@ long shim_do_mknodat(int dirfd, const char* pathname, mode_t mode, dev_t dev) { } /* set up the dentry for FIFO */ - reset_dentry(dent); ret = fifo_setup_dentry(dent, mode & ~S_IFMT, vfd1, vfd2); if (ret < 0) goto out; - dent->state |= DENTRY_VALID; ret = 0; out: diff --git a/LibOS/shim/src/sys/shim_socket.c b/LibOS/shim/src/sys/shim_socket.c index 8c2025b896..b016c23b13 100644 --- a/LibOS/shim/src/sys/shim_socket.c +++ b/LibOS/shim/src/sys/shim_socket.c @@ -456,11 +456,9 @@ static int get_unix_socket_dentry(const char* path, bool is_bind, struct shim_de if (ret < 0) goto out; - if (dent->state & DENTRY_NEGATIVE) { + if (!dent->inode) { /* No file exists, create one. */ - reset_dentry(dent); ret = unix_socket_setup_dentry(dent, PERM_rw_______); - dent->state |= DENTRY_VALID; } else if (is_bind) { /* The file exists and we're inside `bind()`. We should fail. */ ret = -EADDRINUSE; @@ -468,7 +466,7 @@ static int get_unix_socket_dentry(const char* path, bool is_bind, struct shim_de } else { /* The file exists and we're inside `connect()`. We should fail if the file is not a * socket. */ - if (dent->type != S_IFSOCK) { + if (dent->inode->type != S_IFSOCK) { ret = -ECONNREFUSED; goto out; } diff --git a/LibOS/shim/src/sys/shim_stat.c b/LibOS/shim/src/sys/shim_stat.c index 820a2d597f..db7829ca7b 100644 --- a/LibOS/shim/src/sys/shim_stat.c +++ b/LibOS/shim/src/sys/shim_stat.c @@ -17,8 +17,9 @@ static int do_stat(struct shim_dentry* dent, struct stat* stat) { assert(locked(&g_dcache_lock)); + assert(dent->inode); - struct shim_fs* fs = dent->fs; + struct shim_fs* fs = dent->inode->fs; if (!fs || !fs->d_ops || !fs->d_ops->stat) return -EACCES; @@ -135,13 +136,14 @@ long shim_do_readlinkat(int dirfd, const char* file, char* buf, int bufsize) { ret = -EINVAL; /* The correct behavior is to return -EINVAL if file is not a symbolic link */ - if (dent->type != S_IFLNK) + if (dent->inode->type != S_IFLNK) goto out; - if (!dent->fs || !dent->fs->d_ops || !dent->fs->d_ops->follow_link) + struct shim_fs* fs = dent->inode->fs; + if (!fs->d_ops || !fs->d_ops->follow_link) goto out; - ret = dent->fs->d_ops->follow_link(dent, &target); + ret = fs->d_ops->follow_link(dent, &target); if (ret < 0) goto out; @@ -230,12 +232,12 @@ static int do_fstatat_empty_path(int dirfd, struct stat* statbuf) { int ret; - if (dent->state & DENTRY_NEGATIVE) { + if (!dent->inode) { ret = -ENOENT; goto out; } - struct shim_d_ops* d_ops = dent->fs->d_ops; + struct shim_d_ops* d_ops = dent->inode->fs->d_ops; if (!(d_ops && d_ops->stat)) { ret = -EACCES; goto out; @@ -287,7 +289,7 @@ long shim_do_newfstatat(int dirfd, const char* pathname, struct stat* statbuf, i if (ret < 0) goto out; - struct shim_d_ops* d_ops = dent->fs->d_ops; + struct shim_d_ops* d_ops = dent->inode->fs->d_ops; if (!(d_ops && d_ops->stat)) { ret = -EACCES; goto out;