1
0
Fork 0
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:
Martijn Dekker 2020-09-12 19:52:47 +02:00
parent 10cca4767b
commit ab5dedded7
2 changed files with 15 additions and 2 deletions

View file

@ -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"

View file

@ -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);