Index | Thread | Search

From:
Kirill A. Korinsky <kirill@korins.ky>
Subject:
sys/uvn: fix amap_add() failure cleanup
To:
OpenBSD tech <tech@openbsd.org>
Date:
Sat, 08 Aug 2026 21:57:49 +0200

Download raw body.

Thread
  • Kirill A. Korinsky:

    sys/uvn: fix amap_add() failure cleanup

tech@,

uvmfault_promote() creates an anon with one reference and associates its
resident page with the amap lock. If amap_add() cannot allocate an amap
chunk, it returns without publishing the anon, leaving the fault path
responsible for releasing both resources.

The failure cleanup calls uvm_anfree() after releasing the amap lock and
without dropping the anon reference. This violates uvm_anfree()'s
requirements that an_ref be zero and the resident anon's lock be write
held; uvm_pageclean() requires the same lock while detaching the page.
Diagnostic kernels can panic; without those assertions, the resident
page is freed without the required serialization.

Ok?

Thus amap_populate() reads a bit odd here as well. I think we should replace
it by something like that, but it should be separated commit and I not
completley sure about yet:

void
amap_chunk_wait(void)
{
	struct vm_amap_chunk *chunk;

	chunk = pool_get(&uvm_amap_chunk_pool, PR_WAITOK);
	KASSERT(chunk != NULL);
	pool_put(&uvm_amap_chunk_pool, chunk);
}

anyway, the diff:

Index: sys/uvm/uvm_fault.c
===================================================================
RCS file: /home/cvs/src/sys/uvm/uvm_fault.c,v
diff -u -p -r1.173 uvm_fault.c
--- sys/uvm/uvm_fault.c	10 Dec 2025 08:38:18 -0000	1.173
+++ sys/uvm/uvm_fault.c	8 Aug 2026 19:31:21 -0000
@@ -1473,8 +1473,9 @@ uvm_fault_lower(struct uvm_faultinfo *uf
 			atomic_clearbits_int(&pg->pg_flags,
 			    PG_BUSY|PG_FAKE|PG_WANTED);
 			UVM_PAGE_OWN(pg, NULL);
-			uvmfault_unlockall(ufi, amap, uobj);
+			anon->an_ref--;
 			uvm_anfree(anon);
+			uvmfault_unlockall(ufi, amap, uobj);
 			counters_inc(uvmexp_counters, flt_noamap);
 
 			if (uvm_swapisfull())

-- 
wbr, Kirill