Module Name: src
Committed By: snj
Date: Tue Nov 18 18:40:06 UTC 2014
Modified Files:
src/sys/kern [netbsd-7]: vfs_mount.c
src/sys/ufs/ffs [netbsd-7]: ffs_vfsops.c
src/sys/ufs/ufs [netbsd-7]: ufs_extattr.c
Log Message:
Pull up following revision(s) (requested by manu in ticket #246):
sys/kern/vfs_mount.c: revision 1.31
sys/ufs/ffs/ffs_vfsops.c: revision 1.302
sys/ufs/ufs/ufs_extattr.c: revision 1.44
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.30.2.1 src/sys/kern/vfs_mount.c
cvs rdiff -u -r1.299 -r1.299.2.1 src/sys/ufs/ffs/ffs_vfsops.c
cvs rdiff -u -r1.43 -r1.43.4.1 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.30.2.1
--- src/sys/kern/vfs_mount.c:1.30 Fri May 30 08:46:00 2014
+++ src/sys/kern/vfs_mount.c Tue Nov 18 18:40:06 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.30.2.1 2014/11/18 18:40:06 snj 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.30.2.1 2014/11/18 18:40:06 snj 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.299 src/sys/ufs/ffs/ffs_vfsops.c:1.299.2.1
--- src/sys/ufs/ffs/ffs_vfsops.c:1.299 Sat May 24 16:34:04 2014
+++ src/sys/ufs/ffs/ffs_vfsops.c Tue Nov 18 18:40:06 2014
@@ -1,4 +1,4 @@
-/* $NetBSD: ffs_vfsops.c,v 1.299 2014/05/24 16:34:04 christos Exp $ */
+/* $NetBSD: ffs_vfsops.c,v 1.299.2.1 2014/11/18 18:40:06 snj 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.299 2014/05/24 16:34:04 christos Exp $");
+__KERNEL_RCSID(0, "$NetBSD: ffs_vfsops.c,v 1.299.2.1 2014/11/18 18:40:06 snj 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.43.4.1
--- src/sys/ufs/ufs/ufs_extattr.c:1.43 Fri Feb 7 15:29:23 2014
+++ src/sys/ufs/ufs/ufs_extattr.c Tue Nov 18 18:40:06 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.43.4.1 2014/11/18 18:40:06 snj 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.43.4.1 2014/11/18 18:40:06 snj 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)) {
