mirror of
git://git.code.sf.net/p/cdesktopenv/code
synced 2025-03-09 15:50:02 +00:00
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.)
This commit is contained in:
parent
10cca4767b
commit
ab5dedded7
2 changed files with 15 additions and 2 deletions
|
@ -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"
|
||||
|
|
|
@ -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);
|
||||
|
|
Loading…
Add table
Add a link
Reference in a new issue