From ab5dedded7d08f8a954ab2c01672e6ae1897a04f Mon Sep 17 00:00:00 2001
From: Martijn Dekker <martijn@inlv.org>
Date: Sat, 12 Sep 2020 19:52:47 +0200
Subject: [PATCH] Work around process substitution file descriptor leak

File descriptors are not properly closed, causing a leak, when
using a process substitution as an argument to a shell function.
See: https://github.com/ksh93/ksh/issues/67

Process substitution uses /dev/fd/NN pseudofiles if the kernel
provides them. This is tested in src/cmd/ksh93/features/options
which causes SHOPT_DEVFD to be defined if /dev/fd/9 can be used.
If not, ksh uses a fallback mechanism involving a temporary FIFO,
which works on all Unix variants.

As it happens, the leak only occurs when using the /dev/fd
mechanism. So, until a fix is found, we can work around the bug by
disabling it. The FIFO mechanism might be slightly less robust,
but it's an improvement over leaking file descriptors. Plus, there
is room for improving it.

src/cmd/ksh93/include/defs.h:
- Unconditionally redefine SHOPT_DEVFD as 0 for now.

src/cmd/ksh93/sh/args.c: sh_argprocsub():
- pathtemp() does appropriate access checks using access(2), but
  there is an inherent race condition between calling it and
  mkfifo(). Make the FIFO mechanism more robust by handling errors,
  trying again if an error occurs that must have resulted from
  losing that race, e.g. file name conflict or temp dir
  permission/location change.
- Initially create the FIFO without any permissions, then chmod()
  the appropriate user read/write permissions. Since mkfifo()
  honours the umask and chmod() does not, this ensures that process
  substitution continues to work if a shell script sets a umask
  that disallows user read or write. (The /dev/fd/ mechanism does
  not care about the umask, so neither should the fallback.)
---
 src/cmd/ksh93/include/defs.h | 8 ++++++++
 src/cmd/ksh93/sh/args.c      | 9 +++++++--
 2 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/src/cmd/ksh93/include/defs.h b/src/cmd/ksh93/include/defs.h
index 59a4b28ca..92e437fd8 100644
--- a/src/cmd/ksh93/include/defs.h
+++ b/src/cmd/ksh93/include/defs.h
@@ -38,6 +38,14 @@
 #   define mbwide()	0
 #endif
 
+/*
+ * Until a file descriptor leak with process substitution
+ * is fixed, disable /dev/fd use to avoid the problem.
+ * https://github.com/ksh93/ksh/issues/67
+ */
+#undef SHOPT_DEVFD
+#define SHOPT_DEVFD	0
+
 #include	<sfio.h>
 #include	<error.h>
 #include	"FEATURE/externs"
diff --git a/src/cmd/ksh93/sh/args.c b/src/cmd/ksh93/sh/args.c
index db0556225..cb619abac 100644
--- a/src/cmd/ksh93/sh/args.c
+++ b/src/cmd/ksh93/sh/args.c
@@ -683,8 +683,13 @@ struct argnod *sh_argprocsub(Shell_t *shp,struct argnod *argp)
 	sh_pipe(pv);
 #else
 	pv[0] = -1;
-	shp->fifo = pathtemp(0,0,0,"ksh.fifo",0);
-	mkfifo(shp->fifo,S_IRUSR|S_IWUSR);
+	while((shp->fifo = pathtemp(0,0,0,"ksh.fifo",0), mkfifo(shp->fifo,0))<0)
+	{
+		if(errno==EEXIST || errno==EACCES || errno==ENOENT || errno==ENOTDIR || errno==EROFS)
+			continue;		/* lost race (name conflict or tmp dir change); try again */
+		errormsg(SH_DICT,ERROR_system(128),"process substitution: FIFO creation failed");
+	}
+	chmod(shp->fifo,S_IRUSR|S_IWUSR);	/* mkfifo + chmod works regardless of umask */
 	sfputr(shp->stk,shp->fifo,0);
 #endif /* SHOPT_DEVFD */
 	sfputr(shp->stk,fmtbase((long)pv[fd],10,0),0);