From 8e33bced3ce4953427a63f2b2c19ab5e078cd01c Mon Sep 17 00:00:00 2001 From: Xin LI Date: Wed, 24 Nov 2004 18:56:13 +0000 Subject: [PATCH] Try to close a potential, but serious race in our VM subsystem. Historically, our contigmalloc1() and contigmalloc2() assumes that a page in PQ_CACHE can be unconditionally reused by busying and freeing it. Unfortunatelly, when object happens to be not NULL, the code will set m->object to NULL and disregard the fact that the page is actually in the VM page bucket, resulting in page bucket hash table corruption and finally, a filesystem corruption, or a 'page not in hash' panic. This commit has borrowed the idea taken from DragonFlyBSD's fix to the VM fix by Matthew Dillon[1]. This version of patch will do the following checks: - When scanning pages in PQ_CACHE, check hold_count and skip over pages that are held temporarily. - For pages in PQ_CACHE and selected as candidate of being freed, check if it is busy at that time. Note: It seems that this is might be unrelated to kern/72539. Obtained from: DragonFlyBSD, sys/vm/vm_contig.c,v 1.11 and 1.12 [1] Reminded by: Matt Dillon Reworked by: alc MFC After: 1 week --- sys/vm/vm_contig.c | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/sys/vm/vm_contig.c b/sys/vm/vm_contig.c index 952d26d68f2..39c708e94c0 100644 --- a/sys/vm/vm_contig.c +++ b/sys/vm/vm_contig.c @@ -251,11 +251,20 @@ again1: vm_page_t m = &pga[i]; if ((m->queue - m->pc) == PQ_CACHE) { + if (m->hold_count != 0) { + start++; + goto again0; + } object = m->object; if (!VM_OBJECT_TRYLOCK(object)) { start++; goto again0; } + if ((m->flags & PG_BUSY) || m->busy != 0) { + VM_OBJECT_UNLOCK(object); + start++; + goto again0; + } vm_page_free(m); VM_OBJECT_UNLOCK(object); } @@ -280,7 +289,6 @@ again1: ("contigmalloc1: page %p was dirty", m)); m->wire_count = 0; m->busy = 0; - m->object = NULL; } mtx_unlock_spin(&vm_page_queue_free_mtx); vm_page_unlock_queues(); @@ -363,7 +371,6 @@ vm_contig_unqueue_free(vm_page_t m) ("contigmalloc2: page %p was dirty", m)); m->wire_count = 0; m->busy = 0; - m->object = NULL; return (error); } @@ -455,9 +462,15 @@ cleanup_freed: } } if (pqtype == PQ_CACHE) { + if (m->hold_count != 0) + goto retry; object = m->object; if (!VM_OBJECT_TRYLOCK(object)) goto retry; + if ((m->flags & PG_BUSY) || m->busy != 0) { + VM_OBJECT_UNLOCK(object); + goto retry; + } vm_page_free(m); VM_OBJECT_UNLOCK(object); }