Module Name: src Committed By: pooka Date: Fri Jan 15 01:00:46 UTC 2010
Modified Files: src/sys/kern: vfs_syscalls.c Log Message: Fix reference counting for vfsops in mount. Otherwise it's possible (for an unprivileged user) to force vfs modules to remain loaded forever. Also, it's possible for an admin with fat fingers to have to curse out loud (a lot) and reboot. .. or at least fix things as much as seems to be possible without involving 1000 zorkmids. do_sys_mount() takes either struct vfsops (which hopefully came properly referenced) or a userspace string for file system type. The standard in-kernel calling convention of "do_sys_mount(l, vfs_getopsbyname("nfs"), NULL," is not to be considered healthy, kosher, or even tasty (although if vfs_getopsbyname() fails the whole thing *currently* fails without the program counter pointing to hyperspace). To generate a diff of this commit: cvs rdiff -u -r1.402 -r1.403 src/sys/kern/vfs_syscalls.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files.
Modified files: Index: src/sys/kern/vfs_syscalls.c diff -u src/sys/kern/vfs_syscalls.c:1.402 src/sys/kern/vfs_syscalls.c:1.403 --- src/sys/kern/vfs_syscalls.c:1.402 Fri Jan 8 11:35:10 2010 +++ src/sys/kern/vfs_syscalls.c Fri Jan 15 01:00:46 2010 @@ -1,4 +1,4 @@ -/* $NetBSD: vfs_syscalls.c,v 1.402 2010/01/08 11:35:10 pooka Exp $ */ +/* $NetBSD: vfs_syscalls.c,v 1.403 2010/01/15 01:00:46 pooka Exp $ */ /*- * Copyright (c) 2008, 2009 The NetBSD Foundation, Inc. @@ -66,7 +66,7 @@ */ #include <sys/cdefs.h> -__KERNEL_RCSID(0, "$NetBSD: vfs_syscalls.c,v 1.402 2010/01/08 11:35:10 pooka Exp $"); +__KERNEL_RCSID(0, "$NetBSD: vfs_syscalls.c,v 1.403 2010/01/15 01:00:46 pooka Exp $"); #ifdef _KERNEL_OPT #include "opt_fileassoc.h" @@ -299,12 +299,16 @@ error = kauth_authorize_system(l->l_cred, KAUTH_SYSTEM_MOUNT, KAUTH_REQ_SYSTEM_MOUNT_NEW, vp, KAUTH_ARG(flags), data); - if (error) + if (error) { + vfs_delref(vfsops); return error; + } /* Can't make a non-dir a mount-point (from here anyway). */ - if (vp->v_type != VDIR) + if (vp->v_type != VDIR) { + vfs_delref(vfsops); return ENOTDIR; + } /* * If the user is not root, ensure that they own the directory @@ -314,23 +318,32 @@ (va.va_uid != kauth_cred_geteuid(l->l_cred) && (error = kauth_authorize_generic(l->l_cred, KAUTH_GENERIC_ISSUSER, NULL)) != 0)) { + vfs_delref(vfsops); return error; } - if (flags & MNT_EXPORTED) + if (flags & MNT_EXPORTED) { + vfs_delref(vfsops); return EINVAL; + } - if ((error = vinvalbuf(vp, V_SAVE, l->l_cred, l, 0, 0)) != 0) + if ((error = vinvalbuf(vp, V_SAVE, l->l_cred, l, 0, 0)) != 0) { + vfs_delref(vfsops); return error; + } /* * Check if a file-system is not already mounted on this vnode. */ - if (vp->v_mountedhere != NULL) + if (vp->v_mountedhere != NULL) { + vfs_delref(vfsops); return EBUSY; + } - if ((mp = vfs_mountalloc(vfsops, vp)) == NULL) + if ((mp = vfs_mountalloc(vfsops, vp)) == NULL) { + vfs_delref(vfsops); return ENOMEM; + } mp->mnt_stat.f_owner = kauth_cred_geteuid(l->l_cred); @@ -445,14 +458,23 @@ struct vnode *vp; void *data_buf = data; u_int recurse; + bool vfsopsrele = false; int error; + /* XXX: The calling convention of this routine is totally bizarre */ + if (vfsops) + vfsopsrele = true; + /* * Get vnode to be covered */ error = namei_simple_user(path, NSM_FOLLOW_TRYEMULROOT, &vp); - if (error != 0) - return (error); + if (error != 0) { + /* XXXgcc */ + vp = NULL; + recurse = 0; + goto done; + } /* * A lookup in VFS_MOUNT might result in an attempt to @@ -470,6 +492,7 @@ recurse = vn_setrecurse(vp); if (error != 0) goto done; + vfsopsrele = true; } } else { vn_lock(vp, LK_EXCLUSIVE | LK_RETRY); @@ -515,11 +538,15 @@ error = mount_update(l, vp, path, flags, data_buf, &data_len); } else { /* Locking is handled internally in mount_domount(). */ + KASSERT(vfsopsrele == true); error = mount_domount(l, &vp, vfsops, path, flags, data_buf, &data_len, recurse); + vfsopsrele = false; } done: + if (vfsopsrele) + vfs_delref(vfsops); if (vp != NULL) { vn_restorerecurse(vp, recurse); vput(vp);