From 3cd661320bdd2fc344d0903e018309799b49df02 Mon Sep 17 00:00:00 2001 From: Johnothan King Date: Thu, 10 Jul 2025 16:54:59 -0700 Subject: [PATCH 1/2] 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 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 '/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 '/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. --- NEWS | 12 ++++++++ src/cmd/builtin/pty.c | 12 ++++---- src/cmd/ksh93/bltins/cd_pwd.c | 15 ++++++--- src/cmd/ksh93/bltins/mkservice.c | 10 +++--- src/cmd/ksh93/bltins/test.c | 9 ++++-- src/cmd/ksh93/data/msg.c | 1 - src/cmd/ksh93/edit/edit.c | 1 - src/cmd/ksh93/features/fchdir | 1 - src/cmd/ksh93/features/poll | 24 +++++++-------- src/cmd/ksh93/include/defs.h | 1 + src/cmd/ksh93/include/version.h | 2 +- src/cmd/ksh93/sh/fault.c | 4 --- src/cmd/ksh93/sh/io.c | 13 ++++---- src/cmd/ksh93/sh/jobs.c | 3 +- src/cmd/ksh93/sh/main.c | 2 +- src/cmd/ksh93/sh/path.c | 6 ++-- src/cmd/ksh93/sh/subshell.c | 52 +++++++++++++++++++++++++------- src/cmd/ksh93/sh/xec.c | 12 +++----- src/cmd/ksh93/tests/subshell.sh | 30 ++++++++++++++++++ src/lib/libast/aso/aso-fcntl.c | 4 +-- src/lib/libast/comp/arc4random.c | 3 +- src/lib/libast/comp/eaccess.c | 1 - src/lib/libast/comp/spawnveg.c | 4 +-- src/lib/libast/dir/dirlib.h | 1 - src/lib/libast/dir/opendir.c | 4 +-- src/lib/libast/disc/sfdcfilter.c | 2 +- src/lib/libast/features/lib | 2 +- src/lib/libast/include/ast.h | 29 ++++++++++++++++++ src/lib/libast/include/error.h | 1 - src/lib/libast/misc/fastfind.c | 2 +- src/lib/libast/misc/procclose.c | 4 +-- src/lib/libast/misc/proclib.h | 1 - src/lib/libast/misc/procopen.c | 50 +++++++++++++++--------------- src/lib/libast/misc/univlib.h | 1 - src/lib/libast/path/pathicase.c | 4 +-- src/lib/libast/port/astconf.c | 2 +- src/lib/libast/port/astwinsize.c | 2 +- src/lib/libast/regex/reglib.h | 1 - src/lib/libast/sfio/_sfopen.c | 6 ++-- src/lib/libast/sfio/sfclose.c | 12 +++----- src/lib/libast/sfio/sfhdr.h | 3 -- src/lib/libast/sfio/sfmode.c | 2 +- src/lib/libast/sfio/sfsetfd.c | 2 +- src/lib/libast/sfio/sftmp.c | 2 +- src/lib/libast/tm/tvsettime.c | 1 - src/lib/libast/tm/tvtouch.c | 9 +++--- src/lib/libcmd/cp.c | 12 ++++---- src/lib/libcmd/mktemp.c | 2 +- src/lib/libcmd/rm.c | 2 +- src/lib/libcmd/tee.c | 2 +- 50 files changed, 236 insertions(+), 147 deletions(-) diff --git a/NEWS b/NEWS index 81c063c046d0..caf631189db7 100644 --- a/NEWS +++ b/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 diff --git a/src/cmd/builtin/pty.c b/src/cmd/builtin/pty.c index 1f464dced638..a874874aaad1 100644 --- a/src/cmd/builtin/pty.c +++ b/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); diff --git a/src/cmd/ksh93/bltins/cd_pwd.c b/src/cmd/ksh93/bltins/cd_pwd.c index 66a49780ad9d..a168c6324c90 100644 --- a/src/cmd/ksh93/bltins/cd_pwd.c +++ b/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) diff --git a/src/cmd/ksh93/bltins/mkservice.c b/src/cmd/ksh93/bltins/mkservice.c index b60980b2ffff..4d96be8aa839 100644 --- a/src/cmd/ksh93/bltins/mkservice.c +++ b/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); diff --git a/src/cmd/ksh93/bltins/test.c b/src/cmd/ksh93/bltins/test.c index 610b56d470bd..41a2b50b4a43 100644 --- a/src/cmd/ksh93/bltins/test.c +++ b/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); } diff --git a/src/cmd/ksh93/data/msg.c b/src/cmd/ksh93/data/msg.c index 75dfa39166a1..ea26e15004eb 100644 --- a/src/cmd/ksh93/data/msg.c +++ b/src/cmd/ksh93/data/msg.c @@ -26,7 +26,6 @@ #include "shopt.h" #include -#include #include "defs.h" #include "path.h" #include "io.h" diff --git a/src/cmd/ksh93/edit/edit.c b/src/cmd/ksh93/edit/edit.c index c0eea1b15f15..38fd240e1e5f 100644 --- a/src/cmd/ksh93/edit/edit.c +++ b/src/cmd/ksh93/edit/edit.c @@ -29,7 +29,6 @@ #include "shopt.h" #include -#include #include #include "FEATURE/time" #if _hdr_utime diff --git a/src/cmd/ksh93/features/fchdir b/src/cmd/ksh93/features/fchdir index bf098db4dee5..aef7ce3bd77f 100644 --- a/src/cmd/ksh93/features/fchdir +++ b/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 #include - #include #if !_lib_openat #error openat(2) doesn't exist, so this test is pointless #endif diff --git a/src/cmd/ksh93/features/poll b/src/cmd/ksh93/features/poll index 3e4bd11e4b1f..9fa23f7dfbab 100644 --- a/src/cmd/ksh93/features/poll +++ b/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,8 +39,8 @@ 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) @@ -48,20 +48,20 @@ tst pipe_socketpair note{ use socketpair() for peekable pipe() }end execute{ } 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; diff --git a/src/cmd/ksh93/include/defs.h b/src/cmd/ksh93/include/defs.h index ca20ab43ff08..4ba7fdc22698 100644 --- a/src/cmd/ksh93/include/defs.h +++ b/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); diff --git a/src/cmd/ksh93/include/version.h b/src/cmd/ksh93/include/version.h index 845e905acb8f..79e21378aa7e 100644 --- a/src/cmd/ksh93/include/version.h +++ b/src/cmd/ksh93/include/version.h @@ -18,7 +18,7 @@ #include #include "git.h" -#define SH_RELEASE_DATE "2025-06-22" /* must be in this format for $((.sh.version)) */ +#define SH_RELEASE_DATE "2025-07-10" /* must be in this format for $((.sh.version)) */ /* * This comment keeps SH_RELEASE_DATE a few lines away from SH_RELEASE_SVER to avoid * merge conflicts when cherry-picking dev branch commits onto a release branch. diff --git a/src/cmd/ksh93/sh/fault.c b/src/cmd/ksh93/sh/fault.c index 950c6f08bbe4..1f010f9d9db7 100644 --- a/src/cmd/ksh93/sh/fault.c +++ b/src/cmd/ksh93/sh/fault.c @@ -685,10 +685,6 @@ noreturn void sh_done(int sig) if(sh_isoption(SH_NOEXEC)) kiaclose((Lex_t*)sh.lex_context); #endif /* SHOPT_KIA */ -#if _lib_openat - if(sh.pwdfd > 0) - close(sh.pwdfd); -#endif /* _lib_openat */ /* Exit with portable 8-bit status (128 + signum) if last child process exits due to signal */ if(sh.chldexitsig) savxit = savxit & ~SH_EXITSIG | 0200; diff --git a/src/cmd/ksh93/sh/io.c b/src/cmd/ksh93/sh/io.c index 0fa7cf2ffd26..bdb853e1a9b1 100644 --- a/src/cmd/ksh93/sh/io.c +++ b/src/cmd/ksh93/sh/io.c @@ -301,7 +301,7 @@ inetopen(const char* path, int flags) { if (server && !bind(fd, p->ai_addr, p->ai_addrlen) && !listen(fd, 5) || !server && !connect(fd, p->ai_addr, p->ai_addrlen)) goto done; - close(fd); + ast_close(fd); fd = -1; if (errno != EINTR) break; @@ -714,11 +714,12 @@ int sh_close(int fd) sh_iovalidfd(fd); if(!(sp=sh.sftable[fd]) || sfclose(sp) < 0) { - int err=errno; if(fdnotify) (*fdnotify)(fd,SH_FDCLOSE); - while((r=close(fd)) < 0 && errno==EINTR) - errno = err; + errno = 0; + ast_close(fd); + if(errno) + r = -1; } if(fd>2) sh.sftable[fd] = 0; @@ -913,7 +914,7 @@ int sh_iomovefd(int fdold) if((sh.fdstatus[fdold]&IOCLEX) && F_dupfd_cloexec == F_DUPFD) fcntl(fdnew,F_SETFD,FD_CLOEXEC); sh.fdstatus[fdnew] = sh.fdstatus[fdold]; - close(fdold); + ast_close(fdold); sh.fdstatus[fdold] = IOCLOSE; return fdnew; } @@ -1075,7 +1076,7 @@ static char *io_usename(char *name, int *perm, int fno, int mode) if((fd = sh_open(name,O_RDONLY,0)) >= 0) { r = fstat(fd,&statb); - close(fd); + sh_close(fd); if(r) return 0; if(!S_ISREG(statb.st_mode)) diff --git a/src/cmd/ksh93/sh/jobs.c b/src/cmd/ksh93/sh/jobs.c index f98d273b3d20..15219cbc1a31 100644 --- a/src/cmd/ksh93/sh/jobs.c +++ b/src/cmd/ksh93/sh/jobs.c @@ -517,8 +517,7 @@ void job_init(void) char *ttynam; if(job.mypgid<0 || !(ttynam=ttyname(JOBTTY))) return; - while(close(JOBTTY)<0 && errno==EINTR) - ; + ast_close(JOBTTY); if((fd = open(ttynam,O_RDWR)) <0) return; if(fd!=JOBTTY) diff --git a/src/cmd/ksh93/sh/main.c b/src/cmd/ksh93/sh/main.c index b52b149e6d4d..166a8dad018e 100644 --- a/src/cmd/ksh93/sh/main.c +++ b/src/cmd/ksh93/sh/main.c @@ -236,7 +236,7 @@ noreturn void sh_main(int ac, char *av[], Shinit_f userinit) int isdir = 0; if((fdin=sh_open(name,O_RDONLY,0))>=0 &&(fstat(fdin,&statb)<0 || S_ISDIR(statb.st_mode))) { - close(fdin); + sh_close(fdin); isdir = 1; fdin = -1; } diff --git a/src/cmd/ksh93/sh/path.c b/src/cmd/ksh93/sh/path.c index 298bbd2e2b60..ea3ca2a5d16f 100644 --- a/src/cmd/ksh93/sh/path.c +++ b/src/cmd/ksh93/sh/path.c @@ -1417,7 +1417,7 @@ static noreturn void exscript(char *path,char *argv[]) sabuf.ac_etime = compress( (time_t)(after-before)); fd = open( SHACCT , O_WRONLY | O_APPEND | O_CREAT,RW_ALL); write(fd, (const char*)&sabuf, sizeof( sabuf )); - close( fd); + ast_close(fd); } } /* @@ -1517,7 +1517,7 @@ static int checkdotpaths(Pathcomp_t *first, Pathcomp_t* old,Pathcomp_t *pp, int if(!S_ISREG(statb.st_mode)) { /* .paths cannot be a directory */ - close(fd); + ast_close(fd); return 0; } n = statb.st_size; @@ -1526,7 +1526,7 @@ static int checkdotpaths(Pathcomp_t *first, Pathcomp_t* old,Pathcomp_t *pp, int *sp++ = '/'; n=read(fd,cp=sp,n); sp[n] = 0; - close(fd); + ast_close(fd); for(ep=0; n--; cp++) { if(*cp=='=') diff --git a/src/cmd/ksh93/sh/subshell.c b/src/cmd/ksh93/sh/subshell.c index 0b51ee470a2e..63df6cc05caf 100644 --- a/src/cmd/ksh93/sh/subshell.c +++ b/src/cmd/ksh93/sh/subshell.c @@ -109,7 +109,7 @@ void sh_subtmpfile(void) { if(F_dupfd_cloexec == F_DUPFD) sh_fcntl(fd,F_SETFD,FD_CLOEXEC); - close(1); + ast_close(1); } else if(errno!=EBADF) { @@ -486,6 +486,39 @@ int sh_validate_subpwdfd(void) #endif /* _lib_openat */ +/* + * Close all prior file descriptors for the subshell PWDs no longer in use. + * This is always called after forking to avoid leaking unused file descriptors + * to the child processes. + */ +void sh_clear_subshell_pwdfd(void) +{ + struct subshell *sp = subshell_data; +#if _lib_openat + int prevfd = sh.pwdfd; + while(sp) + { + if(sp->pwdfd > -1 && sp->pwdfd != prevfd) + { + sh_close(sp->pwdfd); + prevfd = sp->pwdfd; + } + sp = sp->prev; + } +#else + while(sp) + { + if(sp->pwdclose) + { + sh_close(sp->pwdfd); + sp->pwdclose = 0; + sp->pwdfd = -1; + } + sp = sp->prev; + } +#endif /* _lib_openat */ +} + /* * Run command tree in a virtual subshell * If comsub is not null, then output will be placed in temp file (or buffer) @@ -494,7 +527,12 @@ int sh_validate_subpwdfd(void) */ Sfio_t *sh_subshell(Shnode_t *t, volatile int flags, int comsub) { - struct subshell sub_data; + struct subshell sub_data = { + .options = sh.options, + .subshare = sh.subshare, + .comsub = sh.comsub, + .pwdfd = -1 /* pwdfd should not be initialized to stdin */ + }; struct subshell *sp = &sub_data; int jmpval,isig,nsig=0,fatalerror=0,saveerrno=0; unsigned int savecurenv = sh.curenv; @@ -506,7 +544,6 @@ Sfio_t *sh_subshell(Shnode_t *t, volatile int flags, int comsub) struct sh_scoped savst; struct dolnod *argsav=0; int argcnt; - memset((char*)sp, 0, sizeof(*sp)); sfsync(sh.outpool); sh_sigcheck(); sh.savesig = -1; @@ -523,9 +560,7 @@ Sfio_t *sh_subshell(Shnode_t *t, volatile int flags, int comsub) sh.subshell++; /* increase level of virtual subshells */ sh.realsubshell++; /* increase ${.sh.subshell} */ sp->prev = subshell_data; - sp->sig = 0; subshell_data = sp; - sp->options = sh.options; sp->jobs = job_subsave(); /* make sure initialization has occurred */ if(!sh.pathlist) @@ -543,8 +578,6 @@ Sfio_t *sh_subshell(Shnode_t *t, volatile int flags, int comsub) sh_stats(STAT_COMSUB); else job.curpgid = 0; - sp->subshare = sh.subshare; - sp->comsub = sh.comsub; sh.subshare = comsub==2; if(!sh.subshare) sp->pathlist = path_dup((Pathcomp_t*)sh.pathlist); @@ -558,7 +591,6 @@ Sfio_t *sh_subshell(Shnode_t *t, volatile int flags, int comsub) sp->pwdfd = sh.pwdfd; #else struct subshell *xp; - sp->pwdfd = -1; for(xp=sp->prev; xp; xp=xp->prev) { if(xp->pwdfd>0 && xp->pwd && strcmp(xp->pwd,sh.pwd)==0) @@ -758,9 +790,7 @@ Sfio_t *sh_subshell(Shnode_t *t, volatile int flags, int comsub) /* check if standard output was preserved */ if(sp->tmpfd>=0) { - int err=errno; - while(close(1)<0 && errno==EINTR) - errno = err; + ast_close(1); if (fcntl(sp->tmpfd,F_DUPFD,1) != 1) { saveerrno = errno; diff --git a/src/cmd/ksh93/sh/xec.c b/src/cmd/ksh93/sh/xec.c index 1f78e3a738f4..4031547f93d7 100644 --- a/src/cmd/ksh93/sh/xec.c +++ b/src/cmd/ksh93/sh/xec.c @@ -699,14 +699,13 @@ static void unset_instance(Namval_t *node, struct Namref *nr, long mode) #if SHOPT_FILESCAN static Sfio_t *openstream(struct ionod *iop, int *save) { - int err = errno, savein, fd = sh_redirect(iop,3); + int savein, fd = sh_redirect(iop,3); Sfio_t *sp; savein = dup(0); if(fd==0) fd = savein; sp = sfnew(NULL,NULL,SFIO_UNBOUND,fd,SFIO_READ); - while(close(0)<0 && errno==EINTR) - errno = err; + ast_close(0); open(e_devnull,O_RDONLY); sh.offsets[0] = -1; sh.offsets[1] = 0; @@ -2138,10 +2137,8 @@ int sh_exec(const Shnode_t *t, int flags) #if SHOPT_FILESCAN if(iop) { - int err=errno; sfclose(iop); - while(close(0)<0 && errno==EINTR) - errno = err; + ast_close(0); dup(savein); sh.cur_line = 0; } @@ -2810,6 +2807,7 @@ pid_t _sh_fork(pid_t parent,int flags,int *jobid) /* except for those `lost' by trap */ if(!(flags&FSHOWME)) sh_sigreset(2); + sh_clear_subshell_pwdfd(); sh.realsubshell++; /* increase ${.sh.subshell} */ sh.subshell = 0; /* zero virtual subshells */ sh.comsub = 0; @@ -3238,7 +3236,7 @@ static void coproc_init(int pipes[]) sh.fdstatus[fd] = sh.fdstatus[outfd]|IOCLEX; else sh.fdstatus[fd] = (sh.fdstatus[outfd]&~IOCLEX); - close(outfd); + ast_close(outfd); sh.fdstatus[outfd] = IOCLOSE; sh.cpipe[1] = fd; } diff --git a/src/cmd/ksh93/tests/subshell.sh b/src/cmd/ksh93/tests/subshell.sh index 911ee07a52a7..5ead3583adf5 100755 --- a/src/cmd/ksh93/tests/subshell.sh +++ b/src/cmd/ksh93/tests/subshell.sh @@ -1219,5 +1219,35 @@ got=$("$SHELL" -c 'x=$(fn(){ return 265; };echo ok|fn); echo exited $?' 2>&1) [[ e=$? -eq 0 && $got == "$exp" ]] || err_exit "regression involving SIGPIPE in subshell" \ "(expected status 0 and $(printf %q "$exp"), got status $e and $(printf %q "$got"))" +# ====== +# The saved file descriptors for the PWD of virtual subshells is leaked +# when forking the subshell or executing a script. (The regression test +# uses cd in a virtual subshell to trigger the bug in 93u+m and a redirect +# command to expose it in 93u+.) +nam1=$tmp/fdleaka.$SRANDOM +nam2=$tmp/fdleakb.$SRANDOM +dir1=$tmp/dir1.$SRANDOM +dir2=$tmp/dir2.$SRANDOM +dir3=$tmp/dir3.$SRANDOM +dir4=$tmp/dir4.$SRANDOM +dir5=$tmp/dir5.$SRANDOM +dir6=$tmp/dir6.$SRANDOM +dir7=$tmp/dir7.$SRANDOM +mkdir "$dir1" "$dir2" "$dir3" "$dir4" "$dir5" "$dir6" "$dir7" +exp=ok +got=$(set +x; "$SHELL" -c " +(cd '$dir1'; (cd '$dir2'; (cd '$dir3'; (cd '$dir4'; (cd '$dir5'; (cd '$dir6'; (cd '$dir7' + echo 'ulimit -n 14 && (cd /; redirect 2>&1) && true' >'$nam1' + echo '#/bin/sh' >'$nam2' + cat '$nam1' >>'$nam2' + chmod +x '$nam1' '$nam2' + '$nam1' + '$nam2' + (ulimit -t unlimited; . '$nam1') +))))))) +echo ok" 2>&1) +[[ $exp == $got ]] || err_exit "PWD file descriptors made in virtual subshells leak out of subshells" \ + "(expected $(printf %q "$exp"), got $(printf %q "$got"))" + # ====== exit $((Errors<125?Errors:125)) diff --git a/src/lib/libast/aso/aso-fcntl.c b/src/lib/libast/aso/aso-fcntl.c index abc8227f563e..a4f68b9cc591 100644 --- a/src/lib/libast/aso/aso-fcntl.c +++ b/src/lib/libast/aso/aso-fcntl.c @@ -77,7 +77,7 @@ aso_init_fcntl(void* data, const char* details) if (!references) remove(apl->path); } - close(apl->fd); + ast_close(apl->fd); free(apl); return NULL; } @@ -152,7 +152,7 @@ aso_init_fcntl(void* data, const char* details) if (apl) free(apl); if (fd >= 0) - close(fd); + ast_close(fd); if (drop) remove(path); return NULL; diff --git a/src/lib/libast/comp/arc4random.c b/src/lib/libast/comp/arc4random.c index 04cd34bda25c..0fda3ffc7f83 100644 --- a/src/lib/libast/comp/arc4random.c +++ b/src/lib/libast/comp/arc4random.c @@ -171,11 +171,12 @@ _ast_getentropy(void *s, size_t len) { if (errno == EAGAIN || errno == EINTR || errno == EWOULDBLOCK) continue; + ast_close(fd); return -1; } o += r; } - close(fd); + ast_close(fd); return 0; #else return -1; diff --git a/src/lib/libast/comp/eaccess.c b/src/lib/libast/comp/eaccess.c index 5afd2ecf8c22..6aabdb15e01a 100644 --- a/src/lib/libast/comp/eaccess.c +++ b/src/lib/libast/comp/eaccess.c @@ -22,7 +22,6 @@ */ #include -#include #include #include "FEATURE/eaccess" diff --git a/src/lib/libast/comp/spawnveg.c b/src/lib/libast/comp/spawnveg.c index f4efe46791cf..f6158238a3e1 100644 --- a/src/lib/libast/comp/spawnveg.c +++ b/src/lib/libast/comp/spawnveg.c @@ -219,7 +219,7 @@ spawnveg(const char* path, char* const argv[], char* const envv[], pid_t pgid, i rid = pid; if (err[0] != -1) { - close(err[1]); + ast_close(err[1]); if (pid != -1) { m = 0; @@ -236,7 +236,7 @@ spawnveg(const char* path, char* const argv[], char* const envv[], pid_t pgid, i n = m; } } - close(err[0]); + ast_close(err[0]); } sigcritical(0); if (pid != -1 && pgid > 0) diff --git a/src/lib/libast/dir/dirlib.h b/src/lib/libast/dir/dirlib.h index 1093489901e1..81548d25bf9a 100644 --- a/src/lib/libast/dir/dirlib.h +++ b/src/lib/libast/dir/dirlib.h @@ -32,7 +32,6 @@ #define getdirentries ______getdirentries #include -#include #if _lib_opendir && ( _hdr_dirent || _hdr_ndir || _sys_dir ) diff --git a/src/lib/libast/dir/opendir.c b/src/lib/libast/dir/opendir.c index 28a15ef99fa5..dcb960f7d07f 100644 --- a/src/lib/libast/dir/opendir.c +++ b/src/lib/libast/dir/opendir.c @@ -62,7 +62,7 @@ opendir(const char* path) #endif )) { - close(fd); + ast_close(fd); if (dirp) { if (!freedirp) freedirp = dirp; @@ -84,7 +84,7 @@ closedir(DIR* dirp) { if (dirp) { - close(dirp->dd_fd); + ast_close(dirp->dd_fd); if (!freedirp) freedirp = dirp; else free(dirp); } diff --git a/src/lib/libast/disc/sfdcfilter.c b/src/lib/libast/disc/sfdcfilter.c index e83067b8eb07..2098aa74465e 100644 --- a/src/lib/libast/disc/sfdcfilter.c +++ b/src/lib/libast/disc/sfdcfilter.c @@ -53,7 +53,7 @@ static ssize_t filterread(Sfio_t* f, /* stream reading from */ else { /* eof, close write end of pipes */ sfset(fi->filter,SFIO_READ,0); - close(sffileno(fi->filter)); + ast_close(sffileno(fi->filter)); sfset(fi->filter,SFIO_READ,1); fi->next = fi->endb = NULL; } diff --git a/src/lib/libast/features/lib b/src/lib/libast/features/lib index 4a557acd539e..b18959f2f4b5 100644 --- a/src/lib/libast/features/lib +++ b/src/lib/libast/features/lib @@ -31,7 +31,7 @@ lib gethostname,getpagesize,getrlimit,getuniverse lib glob,iswblank,iswctype,killpg,link,localeconv,madvise lib mbtowc,mbrtowc,memalign,memdup lib mktemp,mktime -lib opendir,openat,pathconf,pipe2 +lib opendir,openat,pathconf,pipe2,posix_close lib rand_r lib rewinddir,setlocale lib setpgrp,setpgrp2,setreuid,setuid diff --git a/src/lib/libast/include/ast.h b/src/lib/libast/include/ast.h index 9f2897c9e6b4..d615ca9cba85 100644 --- a/src/lib/libast/include/ast.h +++ b/src/lib/libast/include/ast.h @@ -36,6 +36,8 @@ #include #endif +#include + /* * All the ast.* global state variables are actually stored in _ast_info. * It's defined/initialized in misc/state.c and the struct is defined in include/ast_std.h. @@ -425,6 +427,33 @@ extern int strvcmp(const char*, const char*); extern size_t utf32toutf8(char*, uint32_t); #endif /* !AST_NOMULTIBYTE */ +/* + * Depending on the implementation, close(2) must either: + * - *Never* be used after EINTR (vide Linux man pages). + * - *Always* be used after EINTR (that's the generic fallback). + * + * What follows are macros that attempt to conform to the required + * behavior for the operating systems ksh supports. + */ +#if _lib_posix_close +/* This function is quite new, but it's ultimately the best option if available */ +#define ast_close(fd) posix_close(fd, 0) +#elif defined(__linux__) || defined(__FreeBSD__) || _WINIX +/* Never try again after EINTR */ +#define ast_close(fd) do { \ + int _cerr = errno; \ + if(close(fd)<0 && errno==EINTR) \ + errno = _cerr; \ + } while(0) +#else +/* Always try again after EINTR */ +#define ast_close(fd) do { \ + int _cerr = errno; \ + while(close(fd)<0 && errno==EINTR) \ + errno = _cerr; \ + } while(0) +#endif + /* * backward compat */ diff --git a/src/lib/libast/include/error.h b/src/lib/libast/include/error.h index b9f1a095e372..9aea49138fe9 100644 --- a/src/lib/libast/include/error.h +++ b/src/lib/libast/include/error.h @@ -29,7 +29,6 @@ #include #include -#include #define ERROR_VERSION 20230222L diff --git a/src/lib/libast/misc/fastfind.c b/src/lib/libast/misc/fastfind.c index 31713ca6e988..60b420c55a4c 100644 --- a/src/lib/libast/misc/fastfind.c +++ b/src/lib/libast/misc/fastfind.c @@ -303,7 +303,7 @@ findopen(const char* file, const char* pattern, const char* type, Finddisc_t* di { if (fp->disc->errorf) (*fp->disc->errorf)(fp, fp->disc, ERROR_SYSTEM|2, "%s: cannot open tmp file", fp->encode.temp); - close(fd); + ast_close(fd); goto drop; } if (fp->disc->flags & FIND_TYPE) diff --git a/src/lib/libast/misc/procclose.c b/src/lib/libast/misc/procclose.c index 1e387f5a4107..f315ec0c835b 100644 --- a/src/lib/libast/misc/procclose.c +++ b/src/lib/libast/misc/procclose.c @@ -36,9 +36,9 @@ procclose(Proc_t* p) if (p) { if (p->rfd >= 0) - close(p->rfd); + ast_close(p->rfd); if (p->wfd >= 0 && p->wfd != p->rfd) - close(p->wfd); + ast_close(p->wfd); if (p->flags & PROC_ORPHAN) status = 0; else diff --git a/src/lib/libast/misc/proclib.h b/src/lib/libast/misc/proclib.h index 751498416e23..c2b007c06c0d 100644 --- a/src/lib/libast/misc/proclib.h +++ b/src/lib/libast/misc/proclib.h @@ -27,7 +27,6 @@ #define _PROCLIB_H #include -#include #include #include diff --git a/src/lib/libast/misc/procopen.c b/src/lib/libast/misc/procopen.c index c53291c9fe19..38657fb25248 100644 --- a/src/lib/libast/misc/procopen.c +++ b/src/lib/libast/misc/procopen.c @@ -175,19 +175,19 @@ modify(Proc_t* proc, int forked, int op, long arg1, long arg2) { if (arg2 != PROC_ARG_NULL) { - close(arg2); + ast_close(arg2); if (fcntl(arg1, F_DUPFD, arg2) != arg2) return -1; } if (op & PROC_FD_CHILD) - close(arg1); + ast_close(arg1); } break; case PROC_fd_ctty: setsid(); for (i = 0; i <= 2; i++) if (arg1 != i) - close(i); + ast_close(i); arg2 = -1; #ifdef TIOCSCTTY if (ioctl(arg1, TIOCSCTTY, NULL) < 0) @@ -202,9 +202,9 @@ modify(Proc_t* proc, int forked, int op, long arg1, long arg2) if (arg1 != i && arg2 != i && fcntl(arg1, F_DUPFD, i) != i) return -1; if (arg1 > 2) - close(arg1); + ast_close(arg1); if (arg2 > 2) - close(arg2); + ast_close(arg2); break; case PROC_sig_dfl: signal(arg1, SIG_DFL); @@ -260,11 +260,11 @@ modify(Proc_t* proc, int forked, int op, long arg1, long arg2) #if F_dupfd_cloexec == F_DUPFD fcntl(m->save, F_SETFD, FD_CLOEXEC); #endif /* F_dupfd_cloexec == F_DUPFD */ - close(arg2); + ast_close(arg2); if (fcntl(arg1, F_DUPFD, arg2) != arg2) return -1; if (op & PROC_FD_CHILD) - close(arg1); + ast_close(arg1); } else if (op & PROC_FD_CHILD) { @@ -336,21 +336,21 @@ restore(Proc_t* proc) case PROC_fd_dup|PROC_FD_CHILD: case PROC_fd_dup|PROC_FD_PARENT|PROC_FD_CHILD: if (m->op & PROC_FD_PARENT) - close(m->arg.fd.parent.fd); + ast_close(m->arg.fd.parent.fd); if (m->arg.fd.child.fd != m->arg.fd.parent.fd && m->arg.fd.child.fd != PROC_ARG_NULL) { if (!(m->op & PROC_FD_PARENT)) { if (m->op & PROC_FD_CHILD) { - close(m->arg.fd.parent.fd); + ast_close(m->arg.fd.parent.fd); fcntl(m->arg.fd.child.fd, F_DUPFD, m->arg.fd.parent.fd); } fcntl(m->arg.fd.parent.fd, F_SETFD, m->arg.fd.parent.flag); } - close(m->arg.fd.child.fd); + ast_close(m->arg.fd.child.fd); fcntl(m->save, F_DUPFD, m->arg.fd.child.fd); - close(m->save); + ast_close(m->save); if (m->arg.fd.child.flag) fcntl(m->arg.fd.child.fd, F_SETFD, FD_CLOEXEC); } @@ -535,8 +535,8 @@ procopen(const char* cmd, char** argv, char** envv, int64_t* modv, int flags) { if (!(proc->pid = fork())) { - close(pop[0]); - close(pop[1]); + ast_close(pop[0]); + ast_close(pop[1]); } else { @@ -755,7 +755,7 @@ procopen(const char* cmd, char** argv, char** envv, int64_t* modv, int flags) { case PROC_fd_dup|PROC_FD_PARENT: case PROC_fd_dup|PROC_FD_PARENT|PROC_FD_CHILD: - close(PROC_ARG(n, 1)); + ast_close(PROC_ARG(n, 1)); break; case PROC_sys_pgrp: if (proc->pgrp < 0) @@ -782,19 +782,19 @@ procopen(const char* cmd, char** argv, char** envv, int64_t* modv, int flags) { case 0: proc->wfd = pio[1]; - close(pio[0]); + ast_close(pio[0]); break; default: #if _pipe_rw || _lib_socketpair proc->wfd = pio[0]; #else proc->wfd = poi[1]; - close(poi[0]); + ast_close(poi[0]); #endif /* _pipe_rw || _lib_socketpair */ /* FALLTHROUGH */ case 1: proc->rfd = pio[0]; - close(pio[1]); + ast_close(pio[1]); break; } if (proc->rfd > 2) @@ -809,7 +809,7 @@ procopen(const char* cmd, char** argv, char** envv, int64_t* modv, int flags) while (waitpid(proc->pid, &i, 0) == -1 && errno == EINTR); if (read(pop[0], &proc->pid, sizeof(proc->pid)) != sizeof(proc->pid)) goto bad; - close(pop[0]); + ast_close(pop[0]); } return proc; } @@ -831,22 +831,22 @@ procopen(const char* cmd, char** argv, char** envv, int64_t* modv, int flags) case PROC_fd_dup|PROC_FD_CHILD: case PROC_fd_dup|PROC_FD_PARENT|PROC_FD_CHILD: if (PROC_ARG(n, 2) != PROC_ARG_NULL) - close(PROC_ARG(n, 1)); + ast_close(PROC_ARG(n, 1)); break; } if (pio[0] >= 0) - close(pio[0]); + ast_close(pio[0]); if (pio[1] >= 0) - close(pio[1]); + ast_close(pio[1]); if (pop[0] >= 0) - close(pop[0]); + ast_close(pop[0]); if (pop[1] >= 0) - close(pop[1]); + ast_close(pop[1]); #if !_pipe_rw && !_lib_socketpair if (poi[0] >= 0) - close(poi[0]); + ast_close(poi[0]); if (poi[1] >= 0) - close(poi[1]); + ast_close(poi[1]); #endif /* !_pipe_rw && !_lib_socketpair */ procfree(proc); return NULL; diff --git a/src/lib/libast/misc/univlib.h b/src/lib/libast/misc/univlib.h index 5902ae60b7f3..01852914f52e 100644 --- a/src/lib/libast/misc/univlib.h +++ b/src/lib/libast/misc/univlib.h @@ -39,7 +39,6 @@ #include #include -#include #define UNIV_SIZE 9 diff --git a/src/lib/libast/path/pathicase.c b/src/lib/libast/path/pathicase.c index 27e95818789f..dba4670429f4 100644 --- a/src/lib/libast/path/pathicase.c +++ b/src/lib/libast/path/pathicase.c @@ -59,11 +59,11 @@ pathicase(const char *path) if (r < 0 && errno == ENOTTY) /* if it's not VFAT/FAT32...*/ { r = ioctl(fd, FS_IOC_GETFLAGS, &attr); - close(fd); + ast_close(fd); return r < 0 ? -1 : (attr & FS_CASEFOLD_FL) != 0; } # endif /* _linux_casefold */ - close(fd); + ast_close(fd); return r < 0 ? (errno != ENOTTY ? -1 : 0) : 1; #elif _WINIX || __APPLE__ /* Windows or Mac without pathconf probe: assume case insensitive */ diff --git a/src/lib/libast/port/astconf.c b/src/lib/libast/port/astconf.c index 89001b87f318..4231e4c6ea03 100644 --- a/src/lib/libast/port/astconf.c +++ b/src/lib/libast/port/astconf.c @@ -1102,7 +1102,7 @@ print(Sfio_t* sp, Lookup_t* look, const char* name, const char* path, int listfl if (streq(p->name, "RELEASE") && (i = open("/proc/version", O_RDONLY|O_cloexec)) >= 0) { n = read(i, buf, sizeof(buf) - 1); - close(i); + ast_close(i); if (n > 0 && buf[n - 1] == '\n') n--; if (n > 0 && buf[n - 1] == '\r') diff --git a/src/lib/libast/port/astwinsize.c b/src/lib/libast/port/astwinsize.c index f1eae238b3c0..b0c4e3bee9a2 100644 --- a/src/lib/libast/port/astwinsize.c +++ b/src/lib/libast/port/astwinsize.c @@ -124,7 +124,7 @@ ttctl(int fd, int op, void* tt) if ((fd = open("/dev/tty", O_RDONLY|O_cloexec)) >= 0) { v = ioctl(fd, op, tt); - close(fd); + ast_close(fd); return v; } } diff --git a/src/lib/libast/regex/reglib.h b/src/lib/libast/regex/reglib.h index 67388c9ac1e0..aa7ea4ace9ec 100644 --- a/src/lib/libast/regex/reglib.h +++ b/src/lib/libast/regex/reglib.h @@ -58,7 +58,6 @@ typedef struct regsubop_s #include "regex.h" #include -#include #if _BLD_DEBUG && !defined(_AST_REGEX_DEBUG) #define _AST_REGEX_DEBUG 1 diff --git a/src/lib/libast/sfio/_sfopen.c b/src/lib/libast/sfio/_sfopen.c index 2d80b24d1047..cb21778ab4e2 100644 --- a/src/lib/libast/sfio/_sfopen.c +++ b/src/lib/libast/sfio/_sfopen.c @@ -91,7 +91,7 @@ Sfio_t* _sfopen(Sfio_t* f, /* old stream structure */ errno = 0; if(fd >= 0) { if((oflags&(O_CREAT|O_EXCL)) == (O_CREAT|O_EXCL) ) - { CLOSE(fd); /* error: file already exists */ + { ast_close(fd); /* error: file already exists */ return NULL; } if(oflags&O_TRUNC ) /* truncate file */ @@ -99,7 +99,7 @@ Sfio_t* _sfopen(Sfio_t* f, /* old stream structure */ while((tf = creat(file,SFIO_CREATMODE)) < 0 && errno == EINTR) errno = 0; - CLOSE(tf); + ast_close(tf); } } else if(oflags&O_CREAT) @@ -107,7 +107,7 @@ Sfio_t* _sfopen(Sfio_t* f, /* old stream structure */ errno = 0; if((oflags&O_ACCMODE) != O_WRONLY) { /* the file now exists, reopen it for read/write */ - CLOSE(fd); + ast_close(fd); while((fd = open(file,oflags&O_ACCMODE)) < 0 && errno == EINTR) errno = 0; diff --git a/src/lib/libast/sfio/sfclose.c b/src/lib/libast/sfio/sfclose.c index 2f8370c33358..ffbcbe9d38c6 100644 --- a/src/lib/libast/sfio/sfclose.c +++ b/src/lib/libast/sfio/sfclose.c @@ -110,14 +110,10 @@ int sfclose(Sfio_t* f) if(_Sfnotify) (*_Sfnotify)(f, SFIO_CLOSING, (void*)((long)f->file)); if(f->file >= 0 && !(f->flags&SFIO_STRING)) - { while(close(f->file) < 0 ) - { if(errno == EINTR) - errno = 0; - else - { rv = -1; - break; - } - } + { errno = 0; + ast_close(f->file); + if(errno) + rv = -1; } f->file = -1; diff --git a/src/lib/libast/sfio/sfhdr.h b/src/lib/libast/sfio/sfhdr.h index e3ce56470cb7..93a8242bae47 100644 --- a/src/lib/libast/sfio/sfhdr.h +++ b/src/lib/libast/sfio/sfhdr.h @@ -466,9 +466,6 @@ typedef struct _sfextern_s ((f)->endb = (f)->endr = (f)->endw = (f)->next = \ (f)->data = NULL) ) -/* safe closing function */ -#define CLOSE(f) { while(close(f) < 0 && errno == EINTR) errno = 0; } - /* the bottomless bit bucket */ #define DEVNULL "/dev/null" #define SFSETNULL(f) ((f)->extent = (Sfoff_t)(-1), (f)->bits |= SFIO_NULL) diff --git a/src/lib/libast/sfio/sfmode.c b/src/lib/libast/sfio/sfmode.c index 677df4614a2a..e88e5fdf70d8 100644 --- a/src/lib/libast/sfio/sfmode.c +++ b/src/lib/libast/sfio/sfmode.c @@ -218,7 +218,7 @@ int _sfpclose(Sfio_t* f) else { /* close the associated stream */ if(p->file >= 0) - CLOSE(p->file); + ast_close(p->file); /* wait for process termination */ sigcritical(SIG_REG_EXEC|SIG_REG_PROC); diff --git a/src/lib/libast/sfio/sfsetfd.c b/src/lib/libast/sfio/sfsetfd.c index d48722ede665..715c292bf075 100644 --- a/src/lib/libast/sfio/sfsetfd.c +++ b/src/lib/libast/sfio/sfsetfd.c @@ -67,7 +67,7 @@ static int sfsetfd_internal(Sfio_t* f, int newfd, int cloexec) { SFOPEN(f,0); return -1; } - CLOSE(oldfd); + ast_close(oldfd); } else { /* sync stream if necessary */ diff --git a/src/lib/libast/sfio/sftmp.c b/src/lib/libast/sfio/sftmp.c index 3474fa0b1ae2..e7217e17967a 100644 --- a/src/lib/libast/sfio/sftmp.c +++ b/src/lib/libast/sfio/sftmp.c @@ -77,7 +77,7 @@ static int _tmprmfile(Sfio_t* f, int type, void* val, Sfdisc_t* disc) if(_Sfnotify) (*_Sfnotify)(f,SFIO_CLOSING,f->file); - CLOSE(f->file); + ast_close(f->file); f->file = -1; while(remove(ff->name) < 0 && errno == EINTR) errno = 0; diff --git a/src/lib/libast/tm/tvsettime.c b/src/lib/libast/tm/tvsettime.c index 2374339c0f0e..84efb5d21f34 100644 --- a/src/lib/libast/tm/tvsettime.c +++ b/src/lib/libast/tm/tvsettime.c @@ -19,7 +19,6 @@ #include #include -#include #include "FEATURE/tvlib" diff --git a/src/lib/libast/tm/tvtouch.c b/src/lib/libast/tm/tvtouch.c index aca8c4d9779f..06514d977868 100644 --- a/src/lib/libast/tm/tvtouch.c +++ b/src/lib/libast/tm/tvtouch.c @@ -136,7 +136,7 @@ tvtouch(const char* path, const Tv_t* av, const Tv_t* mv, const Tv_t* cv, int fl mode = (~mode) & (S_IRUSR|S_IWUSR|S_IRGRP|S_IWGRP|S_IROTH|S_IWOTH); if ((fd = open(path, O_WRONLY|O_CREAT|O_TRUNC|O_cloexec, mode)) < 0) return -1; - close(fd); + ast_close(fd); errno = oerrno; if ((ts[0].tv_nsec != UTIME_NOW || ts[1].tv_nsec != UTIME_NOW) && utimensat(AT_FDCWD, path, ts, (flags & TV_TOUCH_PHYSICAL) ? AT_SYMLINK_NOFOLLOW : 0)) return -1; @@ -246,11 +246,12 @@ tvtouch(const char* path, const Tv_t* av, const Tv_t* mv, const Tv_t* cv, int fl { if (c = (lseek(fd, 0L, 0) == 0L && write(fd, &c, 1) == 1)) errno = oerrno; - close(fd); + ast_close(fd); if (c) return 0; } - close(fd); + else + ast_close(fd); } } #endif @@ -260,7 +261,7 @@ tvtouch(const char* path, const Tv_t* av, const Tv_t* mv, const Tv_t* cv, int fl mode = (~mode) & (S_IRUSR|S_IWUSR|S_IRGRP|S_IWGRP|S_IROTH|S_IWOTH); if ((fd = open(path, O_WRONLY|O_CREAT|O_TRUNC|O_cloexec, mode)) < 0) return -1; - close(fd); + ast_close(fd); errno = oerrno; if (av == (const Tv_t*)&now && mv == (const Tv_t*)&now) return 0; diff --git a/src/lib/libcmd/cp.c b/src/lib/libcmd/cp.c index 37f6f842c11a..6ed188c7f040 100644 --- a/src/lib/libcmd/cp.c +++ b/src/lib/libcmd/cp.c @@ -429,7 +429,7 @@ visit(State_t* state, FTSENT* ent) if (S_ISLNK(st.st_mode) && (n = -1) || (n = open(state->path, O_RDWR|O_BINARY|O_cloexec)) >= 0) { if (n >= 0) - close(n); + ast_close(n); if (state->force) /* ok */; else if (state->interactive) @@ -580,7 +580,7 @@ visit(State_t* state, FTSENT* ent) { error(ERROR_SYSTEM|2, "%s: cannot write", state->path); if (ent->fts_statp->st_size > 0) - close(rfd); + ast_close(rfd); return 0; } else if (ent->fts_statp->st_size > 0) @@ -588,14 +588,14 @@ visit(State_t* state, FTSENT* ent) if (!(ip = sfnew(NULL, NULL, SFIO_UNBOUND, rfd, SFIO_READ))) { error(ERROR_SYSTEM|2, "%s: %s read stream error", ent->fts_path, state->path); - close(rfd); - close(wfd); + ast_close(rfd); + ast_close(wfd); return 0; } if (!(op = sfnew(NULL, NULL, SFIO_UNBOUND, wfd, SFIO_WRITE))) { error(ERROR_SYSTEM|2, "%s: %s write stream error", ent->fts_path, state->path); - close(wfd); + ast_close(wfd); sfclose(ip); return 0; } @@ -615,7 +615,7 @@ visit(State_t* state, FTSENT* ent) } } else - close(wfd); + ast_close(wfd); } else if (S_ISBLK(ent->fts_statp->st_mode) || S_ISCHR(ent->fts_statp->st_mode) || S_ISFIFO(ent->fts_statp->st_mode)) { diff --git a/src/lib/libcmd/mktemp.c b/src/lib/libcmd/mktemp.c index 95654e7be06a..48b6f05f0bb0 100644 --- a/src/lib/libcmd/mktemp.c +++ b/src/lib/libcmd/mktemp.c @@ -155,7 +155,7 @@ b_mktemp(int argc, char** argv, Shbltin_t* context) if (fdp || unsafe || !mkdir(path, mode)) { if (fdp) - close(*fdp); + ast_close(*fdp); sfputr(sfstdout, path, '\n'); break; } diff --git a/src/lib/libcmd/rm.c b/src/lib/libcmd/rm.c index 80c73851f5e6..0c623c546f29 100644 --- a/src/lib/libcmd/rm.c +++ b/src/lib/libcmd/rm.c @@ -296,7 +296,7 @@ rm(State_t* state, FTSENT* ent) c -= sizeof(state->buf); } fsync(n); - close(n); + ast_close(n); } } #endif diff --git a/src/lib/libcmd/tee.c b/src/lib/libcmd/tee.c index 3333a4301503..741d9f20938f 100644 --- a/src/lib/libcmd/tee.c +++ b/src/lib/libcmd/tee.c @@ -99,7 +99,7 @@ tee_cleanup(Tee_t* tp) if (tp->line >= 0) sfset(sfstdout, SFIO_LINE, tp->line); for (hp = tp->fd; (n = *hp) >= 0; hp++) - close(n); + ast_close(n); } } From 22033ea62c7e7db57383347790ac0d9154c4f1a6 Mon Sep 17 00:00:00 2001 From: Johnothan King Date: Fri, 11 Jul 2025 20:06:39 -0700 Subject: [PATCH 2/2] test_stat(): Use strcmp to enable pwdfd optimization for test -ef --- src/cmd/ksh93/bltins/test.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cmd/ksh93/bltins/test.c b/src/cmd/ksh93/bltins/test.c index 41a2b50b4a43..00647a3a00a9 100644 --- a/src/cmd/ksh93/bltins/test.c +++ b/src/cmd/ksh93/bltins/test.c @@ -716,7 +716,7 @@ static int test_stat(const char *name,struct stat *buff) return -1; } #if _lib_openat - if(name==e_dot && sh.pwdfd > -1) + if(sh.pwdfd > -1 && strcmp(name,e_dot)==0) return fstat(sh.pwdfd,buff); #endif if(sh_isdevfd(name))