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);

Reply via email to