d/freebsd-skq
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Right now, dounmount() is called with unreferenced mount point.

Browse Source 
Nothing stops a parallel unmount to suceed before the given call to
dounmount() checks and locks the covered vnode.  Prevent dounmount()
from acting on the freed (although type-stable) memory by changing the
interface to require the mount point to be referenced.  dounmount()
consumes the reference on return, regardless of the sucessfull or
erronous result.

Tested by:	pho
Sponsored by:	The FreeBSD Foundation
MFC after:	2 weeks
This commit is contained in:
Konstantin Belousov 2015-05-27 09:22:50 +00:00
parent 2db0e1f50d
commit 780dca1b1e
5 changed files with 20 additions and 16 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_ctldir.c
View File

@ -697,6 +697,7 @@ zfsctl_unmount_snap(zfs_snapentry_t *sep, int fflags, cred_t *cr)
return (0);
#else
vfs_ref(vn_mountedvfs(svp));
return (dounmount(vn_mountedvfs(svp), fflags, curthread));
#endif
}

1
sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_ioctl.c
View File

@ -3481,6 +3481,7 @@ zfs_unmount_snap(const char *snapname)
#ifdef illumos
(void) dounmount(vfsp, MS_FORCE, kcred);
#else
vfs_ref(vfsp);
(void) dounmount(vfsp, MS_FORCE, curthread);
#endif
return (0);

1
sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_vfsops.c
View File

@ -2315,6 +2315,7 @@ bail:
* unmount this file system.
*/
if (vn_vfswlock(zfsvfs->z_vfs->vfs_vnodecovered) == 0)
vfs_ref(zfsvfs->z_vfs);
(void) dounmount(zfsvfs->z_vfs, MS_FORCE, curthread);
}
return (err);

30
sys/kern/vfs_mount.c
View File

@ -1128,12 +1128,7 @@ struct unmount_args {
#endif
/* ARGSUSED */
int
sys_unmount(td, uap)
struct thread *td;
register struct unmount_args /* {
char *path;
int flags;
} */ *uap;
sys_unmount(struct thread *td, struct unmount_args *uap)
{
struct nameidata nd;
struct mount *mp;
@ -1164,9 +1159,11 @@ sys_unmount(td, uap)
mtx_lock(&mountlist_mtx);
TAILQ_FOREACH_REVERSE(mp, &mountlist, mntlist, mnt_list) {
if (mp->mnt_stat.f_fsid.val[0] == id0 &&
mp->mnt_stat.f_fsid.val[1] == id1)
mp->mnt_stat.f_fsid.val[1] == id1) {
vfs_ref(mp);
break;
}
}
mtx_unlock(&mountlist_mtx);
} else {
/*
@ -1183,9 +1180,11 @@ sys_unmount(td, uap)
}
mtx_lock(&mountlist_mtx);
TAILQ_FOREACH_REVERSE(mp, &mountlist, mntlist, mnt_list) {
if (strcmp(mp->mnt_stat.f_mntonname, pathbuf) == 0)
if (strcmp(mp->mnt_stat.f_mntonname, pathbuf) == 0) {
vfs_ref(mp);
break;
}
}
mtx_unlock(&mountlist_mtx);
}
free(pathbuf, M_TEMP);
@ -1202,8 +1201,10 @@ sys_unmount(td, uap)
/*
* Don't allow unmounting the root filesystem.
*/
if (mp->mnt_flag & MNT_ROOTFS)
if (mp->mnt_flag & MNT_ROOTFS) {
vfs_rel(mp);
return (EINVAL);
}
error = dounmount(mp, uap->flags, td);
return (error);
}
@ -1212,10 +1213,7 @@ sys_unmount(td, uap)
* Do the actual filesystem unmount.
*/
int
dounmount(mp, flags, td)
struct mount *mp;
int flags;
struct thread *td;
dounmount(struct mount *mp, int flags, struct thread *td)
{
struct vnode *coveredvp, *fsrootvp;
int error;
@ -1235,6 +1233,7 @@ dounmount(mp, flags, td)
if (coveredvp->v_mountedhere != mp ||
coveredvp->v_mountedhere->mnt_gen != mnt_gen_r) {
VOP_UNLOCK(coveredvp, 0);
vfs_rel(mp);
return (EBUSY);
}
}
@ -1243,13 +1242,14 @@ dounmount(mp, flags, td)
* original mount is permitted to unmount this filesystem.
*/
error = vfs_suser(mp, td);
if (error) {
if (error != 0) {
if (coveredvp)
VOP_UNLOCK(coveredvp, 0);
vfs_rel(mp);
return (error);
}
vn_start_write(NULL, &mp, V_WAIT);
vn_start_write(NULL, &mp, V_WAIT | V_MNTREF);
MNT_ILOCK(mp);
if ((mp->mnt_kern_flag & MNTK_UNMOUNT) != 0 ||
!TAILQ_EMPTY(&mp->mnt_uppers)) {

3
sys/kern/vfs_subr.c
View File

@ -3502,8 +3502,9 @@ vfs_unmountall(void)
*/
while(!TAILQ_EMPTY(&mountlist)) {
mp = TAILQ_LAST(&mountlist, mntlist);
vfs_ref(mp);
error = dounmount(mp, MNT_FORCE, td);
if (error) {
if (error != 0) {
TAILQ_REMOVE(&mountlist, mp, mnt_list);
/*
* XXX: Due to the way in which we mount the root