Fix various problems in close(2) usage and pwdfd handling

Changelog for this commit:

include/ast.h:
- Added ast_close, which endeavors to close fds while handling
  EINTR correctly, depending on platform specific behavior.
  The following three methods are implemented:
    1) Guarantee a close with posix_close(3p), which is the best
       available method. This function is still *very* new because
       it's a recent addition to POSIX.
    2) Add a version that only calls close once, then copes with EINTR
       by treating it as a non-error. Only Linux, FreeBSD and Cygwin
       use this right now. (Other OSes probably provide functionally
       equivalent leak-proof forms of close, but the documentation
       available for those is too vague on this matter. Perhaps I'll
       have to dive into the code for the open source ones.)
    3) Use the tried and true while loop to repeatedly invoke close,
       which is currently the generic fallback. On Linux, avoiding this
       method is effectively a bugfix, in large part because Linux can
       reuse a file descriptor after it's first closed before a second
       close call occurs, creating the potential for dangerous race
       conditions. For more information, please see the relevant LWN
       article et seq:
       https://lwn.net/Articles/576478/
- Delete many <errno.h> include directives in favor of a single one
  included in ast.h (ast_close uses errno and is implemented as a macro
  in the ast.h header).

src/subshell.c:
- Close all of the previous subshell pwdfds after a fork
  to fix a file descriptor leak (first introduced in
  93u+ 2012-07-27). Reproducer:
     $ cat /tmp/reproducer
     (cd /bin; (cd /usr; (cd /tmp; (cd /
         echo 'ls /proc/${.sh.pid}/fd; :' >/tmp/a
         echo '#/bin/sh' >/tmp/b
         cat /tmp/a >>/tmp/b
         chmod +x /tmp/{a,b}
         /tmp/a  # SHOPT_SPAWN=0
         /tmp/b  $ SHOPT_SPAWN=0
         (ulimit -t unlimited; source /tmp/a)
     ))))
     exit 0 # Avoid optimizing away subshells
- Initialize sp->pwdfd to -1 in all cases (this avoids the potential for
  incorrectly closing stdin).
  - While we're at it, switch to using C99 style struct initialization.

bltins/cd_pwd.c, sh/io.c:
- sh_close(): Set errno to zero to avoid passing on an inherited errno.
  The close(2) call might fail and subsequently set errno, which makes
  the previous behavior bogus.
- b_cd(): If the sh_close() calls in cd fail with anything other
  than EINTR, the errno from fchdir(2) is thrown away and replaced
  with close's errno. This can cause two regression test
  failures in builtins.sh. Prevent this by saving the errno
  and restoring it at the relevant sh_close calls.
  Error output (snipped to fit it into the commit log):
  builtins.sh[996]: FAIL: can cd into a directory without x
                    permission bit (absolute path arg) (expected
                    status 1 and match of ': cd: /tmp/ksh93.shtests
                    377539.24978/builtins.C/no_x_dir: \[?Permission
                    denied]?$', got status 1 and '<SNIP>/builtins.sh[994]:
                    cd: /tmp/ksh93.shtests.377539.24978/builtins.
                    /no_x_dir: Success')
  builtins.sh[1001]: FAIL: can cd into a directory without x permission bit
                     (relative path arg) (expected status 1 and match of
                     ': cd: no_x_dir: \[?Permission denied]?$', got status 1
                     and '<SNIP>/tests/builtins.sh[999]: cd: no_x_dir: No
                     such file or directory')
  This bug was noticed after sh_close was changed to reset errno
  before calling close.
- sh_diropenat(): If openat() managed to obtain a file descriptor >10
  on its first try and O_CLOEXEC is available, the fd wouldn't be
  registered with the shell because no sh_fcntl() call would be reached.
  Fix that by setting the file attributes in sh_diropenat() rather than
  sh_fcntl(). Consequently, we don't need to rely on sh_fcntl and thus
  we can use fcntl(2) directly. (cf. sh_open() in io.c)

bltins/test.c:
- test_stat(): If we need to stat e_dot (the current working directory)
  and sh.pwdfd is available, use fstat(2) because it will avoid
  unnecessarily reopening '.'. This change results in a small
  performance increase for cd(1) usage in subshells:
     $ cat bench/v-cdloop.ksh
     for ((i=0; i!=10000; i++)) do
        (cd /home; (cd /tmp; (cd /bin; (cd /usr; (cd /var)))))
     done; :
     $ shbench -b bench/v-cdloop.ksh -l 4 /tmp/ksh-{before,after}
     ------------------------------------------------------
     name          /tmp/ksh-before      /tmp/ksh-after
     ------------------------------------------------------
     v-cdloop.ksh  0.414 [0.412-0.416]  0.381 [0.341-0.398]
     ------------------------------------------------------

sh/fault.c:
- sh_done(): Delete an unnecessary close call backported from ksh93v-.
  The operating system closes all fds for us via exit(3), so a manual
  close(2) call only serves to make cleanup slower.

comp/arc4random.c:
- _ast_getentropy(): Fix a file descriptor leak when read(2) fails.

tm/tvtouch.c:
- tvtouch(): Add an else branch to avoid closing the same file
  descriptor twice.

tests/subshell.sh:
- Add a regression test capable of detecting the subshell pwdfd leak
  going back as far as ksh93u+ 2012-07-27, where the leak is incipient.

Author: JohnoKing

NEWS

@@ -2,6 +2,18 @@ This documents significant changes in the dev branch of ksh 93u+m.
 For full details, see the git log at: https://github.com/ksh93/ksh
 Uppercase BUG_* IDs are shell bug IDs as used by the Modernish shell library.
 
+2025-07-10:
+
+- Fixed a file descriptor leak introduced in 93u+ 2012-07-27 that occurred
+  when forking nested subshells.
+
+- Fixed a bug that could cause cd(1) to fail with the wrong error message
+  if the underlying close(2) calls failed after cd failed to change the
+  directory with fchdir(2).
+
+- Fixed a file descriptor leak that could occur on systems lacking a
+  useable implementation of arc4random(3).
+
 2025-06-14:
 
 - Fixed a bug occurring on systems with posix_spawn(3), spawnve(2), and

src/cmd/builtin/pty.c

@@ -313,7 +313,7 @@ mkpty(int* master, int* minion)
 		return -1;
 	if (grantpt(*master) || unlockpt(*master) || !(sname = ptsname(*master)) || (*minion = open(sname, O_RDWR|O_cloexec)) < 0)
 	{
-		close(*master);
+		ast_close(*master);
 		return -1;
 	}
 #else
@@ -325,8 +325,8 @@ mkpty(int* master, int* minion)
 		struct termios	tst;
 		if (tcgetattr(*minion, &tst) < 0 && (ioctl(*minion, I_PUSH, "ptem") < 0 || ioctl(*minion, I_PUSH, "ldterm") < 0))
 		{
-			close(*minion);
-			close(*master);
+			ast_close(*minion);
+			ast_close(*master);
 			return -1;
 		}
 	}
@@ -1121,7 +1121,7 @@ b_pty(int argc, char** argv, Shbltin_t* context)
 		error(ERROR_system(1), "unable run %s", argv[0]);
 		UNREACHABLE();
 	}
-	close(minion);
+	ast_close(minion);
 	if (messages)
 	{
 		drop = 1;
@@ -1136,10 +1136,10 @@ b_pty(int argc, char** argv, Shbltin_t* context)
 			error(ERROR_system(1), "%s: cannot redirect messages", messages);
 			UNREACHABLE();
 		}
-		close(2);
+		ast_close(2);
 		dup(fd);
 		if (drop)
-			close(fd);
+			ast_close(fd);
 	}
 	minion = (*fun)(mp, lp, delay, timeout);
 	master = procclose(proc);

src/cmd/ksh93/bltins/cd_pwd.c

@@ -69,9 +69,9 @@ int sh_diropenat(int dir, const char *path)
 	}
 	if(fd < 10)
 	{
-		/* Duplicate the fd and register it with the shell */
-		int shfd = sh_fcntl(fd, F_dupfd_cloexec, 10);
-		close(fd);
+		/* Duplicate the fd */
+		int shfd = fcntl(fd, F_dupfd_cloexec, 10);
+		ast_close(fd);
 		if(shfd < 0)
 			return shfd;
 		if(F_dupfd_cloexec == F_DUPFD)
@@ -81,7 +81,8 @@ int sh_diropenat(int dir, const char *path)
 	else if(O_cloexec == 0)
 		needs_cloexec = 1;
 	if(needs_cloexec)
-		sh_fcntl(fd,F_SETFD,FD_CLOEXEC);
+		fcntl(fd,F_SETFD,FD_CLOEXEC);
+	sh.fdstatus[fd] = (IOREAD|IOCLEX);
 	return fd;
 }
 #endif /* _lib_openat */
@@ -92,7 +93,7 @@ int	b_cd(int argc, char *argv[],Shbltin_t *context)
 	Pathcomp_t *cdpath = 0;
 	const char *dp;
 	int saverrno=0;
-	int rval,pflag=0,eflag=0,ret=1;
+	int rval,pflag=0,eflag=0,ret=1,saverr;
 	char *oldpwd, *cp;
 	Namval_t *opwdnod, *pwdnod;
 #if _lib_openat
@@ -240,7 +241,9 @@ int	b_cd(int argc, char *argv[],Shbltin_t *context)
 				sh_pwdupdate(newdirfd);
 				goto success;
 			}
+			saverr = errno;
 			sh_close(newdirfd);
+			errno = saverr;
 		}
 #if !O_SEARCH
 		else if((rval=chdir(cp)) >= 0)
@@ -268,7 +271,9 @@ int	b_cd(int argc, char *argv[],Shbltin_t *context)
 				sh_pwdupdate(newdirfd);
 				goto success;
 			}
+			saverr = errno;
 			sh_close(newdirfd);
+			errno = saverr;
 		}
 #if !O_SEARCH
 		else if((rval=chdir(dir)) >= 0)

src/cmd/ksh93/bltins/mkservice.c

@@ -204,7 +204,7 @@ static void process_stream(Sfio_t* iop)
 		r = (*sp->actionf)(sp, fd, 0);
 		service_list[fd] = sp;
 		if(r<0)
-			close(fd);
+			ast_close(fd);
 	}
 }
 
@@ -284,7 +284,7 @@ static int Accept(Service_t *sp, int accept_fd)
 	fd = fcntl(accept_fd, F_DUPFD, 10);
 	if (fd >= 0)
 	{
-		close(accept_fd);
+		ast_close(accept_fd);
 		if (nq)
 		{
 			char*	av[3];
@@ -295,7 +295,7 @@ static int Accept(Service_t *sp, int accept_fd)
 			sfsprintf(buff, sizeof(buff), "%d", fd);
 			if (sh_fun(nq, sp->node, av))
 			{
-				close(fd);
+				ast_close(fd);
 				return -1;
 			}
 		}
@@ -382,7 +382,7 @@ static void putval(Namval_t* np, const char* val, int flag, Namfun_t* fp)
 		{
 			if(service_list[i]==sp)
 			{
-				close(i);
+				ast_close(i);
 				if(--sp->refcount<=0)
 					break;
 			}
@@ -447,7 +447,7 @@ int	b_mkservice(int argc, char** argv, Shbltin_t *context)
 		UNREACHABLE();
 	}
 	if((sp->fd = fcntl(fd, F_DUPFD, 10))>=10)
-		close(fd);
+		ast_close(fd);
 	else
 		sp->fd = fd;
 	np = nv_open(var,sh.var_tree,NV_ARRAY|NV_VARNAME);

src/cmd/ksh93/bltins/test.c

@@ -706,7 +706,7 @@ static int test_mode(const char *file)
 }
 
 /*
- * do an fstat() for /dev/fd/n, otherwise stat()
+ * do an fstat() for /dev/fd/n or PWD's fd for better performance, otherwise stat()
  */
 static int test_stat(const char *name,struct stat *buff)
 {
@@ -715,8 +715,11 @@ static int test_stat(const char *name,struct stat *buff)
 		errno = ENOENT;
 		return -1;
 	}
+#if _lib_openat
+	if(name==e_dot && sh.pwdfd > -1)
+		return fstat(sh.pwdfd,buff);
+#endif
 	if(sh_isdevfd(name))
 		return fstat((int)strtol(name+8, NULL, 10),buff);
-	else
-		return stat(name,buff);
+	return stat(name,buff);
 }

src/cmd/ksh93/data/msg.c

@@ -26,7 +26,6 @@
 
 #include	"shopt.h"
 #include	<ast.h>
-#include	<errno.h>
 #include	"defs.h"
 #include	"path.h"
 #include	"io.h"

src/cmd/ksh93/edit/edit.c

@@ -29,7 +29,6 @@
 
 #include	"shopt.h"
 #include	<ast.h>
-#include	<errno.h>
 #include	<fault.h>
 #include	"FEATURE/time"
 #if _hdr_utime

src/cmd/ksh93/features/fchdir

@@ -40,7 +40,6 @@ tst	fchdir_osearch_compat note{ can fchdir(2) use file descriptors obtained usin
 tst	openat_enotdir note{ does openat set ENOTDIR when it fails because the file isn't a directory }end output{
 	#include <ast.h>
 	#include <stdio.h>
-	#include <errno.h>
 	#if !_lib_openat
 	#error openat(2) doesn't exist, so this test is pointless
 	#endif

src/cmd/ksh93/features/poll

@@ -29,7 +29,7 @@ tst	pipe_socketpair note{ use socketpair() for peekable pipe() }end execute{
 		char		buf[256];
 		pid_t		pid;
 		static char	msg[] = "hello world\n";
-		close(0);
+		ast_close(0);
 		if (pipe(pfd) < 0 ||
 		    socketpair(AF_UNIX, SOCK_STREAM, 0, sfd) < 0 ||
 		    shutdown(sfd[1], SHUT_RD) < 0 ||
@@ -39,29 +39,29 @@ tst	pipe_socketpair note{ use socketpair() for peekable pipe() }end execute{
 			return 1;
 		if (pid)
 		{
-			close(pfd[1]);
-			close(sfd[1]);
+			ast_close(pfd[1]);
+			ast_close(sfd[1]);
 			wait(&n);
 			if (sfpkrd(pfd[0], buf, sizeof(buf), '\n', -1, 1) >= 0 ||
 			    sfpkrd(sfd[0], buf, sizeof(buf), '\n', -1, 1) < 0)
 				return 1;
 		}
 		else
 		{
-			close(pfd[0]);
-			close(sfd[0]);
+			ast_close(pfd[0]);
+			ast_close(sfd[0]);
 			write(pfd[1], msg, sizeof(msg) - 1);
 			write(sfd[1], msg, sizeof(msg) - 1);
 			return 0;
 		}
-		close(pfd[0]);
-		close(sfd[0]);
+		ast_close(pfd[0]);
+		ast_close(sfd[0]);
 		signal(SIGPIPE, handler);
 		if (socketpair(AF_UNIX, SOCK_STREAM, 0, sfd) < 0 ||
 		    shutdown(sfd[1], SHUT_RD) < 0 ||
 		    shutdown(sfd[0], SHUT_WR) < 0)
 			return 1;
-		close(sfd[0]);
+		ast_close(sfd[0]);
 		write(sfd[1], msg, sizeof(msg) - 1);
 		return 1;
 	}
@@ -75,18 +75,18 @@ tst	socketpair_devfd note{ /dev/fd/N handles socketpair() }end execute{
 		int		devfd;
 		int		n;
 		int		sfd[2];
-		close(0);
+		ast_close(0);
 		open("/dev/null", O_RDONLY);
 		if ((n = open("/dev/fd/0", O_RDONLY)) < 0)
 			return 1;
-		close(n);
+		ast_close(n);
 		if (socketpair(AF_UNIX, SOCK_STREAM, 0, sfd) < 0 ||
 		    shutdown(sfd[0], 1) < 0 ||
 		    shutdown(sfd[1], 0) < 0)
 			return 1;
-		close(0);
+		ast_close(0);
 		dup(sfd[0]);
-		close(sfd[0]);
+		ast_close(sfd[0]);
 		if ((n = open("/dev/fd/0", O_RDONLY)) < 0)
 			return 1;
 		return 0;

src/cmd/ksh93/include/defs.h

@@ -141,6 +141,7 @@ extern int		sh_trace(char*[],int);
 extern void		sh_trim(char*);
 extern int		sh_type(const char*);
 extern void             sh_unscope(void);
+extern void		sh_clear_subshell_pwdfd(void);
 #if _lib_openat
     extern int		sh_diropenat(int,const char *);
     extern void		sh_pwdupdate(int);