Skip site navigation (1)Skip section navigation (2) 
Date:      Sat, 23 Jan 2010 19:01:25 +0000 (UTC)
From:      Konstantin Belousov <kib@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-7@freebsd.org
Subject:   svn commit: r202895 - stable/7/sys/kern
Message-ID:  <201001231901.o0NJ1Peo091556@svn.freebsd.org>
next in thread | raw e-mail | index | archive | help 
Author: kib
Date: Sat Jan 23 19:01:25 2010
New Revision: 202895
URL: http://svn.freebsd.org/changeset/base/202895

Log:
  MFC r186277:
  The quotactl, statfs and fstatfs syscall implementations may dereference
  NULL pointer to struct mount if the looked up vnode is reclaimed. Also,
  these syscalls only mnt_ref() the mp, still allowing it to be unmounted;
  only struct mount memory is kept from being reused.
  
  Lock the vnode when doing name lookup, then reference its mount point,
  unlock the vnode and vfs_busy the mountpoint. This sequence shall take
  care of both races.
  
  MFC r188141 (by trasz):
  In some situations, mnt_lockref could go negative due to vfs_unbusy() being
  called without calling vfs_busy() first.  This made umount(8) hang waiting
  for mnt_lockref to become zero, which would never happen.
  
  MFC r196887:
  In fhopen, vfs_ref() the mount point while vnode is unlocked, to prevent
  vn_start_write(NULL, &mp) from operating on potentially freed or reused
  struct mount *.
  
  Remove unmatched vfs_rel() in cleanup.
  
  Approved by:	re (bz)

Modified:
  stable/7/sys/kern/vfs_syscalls.c
Directory Properties:
  stable/7/sys/   (props changed)
  stable/7/sys/cddl/contrib/opensolaris/   (props changed)
  stable/7/sys/contrib/dev/acpica/   (props changed)
  stable/7/sys/contrib/pf/   (props changed)

Modified: stable/7/sys/kern/vfs_syscalls.c
==============================================================================
--- stable/7/sys/kern/vfs_syscalls.c	Sat Jan 23 18:42:28 2010	(r202894)
+++ stable/7/sys/kern/vfs_syscalls.c	Sat Jan 23 19:01:25 2010	(r202895)
@@ -200,19 +200,21 @@ quotactl(td, uap)
 	AUDIT_ARG(uid, uap->uid);
 	if (jailed(td->td_ucred) && !prison_quotas)
 		return (EPERM);
-	NDINIT(&nd, LOOKUP, FOLLOW | MPSAFE | AUDITVNODE1,
+	NDINIT(&nd, LOOKUP, FOLLOW | LOCKLEAF | MPSAFE | AUDITVNODE1,
 	   UIO_USERSPACE, uap->path, td);
 	if ((error = namei(&nd)) != 0)
 		return (error);
 	vfslocked = NDHASGIANT(&nd);
 	NDFREE(&nd, NDF_ONLY_PNBUF);
 	mp = nd.ni_vp->v_mount;
-	if ((error = vfs_busy(mp, 0, NULL, td))) {
-		vrele(nd.ni_vp);
+	vfs_ref(mp);
+	vput(nd.ni_vp);
+	error = vfs_busy(mp, 0, NULL, td);
+	vfs_rel(mp);
+	if (error) {
 		VFS_UNLOCK_GIANT(vfslocked);
 		return (error);
 	}
-	vrele(nd.ni_vp);
 	error = VFS_QUOTACTL(mp, uap->cmd, uap->uid, uap->arg, td);
 	vfs_unbusy(mp, td);
 	VFS_UNLOCK_GIANT(vfslocked);
@@ -306,6 +308,12 @@ kern_statfs(struct thread *td, char *pat
 	vfs_ref(mp);
 	NDFREE(&nd, NDF_ONLY_PNBUF);
 	vput(nd.ni_vp);
+	error = vfs_busy(mp, 0, NULL, td);
+	vfs_rel(mp);
+	if (error) {
+		VFS_UNLOCK_GIANT(vfslocked);
+		return (error);
+	}
 #ifdef MAC
 	error = mac_check_mount_stat(td->td_ucred, mp);
 	if (error)
@@ -329,7 +337,7 @@ kern_statfs(struct thread *td, char *pat
 	}
 	*buf = *sp;
 out:
-	vfs_rel(mp);
+	vfs_unbusy(mp);
 	VFS_UNLOCK_GIANT(vfslocked);
 	if (mtx_owned(&Giant))
 		printf("statfs(%d): %s: %d\n", vfslocked, path, error);
@@ -387,10 +395,16 @@ kern_fstatfs(struct thread *td, int fd, 
 		vfs_ref(mp);
 	VOP_UNLOCK(vp, 0, td);
 	fdrop(fp, td);
-	if (vp->v_iflag & VI_DOOMED) {
+	if (mp == NULL) {
 		error = EBADF;
 		goto out;
 	}
+	error = vfs_busy(mp, 0, NULL, td);
+	vfs_rel(mp);
+	if (error) {
+		VFS_UNLOCK_GIANT(vfslocked);
+		return (error);
+	}
 #ifdef MAC
 	error = mac_check_mount_stat(td->td_ucred, mp);
 	if (error)
@@ -415,7 +429,7 @@ kern_fstatfs(struct thread *td, int fd, 
 	*buf = *sp;
 out:
 	if (mp)
-		vfs_rel(mp);
+		vfs_unbusy(mp);
 	VFS_UNLOCK_GIANT(vfslocked);
 	return (error);
 }
@@ -4177,13 +4191,16 @@ fhopen(td, uap)
 			goto bad;
 	}
 	if (fmode & O_TRUNC) {
+		vfs_ref(mp);
 		VOP_UNLOCK(vp, 0, td);				/* XXX */
 		if ((error = vn_start_write(NULL, &mp, V_WAIT | PCATCH)) != 0) {
 			vrele(vp);
+			vfs_rel(mp);
 			goto out;
 		}
 		VOP_LEASE(vp, td, td->td_ucred, LEASE_WRITE);
 		vn_lock(vp, LK_EXCLUSIVE | LK_RETRY, td);	/* XXX */
+		vfs_rel(mp);
 #ifdef MAC
 		/*
 		 * We don't yet have fp->f_cred, so use td->td_ucred, which
@@ -4261,7 +4278,6 @@ fhopen(td, uap)
 
 	VOP_UNLOCK(vp, 0, td);
 	fdrop(fp, td);
-	vfs_rel(mp);
 	VFS_UNLOCK_GIANT(vfslocked);
 	td->td_retval[0] = indx;
 	return (0);

Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201001231901.o0NJ1Peo091556>