Module Name: src
Committed By: manu
Date: Fri Nov 14 10:09:50 UTC 2014
Modified Files:
src/sys/kern: vfs_mount.c
src/sys/ufs/ffs: ffs_vfsops.c
src/sys/ufs/ufs: ufs_extattr.c
Log Message:
Fix use-after-free on failed unmount with extended attribute enabled
When unmount failed, for instance because the mount is still busy,
UFS1 extended attributes structures were left freed while the kernel
assumes extended attributes were still enabled. This led to using
UFS1 extended attributes structures after free. With LOCKDEBUG, with
quickly triggers a panic.
The problem is fixed by:
1) clear MNT_EXTATTR flag after extended attributes structures are freed
2) attempt to restart extended attributes after failed unmount
2) set MNT_EXTATTR correctly after extended attributes restart
As a side effect, extended attribute structures are now only initialized
when extended attributes are started for the filesystem.
To generate a diff of this commit:
cvs rdiff -u -r1.30 -r1.31 src/sys/kern/vfs_mount.c
cvs rdiff -u -r1.301 -r1.302 src/sys/ufs/ffs/ffs_vfsops.c
cvs rdiff -u -r1.43 -r1.44 src/sys/ufs/ufs/ufs_extattr.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_mount.c
diff -u src/sys/kern/vfs_mount.c:1.30 src/sys/kern/vfs_mount.c:1.31
--- src/sys/kern/vfs_mount.c:1.30 Fri May 30 08:46:00 2014
+++ src/sys/kern/vfs_mount.c Fri Nov 14 10:09:50 2014
@@ -1,4 +1,4 @@
-/* $NetBSD: vfs_mount.c,v 1.30 2014/05/30 08:46:00 hannken Exp $ */
+/* $NetBSD: vfs_mount.c,v 1.31 2014/11/14 10:09:50 manu Exp $ */
/*-
* Copyright (c) 1997-2011 The NetBSD Foundation, Inc.
@@ -67,7 +67,7 @@
*/
#include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: vfs_mount.c,v 1.30 2014/05/30 08:46:00 hannken Exp $");
+__KERNEL_RCSID(0, "$NetBSD: vfs_mount.c,v 1.31 2014/11/14 10:09:50 manu Exp $");
#define _VFS_VNODE_PRIVATE
@@ -616,6 +616,22 @@ mount_checkdirs(vnode_t *olddp)
vput(newdp);
}
+/*
+ * Start extended attributes
+ */
+static int
+start_extattr(struct mount *mp)
+{
+ int error;
+
+ error = VFS_EXTATTRCTL(mp, EXTATTR_CMD_START, NULL, 0, NULL);
+ if (error)
+ printf("%s: failed to start extattr: error = %d\n",
+ mp->mnt_stat.f_mntonname, error);
+
+ return error;
+}
+
int
mount_domount(struct lwp *l, vnode_t **vpp, struct vfsops *vfsops,
const char *path, int flags, void *data, size_t *data_len)
@@ -721,13 +737,9 @@ mount_domount(struct lwp *l, vnode_t **v
error = VFS_START(mp, 0);
if (error) {
vrele(vp);
- } else if (flags & MNT_EXTATTR) {
- error = VFS_EXTATTRCTL(vp->v_mountedhere,
- EXTATTR_CMD_START, NULL, 0, NULL);
- if (error)
- printf("%s: failed to start extattr: error = %d\n",
- vp->v_mountedhere->mnt_stat.f_mntonname, error);
- }
+ } else if (flags & MNT_EXTATTR) {
+ (void)start_extattr(mp);
+ }
/* Drop reference held for VFS_START(). */
vfs_destroy(mp);
*vpp = NULL;
@@ -762,7 +774,7 @@ int
dounmount(struct mount *mp, int flags, struct lwp *l)
{
vnode_t *coveredvp;
- int error, async, used_syncer;
+ int error, async, used_syncer, used_extattr;
#if NVERIEXEC > 0
error = veriexec_unmountchk(mp);
@@ -796,6 +808,7 @@ dounmount(struct mount *mp, int flags, s
}
used_syncer = (mp->mnt_syncer != NULL);
+ used_extattr = mp->mnt_flag & MNT_EXTATTR;
/*
* XXX Syncer must be frozen when we get here. This should really
@@ -835,6 +848,12 @@ dounmount(struct mount *mp, int flags, s
mutex_exit(&mp->mnt_updating);
if (used_syncer)
mutex_exit(&syncer_mutex);
+ if (used_extattr) {
+ if (start_extattr(mp) != 0)
+ mp->mnt_flag &= ~MNT_EXTATTR;
+ else
+ mp->mnt_flag |= MNT_EXTATTR;
+ }
return (error);
}
mutex_exit(&mp->mnt_updating);
Index: src/sys/ufs/ffs/ffs_vfsops.c
diff -u src/sys/ufs/ffs/ffs_vfsops.c:1.301 src/sys/ufs/ffs/ffs_vfsops.c:1.302
--- src/sys/ufs/ffs/ffs_vfsops.c:1.301 Thu Oct 30 17:13:41 2014
+++ src/sys/ufs/ffs/ffs_vfsops.c Fri Nov 14 10:09:50 2014
@@ -1,4 +1,4 @@
-/* $NetBSD: ffs_vfsops.c,v 1.301 2014/10/30 17:13:41 maxv Exp $ */
+/* $NetBSD: ffs_vfsops.c,v 1.302 2014/11/14 10:09:50 manu Exp $ */
/*-
* Copyright (c) 2008, 2009 The NetBSD Foundation, Inc.
@@ -61,7 +61,7 @@
*/
#include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: ffs_vfsops.c,v 1.301 2014/10/30 17:13:41 maxv Exp $");
+__KERNEL_RCSID(0, "$NetBSD: ffs_vfsops.c,v 1.302 2014/11/14 10:09:50 manu Exp $");
#if defined(_KERNEL_OPT)
#include "opt_ffs.h"
@@ -1272,14 +1272,6 @@ ffs_mountfs(struct vnode *devvp, struct
}
#endif
}
-#ifdef UFS_EXTATTR
- /*
- * Initialize file-backed extended attributes on UFS1 file
- * systems.
- */
- if (ump->um_fstype == UFS1)
- ufs_extattr_uepm_init(&ump->um_extattr);
-#endif /* UFS_EXTATTR */
if (mp->mnt_flag & MNT_DISCARD)
ump->um_discarddata = ffs_discard_init(devvp, fs);
@@ -1527,6 +1519,7 @@ ffs_flushfiles(struct mount *mp, int fla
ufs_extattr_stop(mp, l);
if (ump->um_extattr.uepm_flags & UFS_EXTATTR_UEPM_INITIALIZED)
ufs_extattr_uepm_destroy(&ump->um_extattr);
+ mp->mnt_flag &= ~MNT_EXTATTR;
}
#endif
if ((error = vflush(mp, 0, SKIPSYSTEM | flags)) != 0)
Index: src/sys/ufs/ufs/ufs_extattr.c
diff -u src/sys/ufs/ufs/ufs_extattr.c:1.43 src/sys/ufs/ufs/ufs_extattr.c:1.44
--- src/sys/ufs/ufs/ufs_extattr.c:1.43 Fri Feb 7 15:29:23 2014
+++ src/sys/ufs/ufs/ufs_extattr.c Fri Nov 14 10:09:50 2014
@@ -1,4 +1,4 @@
-/* $NetBSD: ufs_extattr.c,v 1.43 2014/02/07 15:29:23 hannken Exp $ */
+/* $NetBSD: ufs_extattr.c,v 1.44 2014/11/14 10:09:50 manu Exp $ */
/*-
* Copyright (c) 1999-2002 Robert N. M. Watson
@@ -48,7 +48,7 @@
*/
#include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: ufs_extattr.c,v 1.43 2014/02/07 15:29:23 hannken Exp $");
+__KERNEL_RCSID(0, "$NetBSD: ufs_extattr.c,v 1.44 2014/11/14 10:09:50 manu Exp $");
#ifdef _KERNEL_OPT
#include "opt_ffs.h"
@@ -384,10 +384,11 @@ ufs_extattr_uepm_destroy(struct ufs_exta
panic("ufs_extattr_uepm_destroy: called while still started");
/*
- * It's not clear that either order for the next two lines is
+ * It's not clear that either order for the next three lines is
* ideal, and it should never be a problem if this is only called
* during unmount, and with vfs_busy().
*/
+ uepm->uepm_flags &= ~UFS_EXTATTR_UEPM_STARTED;
uepm->uepm_flags &= ~UFS_EXTATTR_UEPM_INITIALIZED;
mutex_destroy(&uepm->uepm_lock);
}
@@ -403,6 +404,9 @@ ufs_extattr_start(struct mount *mp, stru
ump = VFSTOUFS(mp);
+ if (!(ump->um_extattr.uepm_flags & UFS_EXTATTR_UEPM_INITIALIZED))
+ ufs_extattr_uepm_init(&ump->um_extattr);
+
ufs_extattr_uepm_lock(ump);
if (!(ump->um_extattr.uepm_flags & UFS_EXTATTR_UEPM_INITIALIZED)) {
