Author: mjg
Date: Sun Sep  1 14:01:09 2019
New Revision: 351656
URL: https://svnweb.freebsd.org/changeset/base/351656

Log:
  vfs: stop refing freed mount points in vop_stdgetwritemount
  
  The code used blindly ref based on an unsafely red address and then would
  backpedal if necessary. This was safe in terms of memory access since
  mounts are type-stable, but made for a potential a bug where the mount
  was reused and had the count reset to 0 before this code decreased it.
  
  Reviewed by:  kib
  Sponsored by: The FreeBSD Foundation
  Differential Revision:        https://reviews.freebsd.org/D21411

Modified:
  head/sys/kern/vfs_default.c

Modified: head/sys/kern/vfs_default.c
==============================================================================
--- head/sys/kern/vfs_default.c Sun Sep  1 10:39:16 2019        (r351655)
+++ head/sys/kern/vfs_default.c Sun Sep  1 14:01:09 2019        (r351656)
@@ -588,22 +588,28 @@ vop_stdgetwritemount(ap)
        } */ *ap;
 {
        struct mount *mp;
+       struct vnode *vp;
 
        /*
-        * XXX Since this is called unlocked we may be recycled while
-        * attempting to ref the mount.  If this is the case or mountpoint
-        * will be set to NULL.  We only have to prevent this call from
-        * returning with a ref to an incorrect mountpoint.  It is not
-        * harmful to return with a ref to our previous mountpoint.
+        * Note that having a reference does not prevent forced unmount from
+        * setting ->v_mount to NULL after the lock gets released. This is of
+        * no consequence for typical consumers (most notably vn_start_write)
+        * since in this case the vnode is VI_DOOMED. Unmount might have
+        * progressed far enough that its completion is only delayed by the
+        * reference obtained here. The consumer only needs to concern itself
+        * with releasing it.
         */
-       mp = ap->a_vp->v_mount;
-       if (mp != NULL) {
-               vfs_ref(mp);
-               if (mp != ap->a_vp->v_mount) {
-                       vfs_rel(mp);
-                       mp = NULL;
-               }
+       vp = ap->a_vp;
+       mp = vp->v_mount;
+       MNT_ILOCK(mp);
+       if (mp != vp->v_mount) {
+               MNT_IUNLOCK(mp);
+               mp = NULL;
+               goto out;
        }
+       MNT_REF(mp);
+       MNT_IUNLOCK(mp);
+out:
        *(ap->a_mpp) = mp;
        return (0);
 }
_______________________________________________
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Reply via email to