Serialize accesses to "struct vmspace" and document its refcounting.

The underlying vm_space lock is used as a substitute to the KERNEL_LOCK()
in uvm_grow() to make sure `vm_ssize' is not corrupted.

ok anton@, kettenis@

5 files changed, 37 insertions, 19 deletions

diff --git a/sys/kern/kern_sysctl.c b/sys/kern/kern_sysctl.c
index 19e882ca02c..ccf0e5ffa05 100644
--- a/sys/kern/kern_sysctl.c
+++ b/sys/kern/kern_sysctl.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: kern_sysctl.c,v 1.379 2020/09/01 01:53:50 gnezdo Exp $	*/
+/*	$OpenBSD: kern_sysctl.c,v 1.380 2020/10/19 08:19:46 mpi Exp $	*/
 /*	$NetBSD: kern_sysctl.c,v 1.17 1996/05/20 17:49:05 mrg Exp $	*/
 
 /*-
@@ -1783,7 +1783,7 @@ sysctl_proc_args(int *name, u_int namelen, void *oldp, size_t *oldlenp,
 	/* Execing - danger. */
 	if ((vpr->ps_flags & PS_INEXEC))
 		return (EBUSY);
-	
+
 	/* Only owner or root can get env */
 	if ((op == KERN_PROC_NENV || op == KERN_PROC_ENV) &&
 	    (vpr->ps_ucred->cr_uid != cp->p_ucred->cr_uid &&
@@ -1792,7 +1792,7 @@ sysctl_proc_args(int *name, u_int namelen, void *oldp, size_t *oldlenp,
 
 	ps_strings = vpr->ps_strings;
 	vm = vpr->ps_vmspace;
-	vm->vm_refcnt++;
+	uvmspace_addref(vm);
 	vpr = NULL;
 
 	buf = malloc(PAGE_SIZE, M_TEMP, M_WAITOK);
diff --git a/sys/kern/sys_process.c b/sys/kern/sys_process.c
index 9481d6a7424..4f6bc30cd76 100644
--- a/sys/kern/sys_process.c
+++ b/sys/kern/sys_process.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: sys_process.c,v 1.83 2020/03/16 11:58:46 mpi Exp $	*/
+/*	$OpenBSD: sys_process.c,v 1.84 2020/10/19 08:19:46 mpi Exp $	*/
 /*	$NetBSD: sys_process.c,v 1.55 1996/05/15 06:17:47 tls Exp $	*/
 
 /*-
@@ -850,13 +850,12 @@ process_domem(struct proc *curp, struct process *tr, struct uio *uio, int req)
 	if ((error = process_checkioperm(curp, tr)) != 0)
 		return error;
 
-	/* XXXCDC: how should locking work here? */
 	vm = tr->ps_vmspace;
 	if ((tr->ps_flags & PS_EXITING) || (vm->vm_refcnt < 1))
 		return EFAULT;
 	addr = uio->uio_offset;
 
-	vm->vm_refcnt++;
+	uvmspace_addref(vm);
 
 	error = uvm_io(&vm->vm_map, uio,
 	    (uio->uio_rw == UIO_WRITE) ? UVM_IO_FIXPROT : 0);
@@ -892,7 +891,7 @@ process_auxv_offset(struct proc *curp, struct process *tr, struct uio *uiop)
 	if ((tr->ps_flags & PS_EXITING) || (vm->vm_refcnt < 1))
 		return EFAULT;
 
-	vm->vm_refcnt++;
+	uvmspace_addref(vm);
 	error = uvm_io(&vm->vm_map, &uio, 0);
 	uvmspace_free(vm);
 
diff --git a/sys/uvm/uvm_extern.h b/sys/uvm/uvm_extern.h
index b27ef08ca76..ee8779c36df 100644
--- a/sys/uvm/uvm_extern.h
+++ b/sys/uvm/uvm_extern.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: uvm_extern.h,v 1.153 2020/09/13 10:05:25 mpi Exp $	*/
+/*	$OpenBSD: uvm_extern.h,v 1.154 2020/10/19 08:19:46 mpi Exp $	*/
 /*	$NetBSD: uvm_extern.h,v 1.57 2001/03/09 01:02:12 chs Exp $	*/
 
 /*
@@ -192,11 +192,13 @@ struct pmap;
  * Several fields are temporary (text, data stuff).
  *
  *  Locks used to protect struct members in this file:
+ *	K	kernel lock
  *	I	immutable after creation
+ *	v	vm_map's lock
  */
 struct vmspace {
 	struct	vm_map vm_map;	/* VM address map */
-	int	vm_refcnt;	/* number of references */
+	int	vm_refcnt;	/* [K] number of references */
 	caddr_t	vm_shm;		/* SYS5 shared memory private data XXX */
 /* we copy from vm_startcopy to the end of the structure on fork */
 #define vm_startcopy vm_rssize
@@ -205,9 +207,9 @@ struct vmspace {
 	segsz_t vm_tsize;	/* text size (pages) XXX */
 	segsz_t vm_dsize;	/* data size (pages) XXX */
 	segsz_t vm_dused;	/* data segment length (pages) XXX */
-	segsz_t vm_ssize;	/* stack size (pages) */
-	caddr_t	vm_taddr;	/* user virtual address of text XXX */
-	caddr_t	vm_daddr;	/* user virtual address of data XXX */
+	segsz_t vm_ssize;	/* [v] stack size (pages) */
+	caddr_t	vm_taddr;	/* [I] user virtual address of text */
+	caddr_t	vm_daddr;	/* [I] user virtual address of data */
 	caddr_t vm_maxsaddr;	/* [I] user VA at max stack growth */
 	caddr_t vm_minsaddr;	/* [I] user VA at top of stack */
 };
@@ -413,6 +415,7 @@ void			uvmspace_init(struct vmspace *, struct pmap *,
 			    vaddr_t, vaddr_t, boolean_t, boolean_t);
 void			uvmspace_exec(struct proc *, vaddr_t, vaddr_t);
 struct vmspace		*uvmspace_fork(struct process *);
+void			uvmspace_addref(struct vmspace *);
 void			uvmspace_free(struct vmspace *);
 struct vmspace		*uvmspace_share(struct process *);
 int			uvm_share(vm_map_t, vaddr_t, vm_prot_t,
diff --git a/sys/uvm/uvm_map.c b/sys/uvm/uvm_map.c
index 7b2351faeeb..c5c40ef7637 100644
--- a/sys/uvm/uvm_map.c
+++ b/sys/uvm/uvm_map.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: uvm_map.c,v 1.268 2020/09/22 14:31:08 mpi Exp $	*/
+/*	$OpenBSD: uvm_map.c,v 1.269 2020/10/19 08:19:46 mpi Exp $	*/
 /*	$NetBSD: uvm_map.c,v 1.86 2000/11/27 08:40:03 chs Exp $	*/
 
 /*
@@ -3517,7 +3517,6 @@ uvmspace_init(struct vmspace *vm, struct pmap *pmap, vaddr_t min, vaddr_t max,
 /*
  * uvmspace_share: share a vmspace between two processes
  *
- * - XXX: no locking on vmspace
  * - used for vfork
  */
 
@@ -3526,7 +3525,7 @@ uvmspace_share(struct process *pr)
 {
 	struct vmspace *vm = pr->ps_vmspace;
 
-	vm->vm_refcnt++;
+	uvmspace_addref(vm);
 	return vm;
 }
 
@@ -3630,13 +3629,25 @@ uvmspace_exec(struct proc *p, vaddr_t start, vaddr_t end)
 }
 
 /*
+ * uvmspace_addref: add a reference to a vmspace.
+ */
+void
+uvmspace_addref(struct vmspace *vm)
+{
+	KERNEL_ASSERT_LOCKED();
+	KASSERT(vm->vm_refcnt > 0);
+
+	vm->vm_refcnt++;
+}
+
+/*
  * uvmspace_free: free a vmspace data structure
- *
- * - XXX: no locking on vmspace
  */
 void
 uvmspace_free(struct vmspace *vm)
 {
+	KERNEL_ASSERT_LOCKED();
+
 	if (--vm->vm_refcnt == 0) {
 		/*
 		 * lock the map, to wait out all other references to it.  delete
diff --git a/sys/uvm/uvm_unix.c b/sys/uvm/uvm_unix.c
index 0506b6fd81d..8f7ce154295 100644
--- a/sys/uvm/uvm_unix.c
+++ b/sys/uvm/uvm_unix.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: uvm_unix.c,v 1.69 2020/10/13 08:47:59 mpi Exp $	*/
+/*	$OpenBSD: uvm_unix.c,v 1.70 2020/10/19 08:19:46 mpi Exp $	*/
 /*	$NetBSD: uvm_unix.c,v 1.18 2000/09/13 15:00:25 thorpej Exp $	*/
 
 /*
@@ -108,19 +108,22 @@ void
 uvm_grow(struct proc *p, vaddr_t sp)
 {
 	struct vmspace *vm = p->p_vmspace;
+	vm_map_t map = &vm->vm_map;
 	int si;
 
 	/* For user defined stacks (from sendsig). */
 	if (sp < (vaddr_t)vm->vm_maxsaddr)
 		return;
 
+	vm_map_lock(map);
+
 	/* For common case of already allocated (from trap). */
 #ifdef MACHINE_STACK_GROWS_UP
 	if (sp < (vaddr_t)vm->vm_maxsaddr + ptoa(vm->vm_ssize))
 #else
 	if (sp >= (vaddr_t)vm->vm_minsaddr - ptoa(vm->vm_ssize))
 #endif
-		return;
+		goto out;
 
 	/* Really need to check vs limit and increment stack size if ok. */
 #ifdef MACHINE_STACK_GROWS_UP
@@ -130,6 +133,8 @@ uvm_grow(struct proc *p, vaddr_t sp)
 #endif
 	if (vm->vm_ssize + si <= atop(lim_cur(RLIMIT_STACK)))
 		vm->vm_ssize += si;
+out:
+	vm_map_unlock(map);
 }
 
 #ifndef SMALL_KERNEL