From 2d4a78756411cd983dfa09e7c4b0370797e532c8 Mon Sep 17 00:00:00 2001
From: Martijn Dekker <martijn@inlv.org>
Date: Tue, 11 Jan 2022 14:26:14 +0000
Subject: [PATCH] Fix comsub hang on subshell fork (re: 090b65e7)

The referenced commit introduced a bug that caused command
substitutions to hang, writing infinite zero bytes, when
redirecting standard output on a built-in comand that forks the
command substitution subshell.

The bug was caused by removing the fork when redirecting standard
output in a non-permanent manner. However, simply reintroducing the
fork causes multiple regressions that we had fixed in the meantime.

Thankfully, it looks like this forking workaround is only necessary
when redirecting the output of built-ins. It appears that moving
workaround from io.c to the built-ins handling code in sh_exec() in
xec.c, right before calling sh_redirect(), allows reintroducing the
forking workaround for non-permanent redirections without causing
other regressions.

It would be better if the underlying cause of the hang were fixed
so the workaround becomes unnecessary, but I don't think that is
going to happen any time soon (AT&T didn't manage, either).

src/cmd/ksh93/sh/io.c: sh_redirect():
- Remove forking workaround for redirecting stdout in a comsub.

src/cmd/ksh93/sh/xec.c: sh_exec(): TCOM: built-ins handling code:
- Reimplement the workaround here.

Resolves: https://github.com/ksh93/ksh/issues/416
---
 src/cmd/ksh93/sh/io.c     | 12 ------------
 src/cmd/ksh93/sh/xec.c    | 19 +++++++++++++++++++
 src/cmd/ksh93/tests/io.sh | 14 ++++++++++++++
 3 files changed, 33 insertions(+), 12 deletions(-)

diff --git a/src/cmd/ksh93/sh/io.c b/src/cmd/ksh93/sh/io.c
index a7481dcb9..1e76e00d8 100644
--- a/src/cmd/ksh93/sh/io.c
+++ b/src/cmd/ksh93/sh/io.c
@@ -1132,18 +1132,6 @@ int	sh_redirect(struct ionod *iop, int flag)
 	{
 		iof=iop->iofile;
 		fn = (iof&IOUFD);
-		if(fn==1)
-		{
-			if(sh.subshell && sh.comsub && (flag==1 || flag==2))
-			{
-				if(sh.subshare)
-				{
-					errormsg(SH_DICT,ERROR_exit(1),"cannot redirect stdout inside shared-state comsub");
-					UNREACHABLE();
-				}
-				sh_subfork();
-			}
-		}
 		if(sh.redir0 && fn==0 && !(iof&IOMOV))
 			sh.redir0 = 2;
 		io_op[0] = '0'+(iof&IOUFD);
diff --git a/src/cmd/ksh93/sh/xec.c b/src/cmd/ksh93/sh/xec.c
index d0e16e4e2..5c673e828 100644
--- a/src/cmd/ksh93/sh/xec.c
+++ b/src/cmd/ksh93/sh/xec.c
@@ -1296,6 +1296,25 @@ int sh_exec(register const Shnode_t *t, int flags)
 								}
 							else
 								type = (execflg && !sh.subshell && !sh.st.trapcom[0]);
+							/*
+							 * A command substitution will hang on exit, writing infinite '\0', if,
+							 * within it, standard output (FD 1) is redirected for a built-in command
+							 * that calls sh_subfork(), or redirected permanently using 'exec' or
+							 * 'redirect'. This forking workaround is necessary to avoid that bug.
+							 * For shared-state comsubs, forking is incorrect, so error out then.
+							 * TODO: actually fix the bug and remove this workaround.
+							 */
+							if((io->iofile & IOUFD)==1 && sh.subshell && sh.comsub)
+							{
+								if(!sh.subshare)
+									sh_subfork();
+								else if(type==2)  /* block stdout perma-redirects: would hang */
+								{
+									errormsg(SH_DICT,ERROR_exit(1),"cannot redirect stdout"
+												" inside shared-state comsub");
+									UNREACHABLE();
+								}
+							}
 							sh.redir0 = 1;
 							sh_redirect(io,type);
 							for(item=buffp->olist;item;item=item->next)
diff --git a/src/cmd/ksh93/tests/io.sh b/src/cmd/ksh93/tests/io.sh
index 92fe16435..482be8aa6 100755
--- a/src/cmd/ksh93/tests/io.sh
+++ b/src/cmd/ksh93/tests/io.sh
@@ -905,5 +905,19 @@ then	for cmd in echo print printf
 	done
 fi
 
+# ======
+# Command substitution hangs, writing infinite zero bytes, when redirecting standard output on a built-in that forks
+# https://github.com/ksh93/ksh/issues/416
+exp='line'
+"$SHELL" -c 'echo "$(ulimit -t unlimited >/dev/null 2>&1; echo "ok $$")"' >out 2>&1 &
+pid=$!
+(sleep 1; kill -9 "$pid") 2>/dev/null &
+if	wait "$pid" 2>/dev/null
+then	kill "$!"  # the sleep process
+	[[ $(<out) == "ok $pid" ]] || err_exit "comsub fails after fork with stdout redirection" \
+		"(expected 'ok $pid', got $(printf %q "$(<out)"))"
+else	err_exit "comsub hangs after fork with stdout redirection"
+fi
+
 # ======
 exit $((Errors<125?Errors:125))