external/cde
1
0
Fork 0
mirror of git://git.code.sf.net/p/cdesktopenv/code synced 2025-02-13 03:32:24 +00:00
Code Issues Releases Wiki Activity

Fix "/dev/null: cannot create [Bad file descriptor]" (re: feeb62d1)

Browse source 
Reproducer for a race condition:

  typeset -i i
  cat=${ command -pv cat; }
  for((i=0;i<20000;i++))
  do    printf '\r%d ' "$i"
        ksh |&
        cop=$!
        print -p $'print hello | '$cat$'\nprint '$exp
        read -t 1 -p
        read -t 1 -p
        redirect 5<&p 6>&p 5<&- 6>&-
        { sleep .4; kill $cop; } &
        spy=$!
        if    wait $cop 2>/dev/null # "/dev/null: cannot create"?
        then  kill $spy
        else  kill $spy; exit       # boom
        fi
        wait
  done

Result on my system. The time it takes to fail varies a lot:

  $ arch/*/bin/ksh foo
  717 foo[13]: /dev/null: cannot create [Bad file descriptor]
  $

Analysis:

errno may get a value during SIGCHLD processing. This may cause the
functions called by the signal interrupt to call sh_close() with a
negative file descriptor, giving errno the EBADF value as this loop
in sh_open() is interrupted:

ksh/src/cmd/ksh93/sh/io.c in 94fc983
851:	 while((fd = open(path, flags, mode)) < 0)
852:	 	if(errno!=EINTR || sh.trapnote)
853: 			return(-1);

Commit feeb62d1 caused sh_close() to set errno to EBADF if it gets
called with a negative file descriptor. In the cleanup of
coprocesses during the SIGCHLD interrupt handling, there are
several locations where sh_close() may be called with a negative
file descriptor, which now sets errno to EBADF. If SIGCHLD is
handled between open() being interrupted and the check for
errno!=EINTR, sh_open() will return -1 instead of retrying, causing
sh_redirect() to issue the "cannot create" error message.

SIGCHLD is issued whenever a process (such as the coprocess in the
reproducer) terminates. job_reap() is called (via job_waitsafe())
to handle this. That function does not always restore errno, which
allowed this race condition to happen.

src/cmd/ksh93/sh/jobs.c: job_reap():
- Always save/restore errno.

Resolves: https://github.com/ksh93/ksh/issues/483
This commit is contained in:
Martijn Dekker 2022-06-20 14:46:22 +01:00
parent 80767b1fa9
commit 411481eb04
1 changed files with 9 additions and 3 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

12
src/cmd/ksh93/sh/jobs.c
View file

@ -291,7 +291,7 @@ int job_reap(register int sig)
struct process *px;
register int flags;
struct jobsave *jp;
int nochild=0, oerrno, wstat;
int nochild = 0, oerrno = errno, wstat;
Waitevent_f waitevent = sh.waitevent;
static int wcontinued = WCONTINUED;
int was_ttywait_on;
@ -312,7 +312,6 @@ int job_reap(register int sig)
else
flags = WUNTRACED|wcontinued;
sh.waitevent = 0;
oerrno = errno;
was_ttywait_on = sh_isstate(SH_TTYWAIT); /* save tty wait state */
while(1)
{
@ -478,7 +477,6 @@ int job_reap(register int sig)
}
if(errno==ECHILD)
{
errno = oerrno;
#if SHOPT_BGX
job.numbjob = 0;
#endif /* SHOPT_BGX */
@ -494,6 +492,14 @@ int job_reap(register int sig)
}
if(sig)
signal(sig, job_waitsafe);
/*
* Always restore errno, because this code is run during signal handling which may interrupt loops like:
* while((fd = open(path, flags, mode)) < 0)
* if(errno!=EINTR)
* <throw error>;
* otherwise that may fail if SIGCHLD is handled between the open() call and the errno!=EINTR check.
*/
errno = oerrno;
return(nochild);
}