Skip site navigation (1)Skip section navigation (2) 
Date:      Wed, 6 Nov 2019 16:59:16 +0000 (UTC)
From:      Mark Johnston <markj@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   svn commit: r354400 - head/sys/vm
Message-ID:  <201911061659.xA6GxGwe048100@repo.freebsd.org>
next in thread | raw e-mail | index | archive | help 
Author: markj
Date: Wed Nov  6 16:59:16 2019
New Revision: 354400
URL: https://svnweb.freebsd.org/changeset/base/354400

Log:
  Fix a race in release_page().
  
  Since r354156 we may call release_page() without the page's object lock
  held, specifically following the page copy during a CoW fault.
  release_page() must therefore unbusy the page only after scheduling the
  requeue, to avoid racing with a free of the page.  Previously, the
  object lock prevented this race from occurring.
  
  Add some assertions that were helpful in tracking this down.
  
  Reported by:	pho, syzkaller
  Tested by:	pho
  Reviewed by:	alc, jeff, kib
  Sponsored by:	The FreeBSD Foundation
  Differential Revision:	https://reviews.freebsd.org/D22234

Modified:
  head/sys/vm/vm_fault.c
  head/sys/vm/vm_page.c

Modified: head/sys/vm/vm_fault.c
==============================================================================
--- head/sys/vm/vm_fault.c	Wed Nov  6 16:21:35 2019	(r354399)
+++ head/sys/vm/vm_fault.c	Wed Nov  6 16:59:16 2019	(r354400)
@@ -155,10 +155,14 @@ release_page(struct faultstate *fs)
 {
 
 	if (fs->m != NULL) {
-		vm_page_xunbusy(fs->m);
+		/*
+		 * fs->m's object lock might not be held, so the page must be
+		 * kept busy until we are done with it.
+		 */
 		vm_page_lock(fs->m);
 		vm_page_deactivate(fs->m);
 		vm_page_unlock(fs->m);
+		vm_page_xunbusy(fs->m);
 		fs->m = NULL;
 	}
 }

Modified: head/sys/vm/vm_page.c
==============================================================================
--- head/sys/vm/vm_page.c	Wed Nov  6 16:21:35 2019	(r354399)
+++ head/sys/vm/vm_page.c	Wed Nov  6 16:59:16 2019	(r354400)
@@ -3421,7 +3421,7 @@ vm_page_dequeue(vm_page_t m)
 	struct vm_pagequeue *pq, *pq1;
 	uint8_t aflags;
 
-	KASSERT(mtx_owned(vm_page_lockptr(m)) || m->object == NULL,
+	KASSERT(mtx_owned(vm_page_lockptr(m)) || m->ref_count == 0,
 	    ("page %p is allocated and unlocked", m));
 
 	for (pq = vm_page_pagequeue(m);; pq = pq1) {
@@ -3475,6 +3475,8 @@ vm_page_enqueue(vm_page_t m, uint8_t queue)
 	vm_page_assert_locked(m);
 	KASSERT(m->queue == PQ_NONE && (m->aflags & PGA_QUEUE_STATE_MASK) == 0,
 	    ("%s: page %p is already enqueued", __func__, m));
+	KASSERT(m->ref_count > 0,
+	    ("%s: page %p does not carry any references", __func__, m));
 
 	m->queue = queue;
 	if ((m->aflags & PGA_REQUEUE) == 0)
@@ -3496,6 +3498,8 @@ vm_page_requeue(vm_page_t m)
 	vm_page_assert_locked(m);
 	KASSERT(vm_page_queue(m) != PQ_NONE,
 	    ("%s: page %p is not logically enqueued", __func__, m));
+	KASSERT(m->ref_count > 0,
+	    ("%s: page %p does not carry any references", __func__, m));
 
 	if ((m->aflags & PGA_REQUEUE) == 0)
 		vm_page_aflag_set(m, PGA_REQUEUE);
@@ -3889,6 +3893,8 @@ vm_page_mvqueue(vm_page_t m, const uint8_t nqueue)
 	vm_page_assert_locked(m);
 	KASSERT((m->oflags & VPO_UNMANAGED) == 0,
 	    ("vm_page_mvqueue: page %p is unmanaged", m));
+	KASSERT(m->ref_count > 0,
+	    ("%s: page %p does not carry any references", __func__, m));
 
 	if (vm_page_queue(m) != nqueue) {
 		vm_page_dequeue(m);

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