From owner-svn-src-stable-7@FreeBSD.ORG Sat Jan 23 19:01:26 2010 Return-Path: Delivered-To: svn-src-stable-7@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id EF92C106566C; Sat, 23 Jan 2010 19:01:25 +0000 (UTC) (envelope-from kib@FreeBSD.org) Received: from svn.freebsd.org (svn.freebsd.org [IPv6:2001:4f8:fff6::2c]) by mx1.freebsd.org (Postfix) with ESMTP id DD95C8FC19; Sat, 23 Jan 2010 19:01:25 +0000 (UTC) Received: from svn.freebsd.org (localhost [127.0.0.1]) by svn.freebsd.org (8.14.3/8.14.3) with ESMTP id o0NJ1PMV091558; Sat, 23 Jan 2010 19:01:25 GMT (envelope-from kib@svn.freebsd.org) Received: (from kib@localhost) by svn.freebsd.org (8.14.3/8.14.3/Submit) id o0NJ1Peo091556; Sat, 23 Jan 2010 19:01:25 GMT (envelope-from kib@svn.freebsd.org) Message-Id: <201001231901.o0NJ1Peo091556@svn.freebsd.org> From: Konstantin Belousov Date: Sat, 23 Jan 2010 19:01:25 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-7@freebsd.org X-SVN-Group: stable-7 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cc: Subject: svn commit: r202895 - stable/7/sys/kern X-BeenThere: svn-src-stable-7@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: SVN commit messages for only the 7-stable src tree List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 23 Jan 2010 19:01:26 -0000 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);