CVS commit: src/sys/kern

[Taylor R Campbell]

Module Name:    src
Committed By:   riastradh
Date:           Sat Mar 15 12:11:09 UTC 2025

Modified Files:
        src/sys/kern: kern_exec.c

Log Message:
posix_spawn(2): Fix race between parent and child.

This was an embarrassing misuse of condition variables.

PR kern/59175: posix_spawn hang, hanging other process too
PR kern/52634: possible unhandled spurious wakeup in posix_spawn

(This only resolves one of the symptoms wiz had in PR 59175; there is
also another issue involved with p_vmspace and p_psstrp.)


To generate a diff of this commit:
cvs rdiff -u -r1.525 -r1.526 src/sys/kern/kern_exec.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.


Modified files:

Index: src/sys/kern/kern_exec.c
diff -u src/sys/kern/kern_exec.c:1.525 src/sys/kern/kern_exec.c:1.526
--- src/sys/kern/kern_exec.c:1.525	Fri Dec  6 16:48:13 2024
+++ src/sys/kern/kern_exec.c	Sat Mar 15 12:11:09 2025
@@ -1,4 +1,4 @@
-/*	$NetBSD: kern_exec.c,v 1.525 2024/12/06 16:48:13 riastradh Exp $	*/
+/*	$NetBSD: kern_exec.c,v 1.526 2025/03/15 12:11:09 riastradh Exp $	*/
 
 /*-
  * Copyright (c) 2008, 2019, 2020 The NetBSD Foundation, Inc.
@@ -62,7 +62,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: kern_exec.c,v 1.525 2024/12/06 16:48:13 riastradh Exp $");
+__KERNEL_RCSID(0, "$NetBSD: kern_exec.c,v 1.526 2025/03/15 12:11:09 riastradh Exp $");
 
 #include "opt_exec.h"
 #include "opt_execfmt.h"
@@ -281,6 +281,7 @@ struct spawn_exec_data {
 	kcondvar_t		sed_cv_child_ready;
 	kmutex_t		sed_mtx_child;
 	int			sed_error;
+	bool			sed_child_ready;
 	volatile uint32_t	sed_refcnt;
 };
 
@@ -2347,6 +2348,9 @@ spawn_return(void *arg)
 	    && rw_tryenter(&exec_lock, RW_READER)) {
 		parent_is_waiting = false;
 		mutex_enter(&spawn_data->sed_mtx_child);
+		KASSERT(!spawn_data->sed_child_ready);
+		spawn_data->sed_error = 0;
+		spawn_data->sed_child_ready = true;
 		cv_signal(&spawn_data->sed_cv_child_ready);
 		mutex_exit(&spawn_data->sed_mtx_child);
 	}
@@ -2376,6 +2380,9 @@ spawn_return(void *arg)
 
 	if (parent_is_waiting) {
 		mutex_enter(&spawn_data->sed_mtx_child);
+		KASSERT(!spawn_data->sed_child_ready);
+		spawn_data->sed_error = 0;
+		spawn_data->sed_child_ready = true;
 		cv_signal(&spawn_data->sed_cv_child_ready);
 		mutex_exit(&spawn_data->sed_mtx_child);
 	}
@@ -2409,7 +2416,9 @@ spawn_return(void *arg)
 	if (parent_is_waiting) {
 		/* pass error to parent */
 		mutex_enter(&spawn_data->sed_mtx_child);
+		KASSERT(!spawn_data->sed_child_ready);
 		spawn_data->sed_error = error;
+		spawn_data->sed_child_ready = true;
 		cv_signal(&spawn_data->sed_cv_child_ready);
 		mutex_exit(&spawn_data->sed_mtx_child);
 	} else {
@@ -2814,7 +2823,10 @@ do_posix_spawn(struct lwp *l1, pid_t *pi
 	mutex_exit(p2->p_lock);
 	mutex_exit(&proc_lock);
 
-	cv_wait(&spawn_data->sed_cv_child_ready, &spawn_data->sed_mtx_child);
+	while (!spawn_data->sed_child_ready) {
+		cv_wait(&spawn_data->sed_cv_child_ready,
+		    &spawn_data->sed_mtx_child);
+	}
 	error = spawn_data->sed_error;
 	mutex_exit(&spawn_data->sed_mtx_child);
 	spawn_exec_data_release(spawn_data);