Index | Thread | Search

From:
Martin Pieuchot <mpi@grenadille.net>
Subject:
uvm_fault_lower(): page leak in error path
To:
tech@openbsd.org
Date:
Mon, 2 Dec 2024 13:49:06 +0100

Download raw body.

Thread
When amap_add() has been changed to be able to fail in r1.90.  Since
then a cleanup & wakeup are missing from the error path.

`pg' is allocated line 1326 and still has the PG_BUSY|PG_FAKE flags when
amap_add() fails.  Then the anon is release in uvm_anfree() below and the
page is marked as PG_RELEASED and leaked.

Since the page is allocated with the amap lock held I'm not sure any
other thread can set the PG_WANTED flag.  So this might be revisited
later, for the moment I keep the idiom used in the function.

ok?

Index: uvm/uvm_fault.c
===================================================================
RCS file: /cvs/src/sys/uvm/uvm_fault.c,v
diff -u -p -r1.148 uvm_fault.c
--- uvm/uvm_fault.c	29 Nov 2024 06:44:57 -0000	1.148
+++ uvm/uvm_fault.c	2 Dec 2024 12:16:01 -0000
@@ -1420,6 +1416,12 @@ uvm_fault_lower(struct uvm_faultinfo *uf
 
 		if (amap_add(&ufi->entry->aref,
 		    ufi->orig_rvaddr - ufi->entry->start, anon, 0)) {
+			if (pg->pg_flags & PG_WANTED)
+				wakeup(pg);
+
+			atomic_clearbits_int(&pg->pg_flags,
+			    PG_BUSY|PG_FAKE|PG_WANTED);
+			UVM_PAGE_OWN(pg, NULL);
 			uvmfault_unlockall(ufi, amap, uobj);
 			uvm_anfree(anon);
 			counters_inc(uvmexp_counters, flt_noamap);