From 42bda0bf92634cc6e9f0518046611d2a5dba5257 Mon Sep 17 00:00:00 2001 From: Matheus Date: Mon, 26 Aug 2024 21:47:23 -0300 Subject: [PATCH] vfs: redo locking --- kernel-src/fs/devfs.c | 8 +- kernel-src/fs/ext2.c | 61 ++++-------- kernel-src/fs/initrd.c | 1 + kernel-src/fs/tmpfs.c | 79 +++++---------- kernel-src/fs/vfs.c | 141 ++++++++++++++++++++------- kernel-src/include/kernel/vfs.h | 7 +- kernel-src/io/net/localsocket.c | 11 ++- kernel-src/mm/vmm.c | 13 ++- kernel-src/mm/vmmcache.c | 6 ++ kernel-src/sys/defaultauth.c | 2 +- kernel-src/sys/scheduler.c | 8 +- kernel-src/sys/syscalls/chmod.c | 6 ++ kernel-src/sys/syscalls/chroot.c | 2 + kernel-src/sys/syscalls/execve.c | 10 +- kernel-src/sys/syscalls/faccessat.c | 1 + kernel-src/sys/syscalls/fchownat.c | 11 ++- kernel-src/sys/syscalls/getdent.c | 2 + kernel-src/sys/syscalls/ioctl.c | 2 + kernel-src/sys/syscalls/isatty.c | 2 + kernel-src/sys/syscalls/open.c | 20 ++-- kernel-src/sys/syscalls/pipe2.c | 2 + kernel-src/sys/syscalls/poll.c | 3 + kernel-src/sys/syscalls/pread.c | 4 - kernel-src/sys/syscalls/readlinkat.c | 2 + kernel-src/sys/syscalls/renameat.c | 7 +- kernel-src/sys/syscalls/seek.c | 2 + kernel-src/sys/syscalls/stat.c | 5 + kernel-src/sys/syscalls/sync.c | 2 + kernel-src/sys/syscalls/truncate.c | 5 +- kernel-src/sys/syscalls/unlinkat.c | 2 + kernel-src/sys/syscalls/utime.c | 10 +- kernel-src/sys/syscalls/write.c | 3 + 32 files changed, 271 insertions(+), 169 deletions(-) diff --git a/kernel-src/fs/devfs.c b/kernel-src/fs/devfs.c index e7364650..a0ef733c 100644 --- a/kernel-src/fs/devfs.c +++ b/kernel-src/fs/devfs.c @@ -120,9 +120,7 @@ int devfs_getattr(vnode_t *node, vattr_t *attr, cred_t *cred) { devnode_t *devnode = (devnode_t *)node; if (devnode->physical && devnode->physical != node) { - INTERNAL_LOCK(devnode->physical); int err = VOP_GETATTR(devnode->physical, attr, cred); - INTERNAL_UNLOCK(devnode->physical); return err; } @@ -451,6 +449,7 @@ int devfs_getbyname(char *name, vnode_t **ret) { if (error) goto cleanup; + VOP_UNLOCK(*ret); cleanup: return error; } @@ -480,7 +479,6 @@ int devfs_register(devops_t *devops, char *name, int type, int major, int minor, attr.uid = cred == NULL ? 0 : cred->euid; attr.gid = cred == NULL ? 0 : cred->egid; - vnode_t *dir = NULL; char lastcomp[strlen(name) + 1]; error = vfs_lookup(&dir, (vnode_t *)devfsroot, name, lastcomp, VFS_LOOKUP_PARENT); @@ -493,6 +491,8 @@ int devfs_register(devops_t *devops, char *name, int type, int major, int minor, // the devfs node will have a reference from both the devtable and from the fs link error = VOP_CREATE(dir, lastcomp, &attr, type, &newvnode, NULL); + // locked by vfs_lookup + VOP_UNLOCK(dir); VOP_RELEASE(dir); if (error) { @@ -507,6 +507,8 @@ int devfs_register(devops_t *devops, char *name, int type, int major, int minor, master->attr = newdevnode->attr; master->vnode.type = type; newdevnode->master = master; + // locked by VOP_CREATE + VOP_UNLOCK(newvnode); return 0; } diff --git a/kernel-src/fs/ext2.c b/kernel-src/fs/ext2.c index c4a7e97c..5f944478 100644 --- a/kernel-src/fs/ext2.c +++ b/kernel-src/fs/ext2.c @@ -1071,7 +1071,10 @@ static int ext2_open(vnode_t **vnodep, int flags, cred_t *cred) { if (error) return error; + VOP_LOCK(fifo); error = VOP_OPEN(&fifo, flags, cred); + VOP_UNLOCK(fifo); + if (error == 0) *vnodep = fifo; } @@ -1085,8 +1088,6 @@ static int ext2_close(vnode_t *vnode, int flags, cred_t *cred) { static int ext2_getattr(vnode_t *vnode, vattr_t *attr, cred_t *cred) { ext2node_t *node = (ext2node_t *)vnode; - VOP_LOCK(vnode); - attr->rdevmajor = 0; attr->rdevminor = 0; attr->devmajor = ((ext2fs_t *)vnode->vfs)->backingmajor; @@ -1106,15 +1107,11 @@ static int ext2_getattr(vnode_t *vnode, vattr_t *attr, cred_t *cred) { attr->mtime.s = node->inode.mtime; attr->mtime.ns = 0; attr->blocksused = ROUND_UP(attr->size, attr->fsblocksize) / attr->fsblocksize; - - VOP_UNLOCK(vnode); return 0; } static int ext2_setattr(vnode_t *vnode, vattr_t *attr, int which, cred_t *cred) { ext2node_t *node = (ext2node_t *)vnode; - VOP_LOCK(vnode); - if (which & V_ATTR_MODE) INODE_TYPEPERM_SETPERM(node->inode.typeperm, attr->mode); if (which & V_ATTR_GID) @@ -1128,8 +1125,6 @@ static int ext2_setattr(vnode_t *vnode, vattr_t *attr, int which, cred_t *cred) if (which & V_ATTR_ATIME) node->inode.atime = attr->atime.s; int e = writeinode((ext2fs_t *)vnode->vfs, &node->inode, node->id); - VOP_UNLOCK(vnode); - return e; } @@ -1137,8 +1132,6 @@ static int ext2_lookup(vnode_t *vnode, char *name, vnode_t **result, cred_t *cre ext2node_t *node = (ext2node_t *)vnode; ext2fs_t *fs = (ext2fs_t *)node->vnode.vfs; ext2node_t *newnode = NULL; - VOP_LOCK(vnode); - int inode = 0; int err = findindir(fs, node, name, &inode, NULL); if (err) @@ -1181,7 +1174,9 @@ static int ext2_lookup(vnode_t *vnode, char *name, vnode_t **result, cred_t *cre MUTEX_RELEASE(&fs->inodetablelock); + // VOP_LOOKUP returns the vnode locked *result = &newnode->vnode; + VOP_LOCK(*result); cleanup: if (err && newnode) { @@ -1189,8 +1184,6 @@ static int ext2_lookup(vnode_t *vnode, char *name, vnode_t **result, cred_t *cre VOP_RELEASE(tmp); MUTEX_RELEASE(&fs->inodetablelock); } - - VOP_UNLOCK(vnode); return err; } @@ -1201,8 +1194,6 @@ static int ext2_getdents(vnode_t *vnode, dent_t *buffer, size_t count, uintmax_t if (vnode->type != V_TYPE_DIR) return ENOTDIR; - VOP_LOCK(vnode); - int err = 0; // XXX loading the whole dir into memory at once isn't the best idea but works for now void *dirbuffer = vmm_map(NULL, INODE_SIZE(&node->inode) + 1, VMM_FLAGS_ALLOCATE, BUFFER_MAP_FLAGS, NULL); // + 1 to make sure there is at least one page mapped @@ -1242,7 +1233,6 @@ static int ext2_getdents(vnode_t *vnode, dent_t *buffer, size_t count, uintmax_t if (dirbuffer) vmm_unmap(dirbuffer, INODE_SIZE(&node->inode) + 1, 0); - VOP_UNLOCK(vnode); return err; } @@ -1256,7 +1246,6 @@ static int ext2_readlink(vnode_t *vnode, char **link, cred_t *cred) { if (vnode->type != V_TYPE_LINK) return EINVAL; - VOP_LOCK(vnode); int err = 0; size_t linksize = INODE_SIZE(&node->inode); @@ -1271,7 +1260,6 @@ static int ext2_readlink(vnode_t *vnode, char **link, cred_t *cred) { buf[linksize] = '\0'; - VOP_UNLOCK(vnode); if (err) free(buf); else @@ -1291,7 +1279,6 @@ static int ext2_read(vnode_t *vnode, void *buffer, size_t size, uintmax_t offset ext2fs_t *fs = (ext2fs_t *)vnode->vfs; uintmax_t endoffset = offset + size; int err = 0; - VOP_LOCK(vnode); size_t inodesize = INODE_SIZE(&node->inode); @@ -1317,7 +1304,6 @@ static int ext2_read(vnode_t *vnode, void *buffer, size_t size, uintmax_t offset *readc = err ? -1 : size; cleanup: - VOP_UNLOCK(vnode); return err; } @@ -1331,7 +1317,6 @@ int ext2_write(vnode_t *vnode, void *buffer, size_t size, uintmax_t offset, int ext2node_t *node = (ext2node_t *)vnode; ext2fs_t *fs = (ext2fs_t *)vnode->vfs; uintmax_t endoffset = offset + size; - VOP_LOCK(vnode); int err = 0; size_t inodesize = INODE_SIZE(&node->inode); @@ -1357,7 +1342,6 @@ int ext2_write(vnode_t *vnode, void *buffer, size_t size, uintmax_t offset, int //ASSERT_UNCLEAN(fs, writeinode(fs, &node->inode, node->id) == 0); cleanup: - VOP_UNLOCK(vnode); return err; } @@ -1371,7 +1355,6 @@ int ext2_resize(vnode_t *vnode, size_t newsize, cred_t *cred) { __assert(vnode->type == V_TYPE_REGULAR); ext2node_t *node = (ext2node_t *)vnode; ext2fs_t *fs = (ext2fs_t *)vnode->vfs; - VOP_LOCK(vnode); size_t size = INODE_SIZE(&node->inode); int e = 0; if (size != newsize) { @@ -1379,7 +1362,6 @@ int ext2_resize(vnode_t *vnode, size_t newsize, cred_t *cred) { if (size > newsize) vmmcache_truncate(vnode, newsize); } - VOP_UNLOCK(vnode); return e; } @@ -1414,11 +1396,7 @@ static int ext2_link(vnode_t *vnode, vnode_t *dirvnode, char *name, cred_t *cred ext2node_t *node = (ext2node_t *)vnode; ext2node_t *dirnode = (ext2node_t *)dirvnode; ext2fs_t *fs = (ext2fs_t *)dirvnode->vfs; - VOP_LOCK(dirvnode); - VOP_LOCK(vnode); int err = linkinternal(fs, dirnode, node, name); - VOP_UNLOCK(vnode); - VOP_UNLOCK(dirvnode); return err; } @@ -1482,6 +1460,9 @@ static int internalcreate(vnode_t *parent, char *name, vattr_t *attr, int type, ASSERT_UNCLEAN(fs, changedircount(fs, INODE_GETGROUP(fs, newnode->id), 1) == 0); } + // VOP_CREATE expects result to be locked + VOP_LOCK(*result); + cleanup: if (err && id) ASSERT_UNCLEAN(fs, freestructure(fs, id, true) == 0); @@ -1493,9 +1474,7 @@ static int internalcreate(vnode_t *parent, char *name, vattr_t *attr, int type, } static int ext2_create(vnode_t *parent, char *name, vattr_t *attr, int type, vnode_t **result, cred_t *cred) { - VOP_LOCK(parent); int err = internalcreate(parent, name, attr, type, result, cred); - VOP_UNLOCK(parent); return err; } @@ -1503,17 +1482,14 @@ static int ext2_symlink(vnode_t *vnode, char *name, vattr_t *attr, char *path, c ext2fs_t *fs = (ext2fs_t *)vnode->vfs; size_t linklen = strlen(path); - VOP_LOCK(vnode); vnode_t *newvnode; int err = internalcreate(vnode, name, attr, V_TYPE_LINK, &newvnode, cred); - if (err) { - VOP_UNLOCK(vnode); + if (err) return err; - } - // TODO no lock needed; (probably) not gonna be accessed by anyone else as the direcotry lock is held - VOP_LOCK(newvnode); + // no lock needed, not gonna be accessed by anyone else as the direcotry lock is held ext2node_t *newnode = (ext2node_t *)newvnode; + VOP_UNLOCK(newvnode); if (linklen <= 60) { // store it in the inode @@ -1537,8 +1513,6 @@ static int ext2_symlink(vnode_t *vnode, char *name, vattr_t *attr, char *path, c cleanup: if (err) ASSERT_UNCLEAN(fs, VOP_UNLINK(vnode, name, cred) == 0); - VOP_UNLOCK(newvnode); - VOP_UNLOCK(vnode); return err; } @@ -1618,14 +1592,13 @@ static int internalunlink(ext2fs_t *fs, ext2node_t *node, char *name) { return err; } +// TODO name -> child, name static int ext2_unlink(vnode_t *vnode, char *name, cred_t *cred) { ext2node_t *node = (ext2node_t *)vnode; ext2fs_t *fs = (ext2fs_t *)vnode->vfs; - VOP_LOCK(vnode); int err = internalunlink(fs, node, name); - VOP_UNLOCK(vnode); return err; } @@ -1664,6 +1637,7 @@ static int ext2_putpage(vnode_t *node, uintmax_t offset, struct page_t *page) { return error; } +// TODO redo this (newname -> targetvnode + newname) static int ext2_rename(vnode_t *sourcedir, char *oldname, vnode_t *targetdir, char *newname, int flags) { if (sourcedir->vfs != targetdir->vfs) return EXDEV; @@ -1680,12 +1654,13 @@ static int ext2_rename(vnode_t *sourcedir, char *oldname, vnode_t *targetdir, ch if (err) return err; + VOP_UNLOCK(source); + ext2fs_t *fs = (ext2fs_t *)targetdir->vfs; ext2node_t *ext2targetnode = (ext2node_t *)targetdir; ext2node_t *ext2sourcenode = (ext2node_t *)source; // switch out or create the target dirent - VOP_LOCK(targetdir); int oldinode; err = findindir(fs, ext2targetnode, newname, &oldinode, ext2sourcenode); if (err == ENOENT) { @@ -1699,13 +1674,10 @@ static int ext2_rename(vnode_t *sourcedir, char *oldname, vnode_t *targetdir, ch } if (err) { - VOP_UNLOCK(targetdir); VOP_RELEASE(source); return err; } - VOP_UNLOCK(targetdir); - VOP_LOCK(sourcedir); ext2node_t *ext2sourcedirnode = (ext2node_t *)sourcedir; // unlink the original dirent err = findindir(fs, ext2sourcedirnode, oldname, &oldinode, NULL); @@ -1718,7 +1690,6 @@ static int ext2_rename(vnode_t *sourcedir, char *oldname, vnode_t *targetdir, ch } cleanup: - VOP_UNLOCK(sourcedir); VOP_RELEASE(source); return err; } @@ -1824,7 +1795,9 @@ static int ext2_mount(vfs_t **vfs, vnode_t *mountpoint, vnode_t *backing, void * MUTEX_INIT(&fs->descriptorlock); vattr_t vattr; + VOP_LOCK(backing); int err = VOP_GETATTR(backing, &vattr, NULL); + VOP_UNLOCK(backing); if (err) goto cleanup; diff --git a/kernel-src/fs/initrd.c b/kernel-src/fs/initrd.c index c796d4ea..ccd629c9 100644 --- a/kernel-src/fs/initrd.c +++ b/kernel-src/fs/initrd.c @@ -157,6 +157,7 @@ void initrd_unpack() { if (err) break; + VOP_UNLOCK(node); err = vfs_write(node, datastart, entry.size, 0, &writecount, 0); VOP_RELEASE(node); break; diff --git a/kernel-src/fs/tmpfs.c b/kernel-src/fs/tmpfs.c index b26c3382..a5ef428f 100644 --- a/kernel-src/fs/tmpfs.c +++ b/kernel-src/fs/tmpfs.c @@ -37,18 +37,15 @@ static int tmpfs_mount(vfs_t **vfs, vnode_t *mountpoint, vnode_t *backing, void #define DATAMMUFLAGS (ARCH_MMU_FLAGS_WRITE | ARCH_MMU_FLAGS_READ | ARCH_MMU_FLAGS_NOEXEC) static int tmpfs_getattr(vnode_t *node, vattr_t *attr, cred_t *cred) { - VOP_LOCK(node); tmpfsnode_t *tmpnode = (tmpfsnode_t *)node; *attr = tmpnode->attr; attr->blocksused = ROUND_UP(attr->size, PAGE_SIZE) / PAGE_SIZE; attr->devmajor = 0; attr->devminor = ((tmpfs_t *)node->vfs)->id; - VOP_UNLOCK(node); return 0; } static int tmpfs_setattr(vnode_t *node, vattr_t *attr, int which, cred_t *cred) { - VOP_LOCK(node); tmpfsnode_t *tmpnode = (tmpfsnode_t *)node; if (which & V_ATTR_GID) @@ -64,7 +61,6 @@ static int tmpfs_setattr(vnode_t *node, vattr_t *attr, int which, cred_t *cred) if (which & V_ATTR_CTIME) tmpnode->attr.ctime = attr->ctime; - VOP_UNLOCK(node); return 0; } @@ -74,17 +70,17 @@ static int tmpfs_lookup(vnode_t *parent, char *name, vnode_t **result, cred_t *c if (parent->type != V_TYPE_DIR) return ENOTDIR; - VOP_LOCK(parent); void *r; int err = hashtable_get(&tmpparent->children, &r, name, strlen(name)); - if (err) { - VOP_UNLOCK(parent); + if (err) return err; - } child = r; VOP_HOLD(child); - VOP_UNLOCK(parent); + // VOP_LOOKUP is expected to return child locked + if (child != parent) + VOP_LOCK(child); + *result = child; return 0; } @@ -97,18 +93,13 @@ static int tmpfs_create(vnode_t *parent, char *name, vattr_t *attr, int type, vn size_t namelen = strlen(name); void *v; - VOP_LOCK(parent); - if (hashtable_get(&parenttmpnode->children, &v, name, namelen) == 0) { - VOP_UNLOCK(parent); + if (hashtable_get(&parenttmpnode->children, &v, name, namelen) == 0) return EEXIST; - } tmpfsnode_t *tmpnode = newnode(parent->vfs, type); vnode_t *node = (vnode_t *)tmpnode; - if (tmpnode == NULL) { - VOP_UNLOCK(parent); + if (tmpnode == NULL) return ENOMEM; - } timespec_t time = timekeeper_time(); @@ -122,23 +113,20 @@ static int tmpfs_create(vnode_t *parent, char *name, vattr_t *attr, int type, vn tmpnode->attr.nlinks = 1; if (error) { - VOP_UNLOCK(parent); VOP_RELEASE(node); - return error; + return error; } // if a dir, create the .. entry (the . entry is already made in newnode() if it has the dir type) if (type == V_TYPE_DIR) { error = hashtable_set(&tmpnode->children, parenttmpnode, "..", 2, true); if (error) { - VOP_UNLOCK(parent); VOP_RELEASE(node); return error; } } error = hashtable_set(&parenttmpnode->children, tmpnode, name, namelen, true); - VOP_UNLOCK(parent); if (error) { VOP_RELEASE(node); @@ -152,6 +140,8 @@ static int tmpfs_create(vnode_t *parent, char *name, vattr_t *attr, int type, vn node->vfs = parent->vfs; *result = node; VOP_HOLD(node); + // VOP_CREATE is expected to return child locked + VOP_LOCK(node); return error; } @@ -171,22 +161,28 @@ static int tmpfs_open(vnode_t **nodep, int flags, cred_t *cred) { if (error) return error; - VOP_RELEASE(node); - VOP_HOLD(devnode); - *nodep = devnode; + VOP_LOCK(devnode); + error = VOP_OPEN(&devnode, flags, cred); + VOP_UNLOCK(devnode); + if (error == 0) { + VOP_HOLD(devnode); + *nodep = devnode; + } } if (node->type == V_TYPE_FIFO) { - VOP_LOCK(*nodep); vnode_t *fifo; error = pipefs_getbinding(*nodep, &fifo); - VOP_UNLOCK(*nodep); if (error) return error; + VOP_LOCK(fifo); error = VOP_OPEN(&fifo, flags, cred); - if (error == 0) + VOP_UNLOCK(fifo); + if (error == 0) { *nodep = fifo; + VOP_HOLD(fifo); + } } return error; @@ -249,7 +245,6 @@ static int tmpfs_unlink(vnode_t *node, char *name, cred_t *cred) { size_t namelen = strlen(name); void *r; - VOP_LOCK(node); int err = hashtable_get(&tmpnode->children, &r, name, namelen); if (err) { @@ -261,7 +256,6 @@ static int tmpfs_unlink(vnode_t *node, char *name, cred_t *cred) { tmpfsnode_t *unlinktmpnode = r; err = hashtable_remove(&tmpnode->children, name, namelen); - VOP_UNLOCK(node); if (err) return err; @@ -286,14 +280,12 @@ static int tmpfs_link(vnode_t *node, vnode_t *dir, char *name, cred_t *cred) { tmpfsnode_t *tmpdir = (tmpfsnode_t *)dir; size_t namelen = strlen(name); void *v; - VOP_LOCK(dir); if (hashtable_get(&tmpdir->children, &v, name, namelen) == 0) { VOP_UNLOCK(node); return EEXIST; } int error = hashtable_set(&tmpdir->children, node, name, namelen, true); - VOP_UNLOCK(dir); if (error) return error; @@ -317,13 +309,6 @@ static int tmpfs_rename(vnode_t *source, char *oldname, vnode_t *target, char *n size_t oldlen = strlen(oldname); size_t newlen = strlen(newname); - if (target != source) { - VOP_LOCK(source); - VOP_LOCK(target); - } else { - VOP_LOCK(target); - } - // get original node void *v; int error = hashtable_get(&sourcedir->children, &v, oldname, oldlen); @@ -361,13 +346,6 @@ static int tmpfs_rename(vnode_t *source, char *oldname, vnode_t *target, char *n VOP_RELEASE(oldnode); cleanup: - if (target != source) { - VOP_UNLOCK(target); - VOP_UNLOCK(source); - } else { - VOP_UNLOCK(target); - } - return error; } @@ -388,9 +366,9 @@ static int tmpfs_symlink(vnode_t *node, char *name, vattr_t *attr, char *path, c return err; } - VOP_LOCK(linknode); tmpfsnode_t *linktmpnode = (tmpfsnode_t *)linknode; linktmpnode->link = pathbuf; + // locked by tmpfs_create VOP_UNLOCK(linknode); VOP_RELEASE(linknode); @@ -403,7 +381,6 @@ static int tmpfs_readlink(vnode_t *node, char **link, cred_t *cred) { tmpfsnode_t *tmpnode = (tmpfsnode_t *)node; - VOP_LOCK(node); char *ret = alloc(strlen(tmpnode->link) + 1); if (ret == NULL) { VOP_UNLOCK(node); @@ -413,7 +390,6 @@ static int tmpfs_readlink(vnode_t *node, char **link, cred_t *cred) { strcpy(ret, tmpnode->link); *link = ret; - VOP_UNLOCK(node); return 0; } @@ -432,8 +408,6 @@ static int tmpfs_getdents(vnode_t *node, dent_t *buffer, size_t count, uintmax_t if (node->type != V_TYPE_DIR) return ENOTDIR; - VOP_LOCK(node); - tmpfsnode_t *tmpnode = (tmpfsnode_t *)node; *readcount = 0; @@ -460,7 +434,6 @@ static int tmpfs_getdents(vnode_t *node, dent_t *buffer, size_t count, uintmax_t *readcount += 1; } - VOP_UNLOCK(node); return 0; } @@ -473,8 +446,6 @@ static int tmpfs_inactive(vnode_t *node) { } static int tmpfs_resize(vnode_t *node, size_t size, cred_t *) { - VOP_LOCK(node); - tmpfsnode_t *tmpfsnode = (tmpfsnode_t *)node; if (size != tmpfsnode->attr.size && tmpfsnode->vnode.type == V_TYPE_REGULAR) { if (tmpfsnode->attr.size > size) @@ -482,20 +453,16 @@ static int tmpfs_resize(vnode_t *node, size_t size, cred_t *) { tmpfsnode->attr.size = size; } - - VOP_UNLOCK(node); return 0; } static int tmpfs_getpage(vnode_t *node, uintmax_t offset, struct page_t *page) { int error = 0; - VOP_LOCK(node); tmpfsnode_t *tmpfsnode = (tmpfsnode_t *)node; if (offset >= tmpfsnode->attr.size) error = ENXIO; - VOP_UNLOCK(node); if (error) return error; diff --git a/kernel-src/fs/vfs.c b/kernel-src/fs/vfs.c index 39b94ee1..d149fccc 100644 --- a/kernel-src/fs/vfs.c +++ b/kernel-src/fs/vfs.c @@ -62,6 +62,7 @@ void vfs_inactive(vnode_t *vnode) { vnode->ops->inactive(vnode); } +// backing and pathref are expected to be unlocked int vfs_mount(vnode_t *backing, vnode_t *pathref, char *path, char *name, void *data) { vfsops_t *ops; int err = hashtable_get(&fstable, (void **)(&ops), name, strlen(name)); @@ -75,13 +76,16 @@ int vfs_mount(vnode_t *backing, vnode_t *pathref, char *path, char *name, void * return err; err = auth_filesystem_check(getcred(), AUTH_ACTIONS_FILESYSTEM_MOUNT, mounton); - if (err) { + // locked by vfs_lookup + VOP_UNLOCK(mounton); VOP_RELEASE(mounton); return err; } if (mounton->type != V_TYPE_DIR) { + // locked by vfs_lookup + VOP_UNLOCK(mounton); VOP_RELEASE(mounton); return ENOTDIR; } @@ -91,6 +95,8 @@ int vfs_mount(vnode_t *backing, vnode_t *pathref, char *path, char *name, void * err = ops->mount(&vfs, mounton, backing, data); if (err) { + // locked by vfs_lookup + VOP_UNLOCK(mounton); VOP_RELEASE(mounton); return err; } @@ -105,9 +111,13 @@ int vfs_mount(vnode_t *backing, vnode_t *pathref, char *path, char *name, void * mounton->vfsmounted = vfs; vfs->nodecovered = mounton; + // locked by vfs_lookup + VOP_UNLOCK(mounton); + return 0; } +// ref is expected to be unlocked int vfs_open(vnode_t *ref, char *path, int flags, vnode_t **res) { vnode_t *tmp = NULL; int err = vfs_lookup(&tmp, ref, path, NULL, 0); @@ -115,25 +125,37 @@ int vfs_open(vnode_t *ref, char *path, int flags, vnode_t **res) { return err; err = VOP_ACCESS(tmp, ((flags & V_FFLAGS_READ) ? V_ACCESS_READ : 0) | ((flags & V_FFLAGS_WRITE) ? V_ACCESS_WRITE : 0), getcred()); - if (err) + if (err) { + // locked by vfs_lookup + VOP_UNLOCK(tmp); return err; + } - err = VOP_OPEN(&tmp, flags, getcred()); + vnode_t *new = ref; + err = VOP_OPEN(&new, flags, getcred()); if (err) { VOP_RELEASE(tmp); } else { - *res = tmp; + *res = new != tmp ? new : tmp; } + // locked by vfs_lookup + VOP_UNLOCK(tmp); + return err; } +// node is expected to be unlocked int vfs_close(vnode_t *node, int flags) { + VOP_LOCK(node); int err = VOP_CLOSE(node, flags, getcred()); + VOP_UNLOCK(node); return err; } // if node is not NULL, then a reference is kept and the newnode is returned in node +// ref is expected to be unlocked. +// the new node is returned locked int vfs_create(vnode_t *ref, char *path, vattr_t *attr, int type, vnode_t **node) { vnode_t *parent; char *component = alloc(strlen(path) + 1); @@ -143,21 +165,29 @@ int vfs_create(vnode_t *ref, char *path, vattr_t *attr, int type, vnode_t **node err = VOP_ACCESS(parent, V_ACCESS_WRITE, getcred()); if (err) { + VOP_UNLOCK(parent); VOP_RELEASE(parent); - goto cleanup; + goto cleanup_parent; } vnode_t *ret; err = VOP_CREATE(parent, component, attr, type, &ret, getcred()); VOP_RELEASE(parent); - if (err) + if (err) { + VOP_UNLOCK(parent); goto cleanup; + } - if (node) + if (node) { *node = ret; - else + } else { + // returned locked by VOP_CREATE + VOP_UNLOCK(ret); VOP_RELEASE(ret); + } + cleanup_parent: + VOP_UNLOCK(parent); cleanup: free(component); return err; @@ -188,12 +218,13 @@ static int writenocache(vnode_t *node, page_t *page, uintmax_t pageoffset) { } int vfs_write(vnode_t *node, void *buffer, size_t size, uintmax_t offset, size_t *written, int flags) { - int err; + VOP_LOCK(node); + int err = 0; if (node->type == V_TYPE_REGULAR || node->type == V_TYPE_BLKDEV) { *written = 0; // can't write a size 0 buffer if (size == 0) - return 0; + goto leave; // overflow if (size + offset < offset) { @@ -201,8 +232,6 @@ int vfs_write(vnode_t *node, void *buffer, size_t size, uintmax_t offset, size_t goto leave; } - MUTEX_ACQUIRE(&node->sizelock, false); - vattr_t attr; err = VOP_GETATTR(node, &attr, getcred()); if (err) @@ -248,12 +277,12 @@ int vfs_write(vnode_t *node, void *buffer, size_t size, uintmax_t offset, size_t pageoffset += 1; pagecount -= 1; - if (flags & V_FFLAGS_NOCACHE) { + if (flags & V_FFLAGS_NOCACHE) err = writenocache(node, page, pageoffset * PAGE_SIZE); - if (err) - goto leave; - } + pmm_release(FROM_HHDM(address)); + if (err) + goto leave; } for (uintmax_t offset = 0; offset < pagecount * PAGE_SIZE; offset += PAGE_SIZE) { @@ -268,31 +297,32 @@ int vfs_write(vnode_t *node, void *buffer, size_t size, uintmax_t offset, size_t vmmcache_makedirty(page); *written += writesize; - if (flags & V_FFLAGS_NOCACHE) { + if (flags & V_FFLAGS_NOCACHE) err = writenocache(node, page, pageoffset * PAGE_SIZE + offset); - if (err) - goto leave; - } - pmm_release(FROM_HHDM(address)); - } - leave: - MUTEX_RELEASE(&node->sizelock); + pmm_release(FROM_HHDM(address)); + + if (err) + goto leave; + } } else { // special file, just write as its not being cached err = VOP_WRITE(node, buffer, size, offset, flags, written, getcred()); } + leave: + VOP_UNLOCK(node); return err; } int vfs_read(vnode_t *node, void *buffer, size_t size, uintmax_t offset, size_t *bytesread, int flags) { - int err; + VOP_LOCK(node); + int err = 0; if (node->type == V_TYPE_REGULAR || node->type == V_TYPE_BLKDEV) { *bytesread = 0; // can't read 0 bytes from the cache if (size == 0) - return 0; + goto leave; // overflow if (size + offset < offset) { @@ -300,7 +330,6 @@ int vfs_read(vnode_t *node, void *buffer, size_t size, uintmax_t offset, size_t goto leave; } - MUTEX_ACQUIRE(&node->sizelock, false); size_t nodesize = 0; if (node->type == V_TYPE_REGULAR) { @@ -365,13 +394,12 @@ int vfs_read(vnode_t *node, void *buffer, size_t size, uintmax_t offset, size_t } pmm_release(FROM_HHDM(address)); } - - leave: - MUTEX_RELEASE(&node->sizelock); } else { // special file, just read as size doesn't matter err = VOP_READ(node, buffer, size, offset, flags, bytesread, getcred()); } + leave: + VOP_UNLOCK(node); return err; } @@ -389,22 +417,31 @@ int vfs_link(vnode_t *destref, char *destpath, vnode_t *linkref, char *linkpath, goto cleanup; err = VOP_ACCESS(parent, V_ACCESS_WRITE, getcred()); - if (err) + if (err) { + // locked by vfs_lookup + VOP_UNLOCK(parent); goto cleanup; + } + // TODO rewrite this as it can deadlock as a directory is locked if (type == V_TYPE_REGULAR) { vnode_t *targetnode = NULL; err = vfs_lookup(&targetnode, destref, destpath, NULL, 0); if (err) { - VOP_RELEASE(parent); + // locked by vfs_lookup + VOP_UNLOCK(parent); goto cleanup; } err = VOP_LINK(targetnode, parent, component, getcred()); + // locked by vfs_lookup + VOP_UNLOCK(targetnode); VOP_RELEASE(targetnode); } else { err = VOP_SYMLINK(parent, component, attr, destpath, getcred()); } + // locked by vfs_lookup + VOP_UNLOCK(parent); cleanup: VOP_RELEASE(parent); free(component); @@ -419,6 +456,8 @@ int vfs_unlink(vnode_t *ref, char *path) { goto cleanup; VOP_UNLINK(parent, component, getcred()); + // locked by vfs_lookup + VOP_UNLOCK(parent); VOP_RELEASE(parent); cleanup: @@ -446,6 +485,8 @@ static vnode_t *lowestnodeinmp(vnode_t *node) { } // looks up a pathname +// result will always be returned locked. +// if flags & VFS_LOOKUP_INPUTLOCKED, the input is already locked and will be locked on return // if flags & VFS_LOOKUP_PARENT, the parent of the resulting node will be put in // the result pointer and lastcomp will contain the last component name // if flags & VFS_LOOKUP_NOLINK, the last component will not be dereferenced if its a symbolic link @@ -462,10 +503,15 @@ int vfs_lookup(vnode_t **result, vnode_t *start, char *path, char *lastcomp, int return ENOENT; } - vnode_t *r = start; + vnode_t *r = start; int e = highestnodeinmp(start, &r); if (e) return e; + + if ((flags & VFS_LOOKUP_INPUTLOCKED) == 0) { + VOP_LOCK(r); + } + VOP_HOLD(r); *result = r; return 0; @@ -494,6 +540,9 @@ int vfs_lookup(vnode_t **result, vnode_t *start, char *path, char *lastcomp, int error = 0; VOP_HOLD(current); + if ((flags & VFS_LOOKUP_INPUTLOCKED) == 0 || current != start) + VOP_LOCK(current); + for (int i = 0; i < pathlen; ++i) { if (compbuffer[i] == '\0') continue; @@ -510,7 +559,7 @@ int vfs_lookup(vnode_t **result, vnode_t *start, char *path, char *lastcomp, int // check if its last with trailing '/' (or '\0's in this case, as '/'s were turned into '\0's in the buffer) if (!islast) { int j; - for (j = i + complen; j < pathlen && compbuffer[j] == '\0'; ++j); + for (j = i + complen; j < pathlen && compbuffer[j] == '\0'; ++j) asm(""); islast = j == pathlen; } @@ -528,17 +577,21 @@ int vfs_lookup(vnode_t **result, vnode_t *start, char *path, char *lastcomp, int i += complen; continue; } + // if the root of a mounted fs, go to if (current->flags & V_FLAGS_ROOT) { vnode_t *n = lowestnodeinmp(current); if (n != current) { VOP_HOLD(n); + VOP_UNLOCK(current); VOP_RELEASE(current); current = n; + VOP_LOCK(current); } } } + // locked at the beggining of the function if nescessary or by a previous VOP_LOOKUP error = VOP_ACCESS(current, V_ACCESS_SEARCH, getcred()); if (error) break; @@ -547,24 +600,35 @@ int vfs_lookup(vnode_t **result, vnode_t *start, char *path, char *lastcomp, int if (error) break; + if ((current != start || (flags & VFS_LOOKUP_INPUTLOCKED) == 0) && current != next) + VOP_UNLOCK(current); + vnode_t *r = next; error = highestnodeinmp(next, &r); if (error) { + if (current != next) + VOP_UNLOCK(next); + VOP_RELEASE(next); break; } if (r != next) { VOP_HOLD(r); + VOP_UNLOCK(next); VOP_RELEASE(next); next = r; + VOP_LOCK(next); } // dereference symlink if (next->type == V_TYPE_LINK && (islast == false || (islast == true && (flags & VFS_LOOKUP_NOLINK) == 0))) { // get path char *linkderef; + + // next is already returned locked by VOP_LOOKUP error = VOP_READLINK(next, &linkderef, getcred()); + VOP_UNLOCK(next); if (error) { VOP_RELEASE(next); break; @@ -587,6 +651,7 @@ int vfs_lookup(vnode_t **result, vnode_t *start, char *path, char *lastcomp, int VOP_HOLD(derefstart); } + // TODO possible deadlock when input vnode is locked and the vfs_lookup would lock it error = vfs_lookup(&derefresult, derefstart, linkderef, NULL, pass); if (*linkderef == '/') @@ -595,8 +660,9 @@ int vfs_lookup(vnode_t **result, vnode_t *start, char *path, char *lastcomp, int free(linkderef); VOP_RELEASE(next); - if (error) + if (error) { break; + } next = derefresult; } @@ -607,8 +673,13 @@ int vfs_lookup(vnode_t **result, vnode_t *start, char *path, char *lastcomp, int } if (error) { + // current is locked here, unlock it if not the start or start wasnt locked + if (current != start || (flags & VFS_LOOKUP_INPUTLOCKED) == 0) + VOP_UNLOCK(current); + VOP_RELEASE(current); } else { + // result is already locked here *result = current; } diff --git a/kernel-src/include/kernel/vfs.h b/kernel-src/include/kernel/vfs.h index b93aad9c..dbe5cda1 100644 --- a/kernel-src/include/kernel/vfs.h +++ b/kernel-src/include/kernel/vfs.h @@ -220,9 +220,10 @@ int vfs_unlink(vnode_t *ref, char *path); int vfs_pollstub(vnode_t *node, struct polldata *, int events); void vfs_inactive(vnode_t *node); -#define VFS_LOOKUP_PARENT 0x20000000 -#define VFS_LOOKUP_NOLINK 0x40000000 -#define VFS_LOOKUP_INTERNAL 0x80000000 +#define VFS_LOOKUP_INPUTLOCKED 0x10000000 +#define VFS_LOOKUP_PARENT 0x20000000 +#define VFS_LOOKUP_NOLINK 0x40000000 +#define VFS_LOOKUP_INTERNAL 0x80000000 int vfs_lookup(vnode_t **result, vnode_t *start, char *path, char *lastcomp, int flags); static inline mode_t vfs_getposixtype(int type) { diff --git a/kernel-src/io/net/localsocket.c b/kernel-src/io/net/localsocket.c index 1e20a2ce..ceb89929 100644 --- a/kernel-src/io/net/localsocket.c +++ b/kernel-src/io/net/localsocket.c @@ -413,16 +413,23 @@ static int localsock_connect(socket_t *socket, sockaddr_t *addr, uintmax_t flags // check if its a socket node if (result->type != V_TYPE_SOCKET) { error = ENOTSOCK; + // locked by vfs_lookup + VOP_UNLOCK(result); goto leave; } // and we have write access error = VOP_ACCESS(result, V_ACCESS_WRITE, cred); - if (error) + if (error) { + // locked by vfs_lookup + VOP_UNLOCK(result); goto leave; + } // and if it has a valid binding binding = result->socketbinding; + // locked by vfs_lookup + VOP_UNLOCK(result); if (binding == NULL) { error = ECONNREFUSED; goto leave; @@ -690,8 +697,8 @@ static int localsock_bind(socket_t *socket, sockaddr_t *addr) { binding->server = localsocket; MUTEX_INIT(&binding->mutex); - VOP_LOCK(vnode); vnode->socketbinding = binding; + // locked by vfs_create VOP_UNLOCK(vnode); localsocket->binding = binding; localsocket->bindpath = path; diff --git a/kernel-src/mm/vmm.c b/kernel-src/mm/vmm.c index b661f6e1..92b891f6 100644 --- a/kernel-src/mm/vmm.c +++ b/kernel-src/mm/vmm.c @@ -231,11 +231,15 @@ static void destroyrange(vmmrange_t *range, uintmax_t _offset, size_t size, int // shared file mapping or character device mapping if (range->vnode->type == V_TYPE_CHDEV) { // character device mapping + VOP_LOCK(range->vnode); __assert(VOP_MUNMAP(range->vnode, vaddr, range->offset + offset, mmuflagstovnodeflags(range->mmuflags) | (range->flags & VMM_FLAGS_SHARED ? V_FFLAGS_SHARED : 0), cred) == 0); + VOP_UNLOCK(range->vnode); } else if (arch_mmu_iswritable(_cpu()->vmmctx->pagetable, vaddr) && arch_mmu_isdirty(_cpu()->vmmctx->pagetable, vaddr)) { // dirty page cache mapping arch_mmu_unmap(_cpu()->vmmctx->pagetable, vaddr); + VOP_LOCK(range->vnode); vmmcache_makedirty(pmm_getpage(physical)); + VOP_UNLOCK(range->vnode); pmm_release(physical); } else { // non dirty page mapping @@ -499,12 +503,16 @@ bool vmm_pagefault(void *addr, bool user, int actions) { uintmax_t mapoffset = (uintptr_t)addr - (uintptr_t)range->start; if (range->vnode->type != V_TYPE_BLKDEV && range->vnode->type != V_TYPE_REGULAR) { // map non cacheable vnodes + VOP_LOCK(range->vnode); __assert(VOP_MMAP(range->vnode, addr, range->offset + mapoffset, mmuflagstovnodeflags(range->mmuflags) | (range->flags & VMM_FLAGS_SHARED ? V_FFLAGS_SHARED : 0), cred) == 0); + VOP_UNLOCK(range->vnode); status = true; } else { // cacheable vnode page_t *res = NULL; + VOP_LOCK(range->vnode); int error = vmmcache_getpage(range->vnode, range->offset + mapoffset, &res); + VOP_UNLOCK(range->vnode); if (error == ENXIO || error == ENOMEM) { if (error == ENOMEM) printf("vmm: out of memory to handle getpage (sending SIGBUS)\n"); @@ -540,8 +548,11 @@ bool vmm_pagefault(void *addr, bool user, int actions) { void *oldphys = arch_mmu_getphysical(_cpu()->vmmctx->pagetable, addr); if ((range->flags & VMM_FLAGS_FILE) && (range->flags & VMM_FLAGS_SHARED)) { arch_mmu_remap(_cpu()->vmmctx->pagetable, oldphys, addr, range->mmuflags); - if (range->vnode->type != V_TYPE_CHDEV) + if (range->vnode->type != V_TYPE_CHDEV) { + VOP_LOCK(range->vnode); vmmcache_makedirty(pmm_getpage(oldphys)); + VOP_UNLOCK(range->vnode); + } status = true; } else { diff --git a/kernel-src/mm/vmmcache.c b/kernel-src/mm/vmmcache.c index f316060b..5afb76be 100644 --- a/kernel-src/mm/vmmcache.c +++ b/kernel-src/mm/vmmcache.c @@ -102,6 +102,7 @@ static void removepage(page_t *page) { --vmmcache_cachedpages; } +// vnode expected locked int vmmcache_getpage(vnode_t *vnode, uintmax_t offset, page_t **res) { __assert(vnode->type == V_TYPE_REGULAR || vnode->type == V_TYPE_BLKDEV); __assert((offset % PAGE_SIZE) == 0); @@ -292,6 +293,7 @@ int vmmcache_truncate(vnode_t *vnode, uintmax_t offset) { // called with lock held // returns with lock released +// expects backing lock to be held static int syncpage(page_t *page) { __assert(page->flags & PAGE_FLAGS_DIRTY); page->flags &= ~PAGE_FLAGS_DIRTY; @@ -312,6 +314,7 @@ static int syncpage(page_t *page) { static page_t *dirtylist; +// expects vnode to be held int vmmcache_syncvnode(vnode_t *vnode, uintmax_t offset, size_t size) { offset = ROUND_DOWN(offset, PAGE_SIZE); uintmax_t top = offset + size; @@ -381,6 +384,7 @@ int vmmcache_sync() { return 0; } +// backing expected locked int vmmcache_makedirty(page_t *page) { bool madedirty = false; HOLD_LOCK(); @@ -437,7 +441,9 @@ static void writer() { page->writenext = NULL; // TODO notify error on vmmcache_syncvnode + VOP_LOCK(page->backing); syncpage((page_t *)page); + VOP_UNLOCK(page->backing); } } diff --git a/kernel-src/sys/defaultauth.c b/kernel-src/sys/defaultauth.c index 4a8ab2fd..765d76e9 100644 --- a/kernel-src/sys/defaultauth.c +++ b/kernel-src/sys/defaultauth.c @@ -9,7 +9,7 @@ } static int filesystem(cred_t *cred, int actions, void *arg0, void *arg1, void *arg2) { - vnode_t *vnode = arg0; + vnode_t *vnode = arg0; // expected locked vattr_t attr; VOP_GETATTR(vnode, &attr, NULL); diff --git a/kernel-src/sys/scheduler.c b/kernel-src/sys/scheduler.c index bd821ea2..4240ce09 100644 --- a/kernel-src/sys/scheduler.c +++ b/kernel-src/sys/scheduler.c @@ -760,11 +760,13 @@ void sched_runinit() { vnode_t *consolenode; __assert(devfs_getbyname("console", &consolenode) == 0); - __assert(VOP_OPEN(&consolenode, V_FFLAGS_READ | V_FFLAGS_NOCTTY, &proc->cred) == 0) + VOP_LOCK(consolenode); + __assert(VOP_OPEN(&consolenode, V_FFLAGS_READ | V_FFLAGS_NOCTTY, &proc->cred) == 0); VOP_HOLD(consolenode); - __assert(VOP_OPEN(&consolenode, V_FFLAGS_WRITE | V_FFLAGS_NOCTTY, &proc->cred) == 0) + __assert(VOP_OPEN(&consolenode, V_FFLAGS_WRITE | V_FFLAGS_NOCTTY, &proc->cred) == 0); VOP_HOLD(consolenode); - __assert(VOP_OPEN(&consolenode, V_FFLAGS_WRITE | V_FFLAGS_NOCTTY, &proc->cred) == 0) + __assert(VOP_OPEN(&consolenode, V_FFLAGS_WRITE | V_FFLAGS_NOCTTY, &proc->cred) == 0); + VOP_UNLOCK(consolenode); file_t *stdin = fd_allocate(); file_t *stdout = fd_allocate(); diff --git a/kernel-src/sys/syscalls/chmod.c b/kernel-src/sys/syscalls/chmod.c index 587c597b..2d4c1644 100644 --- a/kernel-src/sys/syscalls/chmod.c +++ b/kernel-src/sys/syscalls/chmod.c @@ -5,6 +5,7 @@ #include #include +// node expected to be locked static int dochmod(vnode_t *node, mode_t mode) { vattr_t attr; cred_t *cred = &_cpu()->thread->proc->cred; @@ -58,6 +59,9 @@ syscallret_t syscall_fchmodat(context_t *, int dirfd, char *upath, mode_t mode, ret.errno = dochmod(node, mode); ret.ret = ret.errno ? -1 : 0; + // locked by vfs_lookup + VOP_UNLOCK(node); + cleanup: if (node) @@ -82,8 +86,10 @@ syscallret_t syscall_fchmod(context_t *, int fd, mode_t mode) { return ret; } + VOP_LOCK(file->vnode); ret.errno = dochmod(file->vnode, mode); ret.ret = ret.errno ? -1 : 0; + VOP_UNLOCK(file->vnode); fd_release(file); diff --git a/kernel-src/sys/syscalls/chroot.c b/kernel-src/sys/syscalls/chroot.c index 89ad49fc..9f0d9232 100644 --- a/kernel-src/sys/syscalls/chroot.c +++ b/kernel-src/sys/syscalls/chroot.c @@ -38,6 +38,8 @@ syscallret_t syscall_chroot(context_t *, char *upath) { if (ret.errno) goto cleanup; + VOP_UNLOCK(new); + if (new->type != V_TYPE_DIR) { ret.errno = ENOTDIR; goto cleanup; diff --git a/kernel-src/sys/syscalls/execve.c b/kernel-src/sys/syscalls/execve.c index 7326c726..f475ea30 100644 --- a/kernel-src/sys/syscalls/execve.c +++ b/kernel-src/sys/syscalls/execve.c @@ -178,8 +178,10 @@ static syscallret_t execve(context_t *context, char *upath, char *uargv[], char goto error; ret.errno = VOP_ACCESS(node, V_ACCESS_EXECUTE, &_cpu()->thread->proc->cred); - if (ret.errno) + VOP_UNLOCK(node); + if (ret.errno) { goto error; + } size_t readcount; ret.errno = vfs_read(node, shebang, SHEBANG_SIZE - 1, 0, &readcount, 0); @@ -229,8 +231,6 @@ static syscallret_t execve(context_t *context, char *upath, char *uargv[], char goto error; } - // TODO permission and type checks and all - vmm_switchcontext(vmmctx); auxv64list_t auxv64; @@ -246,6 +246,8 @@ static syscallret_t execve(context_t *context, char *upath, char *uargv[], char if (ret.errno) goto error; + VOP_UNLOCK(interpnode); + auxv64list_t interpauxv; char *interpinterp = NULL; ret.errno = elf_load(interpnode, INTERP_BASE, &entry, &interpinterp, &interpauxv); @@ -263,7 +265,9 @@ static syscallret_t execve(context_t *context, char *upath, char *uargv[], char } vattr_t vattr; + VOP_LOCK(node); ret.errno = VOP_GETATTR(node, &vattr, NULL); + VOP_UNLOCK(node); if (ret.errno) goto error; diff --git a/kernel-src/sys/syscalls/faccessat.c b/kernel-src/sys/syscalls/faccessat.c index a9231b4d..3dbcda86 100644 --- a/kernel-src/sys/syscalls/faccessat.c +++ b/kernel-src/sys/syscalls/faccessat.c @@ -39,6 +39,7 @@ syscallret_t syscall_faccessat(context_t *, int dirfd, char *upath, int mode, in goto cleanup; ret.errno = VOP_ACCESS(node, mode, &_cpu()->thread->proc->cred); + VOP_UNLOCK(node); ret.ret = 0; cleanup: diff --git a/kernel-src/sys/syscalls/fchownat.c b/kernel-src/sys/syscalls/fchownat.c index b456f914..914468c5 100644 --- a/kernel-src/sys/syscalls/fchownat.c +++ b/kernel-src/sys/syscalls/fchownat.c @@ -27,6 +27,7 @@ syscallret_t syscall_fchownat(context_t *, int fd, char *upath, uid_t owner, gid node = file->vnode; VOP_HOLD(node); + VOP_LOCK(node); fd_release(file); } else { // chown is done on the file pointed to by the path relative to fd @@ -50,13 +51,17 @@ syscallret_t syscall_fchownat(context_t *, int fd, char *upath, uid_t owner, gid } ret.errno = auth_filesystem_check(&_cpu()->thread->proc->cred, AUTH_ACTIONS_FILESYSTEM_SETATTR, node); - if (ret.errno) + if (ret.errno) { + VOP_UNLOCK(node); goto cleanup; + } vattr_t attr; ret.errno = VOP_GETATTR(node, &attr, &_cpu()->thread->proc->cred); - if (ret.errno) + if (ret.errno) { + VOP_UNLOCK(node); goto cleanup; + } // TODO retain set uid and set gid bits if an auth access was ok attr.uid = owner; @@ -66,6 +71,8 @@ syscallret_t syscall_fchownat(context_t *, int fd, char *upath, uid_t owner, gid ret.errno = VOP_SETATTR(node, &attr, ((owner == -1) ? 0 : V_ATTR_UID) | ((group == -1) ? 0 : V_ATTR_GID), &_cpu()->thread->proc->cred); ret.ret = ret.errno ? -1 : 0; + VOP_UNLOCK(node); + cleanup: if (node) VOP_RELEASE(node); diff --git a/kernel-src/sys/syscalls/getdent.c b/kernel-src/sys/syscalls/getdent.c index 445a7d71..d341804a 100644 --- a/kernel-src/sys/syscalls/getdent.c +++ b/kernel-src/sys/syscalls/getdent.c @@ -35,7 +35,9 @@ syscallret_t syscall_getdents(context_t *, int dirfd, void *ubuffer, size_t read } size_t offset = fd->offset; + VOP_LOCK(fd->vnode); ret.errno = VOP_GETDENTS(fd->vnode, buffer, count, offset, &ret.ret); + VOP_UNLOCK(fd->vnode); if (ret.errno) goto cleanup; diff --git a/kernel-src/sys/syscalls/ioctl.c b/kernel-src/sys/syscalls/ioctl.c index 1194c39e..462ff0d8 100644 --- a/kernel-src/sys/syscalls/ioctl.c +++ b/kernel-src/sys/syscalls/ioctl.c @@ -19,7 +19,9 @@ syscallret_t syscall_ioctl(context_t *, int fd, unsigned long request, void *arg } int r = 0; + VOP_LOCK(file->vnode); ret.errno = VOP_IOCTL(file->vnode, request, arg, &r); + VOP_UNLOCK(file->vnode); ret.ret = ret.errno ? -1 : r; fd_release(file); diff --git a/kernel-src/sys/syscalls/isatty.c b/kernel-src/sys/syscalls/isatty.c index 3d4b39ad..32890989 100644 --- a/kernel-src/sys/syscalls/isatty.c +++ b/kernel-src/sys/syscalls/isatty.c @@ -14,7 +14,9 @@ syscallret_t syscall_isatty(context_t *context, int fd) { } // mlibc expects isatty to return 1 if a terminal and 0 if not. + VOP_LOCK(file->vnode); int e = VOP_ISATTY(file->vnode); + VOP_UNLOCK(file->vnode); ret.errno = e; ret.ret = e ? 0 : 1; diff --git a/kernel-src/sys/syscalls/open.c b/kernel-src/sys/syscalls/open.c index 4c56ad95..11949deb 100644 --- a/kernel-src/sys/syscalls/open.c +++ b/kernel-src/sys/syscalls/open.c @@ -43,16 +43,16 @@ syscallret_t syscall_openat(context_t *context, int dirfd, char *path, int flags int newfd; ret.errno = fd_new(flags & O_CLOEXEC, &newfile, &newfd); if (ret.errno) - goto cleanup; + goto cleanup_nounlock; vnode_t *vnode = NULL; ret.errno = vfs_open(dirnode, pathbuf, fileflagstovnodeflags(flags), &vnode); if (ret.errno == 0 && (flags & O_CREAT) && (flags & O_EXCL)) { + // exclusive and file already exists ret.errno = EEXIST; - return ret; - } - - if (ret.errno == ENOENT && (flags & O_CREAT)) { + goto cleanup_nounlock; + } else if (ret.errno == ENOENT && (flags & O_CREAT)) { + // create file vattr_t attr = { .mode = UMASK(mode), .gid = _cpu()->thread->proc->cred.egid, @@ -62,6 +62,12 @@ syscallret_t syscall_openat(context_t *context, int dirfd, char *path, int flags ret.errno = vfs_create(dirnode, pathbuf, &attr, V_TYPE_REGULAR, &vnode); if (ret.errno == EEXIST && (flags & O_EXCL) == 0) goto retry_open; + } else if (ret.errno) { + // any other error + goto cleanup_nounlock; + } else { + // success, lock file + VOP_LOCK(vnode); } if (ret.errno) @@ -78,9 +84,7 @@ syscallret_t syscall_openat(context_t *context, int dirfd, char *path, int flags goto cleanup; if (vnode->type == V_TYPE_REGULAR && (flags & O_TRUNC) && (flags & FILE_WRITE)) { - MUTEX_ACQUIRE(&vnode->sizelock, false); ret.errno = VOP_RESIZE(vnode, 0, &_cpu()->thread->proc->cred); - MUTEX_RELEASE(&vnode->sizelock); if (ret.errno) goto cleanup; } @@ -95,6 +99,8 @@ syscallret_t syscall_openat(context_t *context, int dirfd, char *path, int flags ret.ret = newfd; cleanup: + VOP_UNLOCK(vnode); + cleanup_nounlock: if (vnode && ret.errno) { vfs_close(vnode, fileflagstovnodeflags(flags)); VOP_RELEASE(vnode); diff --git a/kernel-src/sys/syscalls/pipe2.c b/kernel-src/sys/syscalls/pipe2.c index c2122331..70626e30 100644 --- a/kernel-src/sys/syscalls/pipe2.c +++ b/kernel-src/sys/syscalls/pipe2.c @@ -11,7 +11,9 @@ static void open(file_t *file, vnode_t *node, int flags) { file->flags = flags; file->offset = 0; file->mode = 0777; + VOP_LOCK(node); VOP_OPEN(&node, fileflagstovnodeflags(flags) | V_FFLAGS_NONBLOCKING, &_cpu()->thread->proc->cred); + VOP_UNLOCK(node); } syscallret_t syscall_pipe2(context_t *, int flags) { diff --git a/kernel-src/sys/syscalls/poll.c b/kernel-src/sys/syscalls/poll.c index d21f268e..69cb949b 100644 --- a/kernel-src/sys/syscalls/poll.c +++ b/kernel-src/sys/syscalls/poll.c @@ -62,7 +62,10 @@ syscallret_t syscall_poll(context_t *, pollfd_t *fds, size_t nfds, int timeoutms filebuff[i] = file; + VOP_LOCK(file->vnode); int revents = VOP_POLL(file->vnode, timeoutms == 0 ? NULL : &desc.data[i], fdsbuff[i].events); + VOP_UNLOCK(file->vnode); + if (revents) { fdsbuff[i].revents = revents; ++eventcount; diff --git a/kernel-src/sys/syscalls/pread.c b/kernel-src/sys/syscalls/pread.c index 65a2aceb..68693ec7 100644 --- a/kernel-src/sys/syscalls/pread.c +++ b/kernel-src/sys/syscalls/pread.c @@ -35,10 +35,6 @@ syscallret_t syscall_pread(context_t *context, int fd, void *buffer, size_t size goto cleanup; } - - vattr_t attr; - VOP_GETATTR(file->vnode, &attr, NULL); - size_t bytesread; ret.errno = vfs_read(file->vnode, kernelbuff, size, offset, &bytesread, fileflagstovnodeflags(file->flags)); diff --git a/kernel-src/sys/syscalls/readlinkat.c b/kernel-src/sys/syscalls/readlinkat.c index d8ffaecf..59a76788 100644 --- a/kernel-src/sys/syscalls/readlinkat.c +++ b/kernel-src/sys/syscalls/readlinkat.c @@ -39,6 +39,8 @@ syscallret_t syscall_readlinkat(context_t *, int dirfd, char *upath, char *ubuff goto cleanup; ret.errno = VOP_READLINK(node, &buffer, &_cpu()->thread->proc->cred); + // locked by vfs_lookup + VOP_UNLOCK(node); if (ret.errno) goto cleanup; diff --git a/kernel-src/sys/syscalls/renameat.c b/kernel-src/sys/syscalls/renameat.c index 53ff70e5..7f6c6736 100644 --- a/kernel-src/sys/syscalls/renameat.c +++ b/kernel-src/sys/syscalls/renameat.c @@ -60,16 +60,21 @@ syscallret_t syscall_renameat(context_t *context, int olddirfd, char *uoldpath, if (oldcomponent == NULL && newcomponent == NULL) goto cleanup; + // TODO possibly deadlocks ret.errno = vfs_lookup(&sourcedirnode, olddirnode, oldpath, oldcomponent, VFS_LOOKUP_NOLINK | VFS_LOOKUP_PARENT); if (ret.errno) goto cleanup; ret.errno = vfs_lookup(&targetdirnode, newdirnode, newpath, newcomponent, VFS_LOOKUP_NOLINK | VFS_LOOKUP_PARENT); - if (ret.errno) + if (ret.errno) { + VOP_UNLOCK(sourcedirnode); goto cleanup; + } ret.errno = VOP_RENAME(sourcedirnode, oldcomponent, targetdirnode, newcomponent, flags); ret.ret = ret.errno ? -1 : 0; + VOP_UNLOCK(targetdirnode); + VOP_UNLOCK(sourcedirnode); cleanup: diff --git a/kernel-src/sys/syscalls/seek.c b/kernel-src/sys/syscalls/seek.c index 71a5edef..680c1a72 100644 --- a/kernel-src/sys/syscalls/seek.c +++ b/kernel-src/sys/syscalls/seek.c @@ -43,7 +43,9 @@ syscallret_t syscall_seek(context_t *context, int fd, off_t offset, int whence) } } else { vattr_t attr; + VOP_LOCK(file->vnode); ret.errno = VOP_GETATTR(file->vnode, &attr, &_cpu()->thread->proc->cred); + VOP_UNLOCK(file->vnode); if (ret.errno) goto cleanup; size = attr.size; diff --git a/kernel-src/sys/syscalls/stat.c b/kernel-src/sys/syscalls/stat.c index 078bb7a4..0d65c448 100644 --- a/kernel-src/sys/syscalls/stat.c +++ b/kernel-src/sys/syscalls/stat.c @@ -6,6 +6,7 @@ #include #include +// vnode should be locked static int dostat(vnode_t *vnode, stat_t *stat) { vattr_t attr; int e = VOP_GETATTR(vnode, &attr, &_cpu()->thread->proc->cred); @@ -40,7 +41,9 @@ syscallret_t syscall_fstat(context_t *ctx, int fd, stat_t *ustat) { } stat_t buf; + VOP_LOCK(file->vnode); ret.errno = dostat(file->vnode, &buf); + VOP_UNLOCK(file->vnode); if (ret.errno) goto cleanup; @@ -87,6 +90,8 @@ syscallret_t syscall_fstatat(context_t *ctx, int dirfd, char *upath, stat_t *ust stat_t buf; ret.errno = dostat(node, &buf); + // locked by vfs_lookup + VOP_UNLOCK(node); if (ret.errno) goto cleanup; diff --git a/kernel-src/sys/syscalls/sync.c b/kernel-src/sys/syscalls/sync.c index 8daccd66..877af7f5 100644 --- a/kernel-src/sys/syscalls/sync.c +++ b/kernel-src/sys/syscalls/sync.c @@ -18,7 +18,9 @@ syscallret_t syscall_fsync(context_t *context, int fd) { return ret; } + VOP_LOCK(file->vnode); ret.errno = VOP_SYNC(file->vnode); + VOP_UNLOCK(file->vnode); fd_release(file); ret.ret = ret.errno ? -1 : 0; diff --git a/kernel-src/sys/syscalls/truncate.c b/kernel-src/sys/syscalls/truncate.c index 20ff9355..d1c5031e 100644 --- a/kernel-src/sys/syscalls/truncate.c +++ b/kernel-src/sys/syscalls/truncate.c @@ -17,9 +17,10 @@ syscallret_t syscall_ftruncate(int fd, size_t size) { goto cleanup; } - MUTEX_ACQUIRE(&file->vnode->sizelock, false); + VOP_LOCK(file->vnode); ret.errno = VOP_RESIZE(file->vnode, size, &_cpu()->thread->proc->cred); - MUTEX_RELEASE(&file->vnode->sizelock); + VOP_UNLOCK(file->vnode); + ret.ret = ret.errno ? -1 : 0; cleanup: diff --git a/kernel-src/sys/syscalls/unlinkat.c b/kernel-src/sys/syscalls/unlinkat.c index b33bcc8b..b39ddfdd 100644 --- a/kernel-src/sys/syscalls/unlinkat.c +++ b/kernel-src/sys/syscalls/unlinkat.c @@ -49,6 +49,8 @@ syscallret_t syscall_unlinkat(context_t *, int dirfd, char *upath, int flags) { ret.errno = VOP_UNLINK(node, component, &_cpu()->thread->proc->cred); ret.ret = ret.errno ? -1 : 0; + // locked by vfs_lookup + VOP_UNLOCK(node); cleanup: if (component) diff --git a/kernel-src/sys/syscalls/utime.c b/kernel-src/sys/syscalls/utime.c index abeaf05f..ea1b9b74 100644 --- a/kernel-src/sys/syscalls/utime.c +++ b/kernel-src/sys/syscalls/utime.c @@ -32,6 +32,7 @@ syscallret_t syscall_utimensat(context_t *, int fd, char *upath, timespec_t uts[ } node = file->vnode; + VOP_LOCK(node); VOP_HOLD(node); fd_release(file); } else { @@ -62,8 +63,10 @@ syscallret_t syscall_utimensat(context_t *, int fd, char *upath, timespec_t uts[ vattr_t attr; ret.errno = VOP_GETATTR(node, &attr, &_cpu()->thread->proc->cred); - if (ret.errno) + if (ret.errno) { + VOP_UNLOCK(node); goto cleanup; + } attr.atime = ts[0]; attr.mtime = ts[1]; @@ -73,12 +76,15 @@ syscallret_t syscall_utimensat(context_t *, int fd, char *upath, timespec_t uts[ if (uts == NULL) ret.errno = VOP_ACCESS(node, V_ACCESS_WRITE, &_cpu()->thread->proc->cred); - if (ret.errno) + if (ret.errno) { + VOP_UNLOCK(node); goto cleanup; + } } ret.errno = VOP_SETATTR(node, &attr, V_ATTR_ATIME | V_ATTR_MTIME, &_cpu()->thread->proc->cred); ret.ret = ret.errno ? -1 : 0; + VOP_UNLOCK(node); cleanup: if (node) diff --git a/kernel-src/sys/syscalls/write.c b/kernel-src/sys/syscalls/write.c index eede6ae0..73be44f1 100644 --- a/kernel-src/sys/syscalls/write.c +++ b/kernel-src/sys/syscalls/write.c @@ -36,7 +36,10 @@ syscallret_t syscall_write(context_t *context, int fd, void *buffer, size_t size if (file->flags & O_APPEND) { vattr_t attr; + // TODO racey + VOP_LOCK(file->vnode); ret.errno = VOP_GETATTR(file->vnode, &attr, &_cpu()->thread->proc->cred); + VOP_UNLOCK(file->vnode); if (ret.errno) goto cleanup;