Skip site navigation (1)Skip section navigation (2) 
Date:      Sun, 16 Feb 2020 03:33:35 +0000 (UTC)
From:      Mateusz Guzik <mjg@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   svn commit: r357991 - head/sys/kern
Message-ID:  <202002160333.01G3XZj4087086@repo.freebsd.org>
next in thread | raw e-mail | index | archive | help 
Author: mjg
Date: Sun Feb 16 03:33:34 2020
New Revision: 357991
URL: https://svnweb.freebsd.org/changeset/base/357991

Log:
  vfs: fix vlrureclaim ->v_object access
  
  The routine was checking for ->v_type == VBAD. Since vgone drops the interlock
  early sets this type at the end of the process of dooming a vnode, this opens
  a time window where it can clear the pointer while the inerlock-holders is
  accessing it.
  
  Another note is that the code was:
  	   (vp->v_object != NULL &&
  	   vp->v_object->resident_page_count > trigger)
  
  With the compiler being fully allowed to emit another read to get the pointer,
  and in fact it did on the kernel used by pho.
  
  Use atomic_load_ptr and remember the result.
  
  Note that this depends on type-safety of vm_object.
  
  Reported by:	pho

Modified:
  head/sys/kern/vfs_subr.c

Modified: head/sys/kern/vfs_subr.c
==============================================================================
--- head/sys/kern/vfs_subr.c	Sun Feb 16 03:16:28 2020	(r357990)
+++ head/sys/kern/vfs_subr.c	Sun Feb 16 03:33:34 2020	(r357991)
@@ -1100,6 +1100,7 @@ vlrureclaim(bool reclaim_nc_src, int trigger, u_long t
 {
 	struct vnode *vp, *mvp;
 	struct mount *mp;
+	struct vm_object *object;
 	u_long done;
 	bool retried;
 
@@ -1137,12 +1138,17 @@ restart:
 
 		if (vp->v_usecount > 0 || vp->v_holdcnt == 0 ||
 		    (!reclaim_nc_src && !LIST_EMPTY(&vp->v_cache_src)) ||
-		    vp->v_type == VBAD || vp->v_type == VNON ||
-		    (vp->v_object != NULL &&
-		    vp->v_object->resident_page_count > trigger)) {
+		    VN_IS_DOOMED(vp) || vp->v_type == VNON) {
 			VI_UNLOCK(vp);
 			goto next_iter;
 		}
+
+		object = atomic_load_ptr(&vp->v_object);
+		if (object == NULL || object->resident_page_count > trigger) {
+			VI_UNLOCK(vp);
+			goto next_iter;
+		}
+
 		vholdl(vp);
 		VI_UNLOCK(vp);
 		TAILQ_REMOVE(&vnode_list, mvp, v_vnodelist);

Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202002160333.01G3XZj4087086>