From 7a06d911e0939e1027a67a3a96627935513f64f5 Mon Sep 17 00:00:00 2001 From: Martijn Dekker Date: Mon, 20 Jun 2022 16:09:35 +0100 Subject: [PATCH] do not sh_close() a -1 file descriptor (re: 80767b1f, 411481eb) Researching #483 revealed several instances in coprocess cleanup where sh_close() is called with a file descriptor of -1 (which flags that the pipe is already closed). As of feeb62d1, this sets errno to EBADF. While the race condition triggered by this was fixed in 411481eb, it's still better to avoid it. This re-applies most of the changes reverted in 80767b1f. --- src/cmd/ksh93/sh/jobs.c | 9 +++++---- src/cmd/ksh93/sh/path.c | 7 +++++-- src/cmd/ksh93/sh/subshell.c | 6 ++++-- 3 files changed, 14 insertions(+), 8 deletions(-) diff --git a/src/cmd/ksh93/sh/jobs.c b/src/cmd/ksh93/sh/jobs.c index a8f0d03bf..aed1fc626 100644 --- a/src/cmd/ksh93/sh/jobs.c +++ b/src/cmd/ksh93/sh/jobs.c @@ -395,10 +395,11 @@ int job_reap(register int sig) /* check for coprocess completion */ if(pid==sh.cpid) { - sh_close(sh.coutpipe); - sh_close(sh.cpipe[1]); - sh.cpipe[1] = -1; - sh.coutpipe = -1; + if(sh.coutpipe > -1) + sh_close(sh.coutpipe); + if(sh.cpipe[1] > -1) + sh_close(sh.cpipe[1]); + sh.coutpipe = sh.cpipe[1] = -1; } else if(sh.subshell) sh_subjobcheck(pid); diff --git a/src/cmd/ksh93/sh/path.c b/src/cmd/ksh93/sh/path.c index e3bda4046..363a1debf 100644 --- a/src/cmd/ksh93/sh/path.c +++ b/src/cmd/ksh93/sh/path.c @@ -1275,10 +1275,13 @@ static noreturn void exscript(register char *path,register char *argv[],char **e sh.bckpid = 0; sh.st.ioset=0; /* clean up any cooperating processes */ - if(sh.cpipe[0]>0) + if(sh.cpipe[0] > -1) sh_pclose(sh.cpipe); - if(sh.cpid && sh.outpipe) + if(sh.cpid && sh.outpipe && *sh.outpipe > -1) + { sh_close(*sh.outpipe); + *sh.outpipe = -1; + } sh.cpid = 0; if(sp=fcfile()) while(sfstack(sp,SF_POPSTACK)); diff --git a/src/cmd/ksh93/sh/subshell.c b/src/cmd/ksh93/sh/subshell.c index 05dbdf2cd..92b2a78a7 100644 --- a/src/cmd/ksh93/sh/subshell.c +++ b/src/cmd/ksh93/sh/subshell.c @@ -455,8 +455,10 @@ void sh_subjobcheck(pid_t pid) { if(sp->cpid==pid) { - sh_close(sp->coutpipe); - sh_close(sp->cpipe); + if(sp->coutpipe > -1) + sh_close(sp->coutpipe); + if(sp->cpipe > -1) + sh_close(sp->cpipe); sp->coutpipe = sp->cpipe = -1; return; }