diff --git a/kernel-src/fs/ext2.c b/kernel-src/fs/ext2.c index 5f944478..4c593920 100644 --- a/kernel-src/fs/ext2.c +++ b/kernel-src/fs/ext2.c @@ -1176,7 +1176,8 @@ static int ext2_lookup(vnode_t *vnode, char *name, vnode_t **result, cred_t *cre // VOP_LOOKUP returns the vnode locked *result = &newnode->vnode; - VOP_LOCK(*result); + if (newnode != node) + VOP_LOCK(*result); cleanup: if (err && newnode) { @@ -1487,9 +1488,7 @@ static int ext2_symlink(vnode_t *vnode, char *name, vattr_t *attr, char *path, c if (err) return err; - // 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 @@ -1511,8 +1510,12 @@ static int ext2_symlink(vnode_t *vnode, char *name, vattr_t *attr, char *path, c VOP_RELEASE(newvnode); cleanup: - if (err) - ASSERT_UNCLEAN(fs, VOP_UNLINK(vnode, name, cred) == 0); + if (err) { + ASSERT_UNCLEAN(fs, VOP_UNLINK(vnode, newvnode, name, cred) == 0); + } else { + VOP_UNLOCK(newvnode); // locked by internalcreate + } + return err; } @@ -1582,22 +1585,16 @@ static int handleinodeunlink(ext2fs_t *fs, ext2node_t *node, int inode) { return err; } -static int internalunlink(ext2fs_t *fs, ext2node_t *node, char *name) { +static int ext2_unlink(vnode_t *vnode, vnode_t *child, char *name, cred_t *cred) { + ext2node_t *node = (ext2node_t *)vnode; + ext2fs_t *fs = (ext2fs_t *)vnode->vfs; + int inode = 0; int err = removedent(fs, node, name, &inode); if (err) return err; err = handleinodeunlink(fs, node, inode); - 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; - - int err = internalunlink(fs, node, name); return err; } @@ -1686,7 +1683,7 @@ static int ext2_rename(vnode_t *sourcedir, char *oldname, vnode_t *targetdir, ch if (ext2sourcenode->id == oldinode) { // link still points to the same inode, unlink it - err = internalunlink(fs, ext2sourcedirnode, oldname); + err = VOP_UNLINK(sourcedir, source, oldname, NULL); } cleanup: diff --git a/kernel-src/fs/tmpfs.c b/kernel-src/fs/tmpfs.c index a5ef428f..c613e8d1 100644 --- a/kernel-src/fs/tmpfs.c +++ b/kernel-src/fs/tmpfs.c @@ -235,7 +235,7 @@ static int tmpfs_access(vnode_t *vnode, mode_t mode, cred_t *cred) { return auth_filesystem_check(cred, auth_filesystem_convertaccess(mode), vnode) ? EACCES : 0; } -static int tmpfs_unlink(vnode_t *node, char *name, cred_t *cred) { +static int tmpfs_unlink(vnode_t *node, vnode_t *child, char *name, cred_t *cred) { tmpfsnode_t *tmpnode = (tmpfsnode_t *)node; if (node->type != V_TYPE_DIR) return ENOTDIR; @@ -254,6 +254,7 @@ static int tmpfs_unlink(vnode_t *node, char *name, cred_t *cred) { vnode_t *unlinknode = r; tmpfsnode_t *unlinktmpnode = r; + __assert(child == unlinknode); err = hashtable_remove(&tmpnode->children, name, namelen); if (err) diff --git a/kernel-src/fs/vfs.c b/kernel-src/fs/vfs.c index d149fccc..8e50b558 100644 --- a/kernel-src/fs/vfs.c +++ b/kernel-src/fs/vfs.c @@ -40,6 +40,15 @@ int vfs_pollstub(vnode_t *node, struct polldata *, int events) { return revents; } +static int noop() { + return 0; +} + +static vops_t vnops = { + .lock = noop, + .unlock = noop +}; + void vfs_init() { __assert(hashtable_init(&fstable, 20) == 0); vfsroot = alloc(sizeof(vnode_t)); @@ -47,6 +56,7 @@ void vfs_init() { SPINLOCK_INIT(listlock); vfsroot->type = V_TYPE_DIR; vfsroot->refcount = 1; + vfsroot->ops = &vnops; } int vfs_register(vfsops_t *ops, char *name) { @@ -131,7 +141,7 @@ int vfs_open(vnode_t *ref, char *path, int flags, vnode_t **res) { return err; } - vnode_t *new = ref; + vnode_t *new = tmp; err = VOP_OPEN(&new, flags, getcred()); if (err) { VOP_RELEASE(tmp); @@ -455,7 +465,25 @@ int vfs_unlink(vnode_t *ref, char *path) { if (err) goto cleanup; - VOP_UNLINK(parent, component, getcred()); + vnode_t *child = NULL; + err = VOP_LOOKUP(parent, component, &child, getcred()); + if (err) { + VOP_UNLOCK(parent); + goto cleanup; + } + + // TODO auth check + if (0) { + // locked by VOP_LOOKUP + VOP_UNLOCK(child); + // locked by vfs_lookup + VOP_UNLOCK(parent); + return err; + } + + err = VOP_UNLINK(parent, child, component, getcred()); + // locked by VOP_LOOKUP + VOP_UNLOCK(child); // locked by vfs_lookup VOP_UNLOCK(parent); VOP_RELEASE(parent); @@ -509,7 +537,9 @@ int vfs_lookup(vnode_t **result, vnode_t *start, char *path, char *lastcomp, int return e; if ((flags & VFS_LOOKUP_INPUTLOCKED) == 0) { + arch_e9_puts("lock1\n"); VOP_LOCK(r); + arch_e9_puts("lock1ok\n"); } VOP_HOLD(r); @@ -540,8 +570,10 @@ int vfs_lookup(vnode_t **result, vnode_t *start, char *path, char *lastcomp, int error = 0; VOP_HOLD(current); + arch_e9_puts("lock2\n"); if ((flags & VFS_LOOKUP_INPUTLOCKED) == 0 || current != start) VOP_LOCK(current); + arch_e9_puts("lock2ok\n"); for (int i = 0; i < pathlen; ++i) { if (compbuffer[i] == '\0') @@ -563,12 +595,16 @@ int vfs_lookup(vnode_t **result, vnode_t *start, char *path, char *lastcomp, int islast = j == pathlen; } + arch_e9_puts("component: "); + arch_e9_puts(component); + if (islast && (flags & VFS_LOOKUP_PARENT)) { __assert(lastcomp); strcpy(lastcomp, component); break; } + if (strcmp(component, "..") == 0) { // if the tree root, skip to next component vnode_t *root = NULL; @@ -581,6 +617,7 @@ int vfs_lookup(vnode_t **result, vnode_t *start, char *path, char *lastcomp, int // if the root of a mounted fs, go to if (current->flags & V_FLAGS_ROOT) { vnode_t *n = lowestnodeinmp(current); + arch_e9_puts("lock3\n"); if (n != current) { VOP_HOLD(n); VOP_UNLOCK(current); @@ -588,6 +625,7 @@ int vfs_lookup(vnode_t **result, vnode_t *start, char *path, char *lastcomp, int current = n; VOP_LOCK(current); } + arch_e9_puts("lock3ok\n"); } } @@ -596,7 +634,9 @@ int vfs_lookup(vnode_t **result, vnode_t *start, char *path, char *lastcomp, int if (error) break; + arch_e9_puts("lookup\n"); error = VOP_LOOKUP(current, component, &next, getcred()); + arch_e9_puts("lookupok\n"); if (error) break; @@ -614,11 +654,13 @@ int vfs_lookup(vnode_t **result, vnode_t *start, char *path, char *lastcomp, int } if (r != next) { + arch_e9_puts("lock4\n"); VOP_HOLD(r); VOP_UNLOCK(next); VOP_RELEASE(next); next = r; VOP_LOCK(next); + arch_e9_puts("lock4ok\n"); } // dereference symlink @@ -652,7 +694,9 @@ int vfs_lookup(vnode_t **result, vnode_t *start, char *path, char *lastcomp, int } // TODO possible deadlock when input vnode is locked and the vfs_lookup would lock it + arch_e9_puts("it1\n"); error = vfs_lookup(&derefresult, derefstart, linkderef, NULL, pass); + arch_e9_puts("it1ok\n"); if (*linkderef == '/') VOP_RELEASE(derefstart); diff --git a/kernel-src/include/kernel/vfs.h b/kernel-src/include/kernel/vfs.h index dbe5cda1..017394d5 100644 --- a/kernel-src/include/kernel/vfs.h +++ b/kernel-src/include/kernel/vfs.h @@ -126,7 +126,7 @@ typedef struct vops_t { int (*setattr)(vnode_t *node, vattr_t *attr, int attrs, cred_t *cred); int (*poll)(vnode_t *node, struct polldata *, int events); int (*access)(vnode_t *node, mode_t mode, cred_t *cred); - int (*unlink)(vnode_t *node, char *name, cred_t *cred); + int (*unlink)(vnode_t *node, vnode_t *child, char *name, cred_t *cred); int (*link)(vnode_t *node, vnode_t *dir, char *name, cred_t *cred); int (*symlink)(vnode_t *parent, char *name, vattr_t *attr, char *path, cred_t *cred); int (*readlink)(vnode_t *parent, char **link, cred_t *cred); @@ -180,7 +180,7 @@ typedef struct vops_t { #define VOP_GETATTR(v, a, c) (v)->ops->getattr(v, a, c) #define VOP_SETATTR(v, a, w, c) (v)->ops->setattr(v, a, w, c) #define VOP_ACCESS(v, m, c) (v)->ops->access(v, m, c) -#define VOP_UNLINK(v, n, c) (v)->ops->unlink(v, n, c) +#define VOP_UNLINK(v, ch, n, c) (v)->ops->unlink(v, ch, n, c) #define VOP_LINK(v, d, n, c) (v)->ops->link(v, d, n, c) #define VOP_SYMLINK(v, n, a, p, c) (v)->ops->symlink(v, n, a, p, c) #define VOP_READLINK(v, l, c) (v)->ops->readlink(v, l, c)