Index | Thread | Search

From:
Martin Pieuchot <mpi@grenadille.net>
Subject:
Prevent race in uvm_fault_lower_io()
To:
tech@openbsd.org
Date:
Sun, 22 Dec 2024 18:10:44 +0100

Download raw body.

Thread
Read entry's value before releasing the locks.  Otherwise pgo_get()
could be called with garbage values as found by claudio@ the hardway.

ok?

Index: uvm/uvm_fault.c
===================================================================
RCS file: /cvs/src/sys/uvm/uvm_fault.c,v
diff -u -p -r1.155 uvm_fault.c
--- uvm/uvm_fault.c	20 Dec 2024 18:46:51 -0000	1.155
+++ uvm/uvm_fault.c	22 Dec 2024 16:57:32 -0000
@@ -1503,22 +1503,28 @@ uvm_fault_lower_io(
 	struct uvm_object *uobj = *ruobj;
 	struct vm_page *pg;
 	boolean_t locked;
-	int gotpages;
+	int gotpages, advice;
 	int result;
 	voff_t uoff;
+	vm_prot_t access_type;
+
+	/* grab everything we need from the entry before we unlock */
+	uoff = (ufi->orig_rvaddr - ufi->entry->start) + ufi->entry->offset;
+	access_type = flt->access_type & MASK(ufi->entry);
+	advice = ufi->entry->advice;
+
+	uvmfault_unlockall(ufi, amap, NULL);
 
 	/* update rusage counters */
 	curproc->p_ru.ru_majflt++;
 
-	uvmfault_unlockall(ufi, amap, NULL);
+	KASSERT(rw_write_held(uobj->vmobjlock));
 
 	counters_inc(uvmexp_counters, flt_get);
 	gotpages = 1;
 	pg = NULL;
-	uoff = (ufi->orig_rvaddr - ufi->entry->start) + ufi->entry->offset;
 	result = uobj->pgops->pgo_get(uobj, uoff, &pg, &gotpages,
-	    0, flt->access_type & MASK(ufi->entry), ufi->entry->advice,
-	    PGO_SYNCIO);
+	    0, access_type, advice, PGO_SYNCIO);
 
 	/*
 	 * recover from I/O