From: Stefan Sperling <stsp@stsp.name>
Subject: got-read-gotconfig zombies
To: gameoftrees@openbsd.org
Date: Thu, 28 Nov 2024 17:19:48 +0100

One of my gotwebd servers stopped responding to requests and had
failing cronjobs because the system's process table was full.

Upon closer inspection I found several thousand got-read-gotconfig
zombie child processes of gotwebd.

I am not sure why those were never cleaned up, however the code
doesn't wait for children if an errror occurs after forking so
maybe it ran out of files or memory first and then accumulated
all those zombies?

Has anyone else seen something like this happen?

Attempt to fix this for both gitconfig and gotconfig below, I am
running this on my server now.

ok?

diff /home/stsp/src/got
path + /home/stsp/src/got
commit - b9d87303f685642215a2ac4216badc0c42489e0b
blob - 7f63a506515ee1d8a67b0ae928d5421def842e96
file + lib/read_gitconfig_privsep.c
--- lib/read_gitconfig_privsep.c
+++ lib/read_gitconfig_privsep.c
@@ -104,55 +104,55 @@ got_repo_read_gitconfig(int *gitconfig_repository_form
 
 	if (close(imsg_fds[1]) == -1) {
 		err = got_error_from_errno("close");
-		goto done;
+		goto wait;
 	}
 	imsg_fds[1] = -1;
 	if (imsgbuf_init(ibuf, imsg_fds[0]) == -1) {
 		err = got_error_from_errno("imsgbuf_init");
-		goto done;
+		goto wait;
 	}
 	imsgbuf_allow_fdpass(ibuf);
 
 	err = got_privsep_send_gitconfig_parse_req(ibuf, fd);
 	if (err)
-		goto done;
+		goto wait;
 	fd = -1;
 
 	err = got_privsep_send_gitconfig_repository_format_version_req(ibuf);
 	if (err)
-		goto done;
+		goto wait;
 
 	err = got_privsep_recv_gitconfig_int(
 	    gitconfig_repository_format_version, ibuf);
 	if (err)
-		goto done;
+		goto wait;
 
 	if (extnames && extvals && nextensions) {
 		err = got_privsep_send_gitconfig_repository_extensions_req(
 		    ibuf);
 		if (err)
-			goto done;
+			goto wait;
 		err = got_privsep_recv_gitconfig_int(nextensions, ibuf);
 		if (err)
-			goto done;
+			goto wait;
 		if (*nextensions > 0) {
 			int i;
 			*extnames = calloc(*nextensions, sizeof(char *));
 			if (*extnames == NULL) {
 				err = got_error_from_errno("calloc");
-				goto done;
+				goto wait;
 			}
 			*extvals = calloc(*nextensions, sizeof(char *));
 			if (*extvals == NULL) {
 				err = got_error_from_errno("calloc");
-				goto done;
+				goto wait;
 			}
 			for (i = 0; i < *nextensions; i++) {
 				char *ext, *val;
 				err = got_privsep_recv_gitconfig_pair(&ext,
 				    &val, ibuf);
 				if (err)
-					goto done;
+					goto wait;
 				(*extnames)[i] = ext;
 				(*extvals)[i] = val;
 			}
@@ -161,41 +161,42 @@ got_repo_read_gitconfig(int *gitconfig_repository_form
 
 	err = got_privsep_send_gitconfig_author_name_req(ibuf);
 	if (err)
-		goto done;
+		goto wait;
 
 	err = got_privsep_recv_gitconfig_str(gitconfig_author_name, ibuf);
 	if (err)
-		goto done;
+		goto wait;
 
 	err = got_privsep_send_gitconfig_author_email_req(ibuf);
 	if (err)
-		goto done;
+		goto wait;
 
 	err = got_privsep_recv_gitconfig_str(gitconfig_author_email, ibuf);
 	if (err)
-		goto done;
+		goto wait;
 
 	if (remotes && nremotes) {
 		err = got_privsep_send_gitconfig_remotes_req(ibuf);
 		if (err)
-			goto done;
+			goto wait;
 
 		err = got_privsep_recv_gitconfig_remotes(remotes,
 		    nremotes, ibuf);
 		if (err)
-			goto done;
+			goto wait;
 	}
 
 	if (gitconfig_owner) {
 		err = got_privsep_send_gitconfig_owner_req(ibuf);
 		if (err)
-			goto done;
+			goto wait;
 		err = got_privsep_recv_gitconfig_str(gitconfig_owner, ibuf);
 		if (err)
-			goto done;
+			goto wait;
 	}
-
-	err = got_privsep_send_stop(imsg_fds[0]);
+wait:
+	if (imsg_fds[0] != -1)
+		got_privsep_send_stop(imsg_fds[0]);
 	child_err = got_privsep_wait_for_child(pid);
 	if (child_err && err == NULL)
 		err = child_err;
commit - b9d87303f685642215a2ac4216badc0c42489e0b
blob - 65a45d6c4b0370327816d24b4647648c52f66e46
file + lib/read_gotconfig_privsep.c
--- lib/read_gotconfig_privsep.c
+++ lib/read_gotconfig_privsep.c
@@ -86,64 +86,65 @@ got_gotconfig_read(struct got_gotconfig **conf, const 
 
 	if (close(imsg_fds[1]) == -1) {
 		err = got_error_from_errno("close");
-		goto done;
+		goto wait;
 	}
 	imsg_fds[1] = -1;
 	if (imsgbuf_init(ibuf, imsg_fds[0]) == -1) {
 		err = got_error_from_errno("imsgbuf_init");
-		goto done;
+		goto wait;
 	}
 	imsgbuf_allow_fdpass(ibuf);
 
 	err = got_privsep_send_gotconfig_parse_req(ibuf, fd);
 	if (err)
-		goto done;
+		goto wait;
 	fd = -1;
 
 	err = got_privsep_send_gotconfig_author_req(ibuf);
 	if (err)
-		goto done;
+		goto wait;
 
 	err = got_privsep_recv_gotconfig_str(&(*conf)->author, ibuf);
 	if (err)
-		goto done;
+		goto wait;
 
 	err = got_privsep_send_gotconfig_allowed_signers_req(ibuf);
 	if (err)
-		goto done;
+		goto wait;
 
 	err = got_privsep_recv_gotconfig_str(&(*conf)->allowed_signers_file,
 	    ibuf);
 	if (err)
-		goto done;
+		goto wait;
 
 	err = got_privsep_send_gotconfig_revoked_signers_req(ibuf);
 	if (err)
-		goto done;
+		goto wait;
 
 	err = got_privsep_recv_gotconfig_str(&(*conf)->revoked_signers_file,
 	    ibuf);
 	if (err)
-		goto done;
+		goto wait;
 
 	err = got_privsep_send_gotconfig_signer_id_req(ibuf);
 	if (err)
-		goto done;
+		goto wait;
 
 	err = got_privsep_recv_gotconfig_str(&(*conf)->signer_id, ibuf);
 	if (err)
-		goto done;
+		goto wait;
 
 	err = got_privsep_send_gotconfig_remotes_req(ibuf);
 	if (err)
-		goto done;
+		goto wait;
 
 	err = got_privsep_recv_gotconfig_remotes(&(*conf)->remotes,
 	    &(*conf)->nremotes, ibuf);
 	if (err)
-		goto done;
-
-	err = got_privsep_send_stop(imsg_fds[0]);
+		goto wait;
+wait:
+	if (imsg_fds[0] != -1)
+		got_privsep_send_stop(imsg_fds[0]);
 	child_err = got_privsep_wait_for_child(pid);
 	if (child_err && err == NULL)
 		err = child_err;