[src/trunk]: src/sys Fix use-after-free on failed unmount with extended attri...

[manu <manu%NetBSD.org@localhost>]

details:   https://anonhg.NetBSD.org/src/rev/81150e38e2ae
branches:  trunk
changeset: 333694:81150e38e2ae
user:      manu <manu%NetBSD.org@localhost>
date:      Fri Nov 14 10:09:50 2014 +0000

description:
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.

diffstat:

 sys/kern/vfs_mount.c      |  39 +++++++++++++++++++++++++++++----------
 sys/ufs/ffs/ffs_vfsops.c  |  13 +++----------
 sys/ufs/ufs/ufs_extattr.c |  10 +++++++---
 3 files changed, 39 insertions(+), 23 deletions(-)

diffs (170 lines):

diff -r c814c5dd00c5 -r 81150e38e2ae sys/kern/vfs_mount.c
--- a/sys/kern/vfs_mount.c      Fri Nov 14 09:03:39 2014 +0000
+++ b/sys/kern/vfs_mount.c      Fri Nov 14 10:09:50 2014 +0000
@@ -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 @@
        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 @@
        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 @@
 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 @@
        }
 
        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 @@
                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);
diff -r c814c5dd00c5 -r 81150e38e2ae sys/ufs/ffs/ffs_vfsops.c
--- a/sys/ufs/ffs/ffs_vfsops.c  Fri Nov 14 09:03:39 2014 +0000
+++ b/sys/ufs/ffs/ffs_vfsops.c  Fri Nov 14 10:09:50 2014 +0000
@@ -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 @@
                }
 #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 @@
                        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)
diff -r c814c5dd00c5 -r 81150e38e2ae sys/ufs/ufs/ufs_extattr.c
--- a/sys/ufs/ufs/ufs_extattr.c Fri Nov 14 09:03:39 2014 +0000
+++ b/sys/ufs/ufs/ufs_extattr.c Fri Nov 14 10:09:50 2014 +0000
@@ -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 @@
                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 @@
 
        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)) {