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

Fix crash while handling subshell trap (rhbz#1117404)

Browse source 
Contrary to the RH bug report, this is yet another bug with
virtual/non-forked subshells and has nothing to do with functions.
If a signal is ignored (empty trap) in the main shell while any
trap (empty or not) is set on the same signal in a subshell, a
crash eventually occurred upon restoring state when leaving the
subshell.

Original patch:
https://src.fedoraproject.org/rpms/ksh/blob/642af4d6/f/ksh-20120801-trapcom.patch

Prior discussion:
https://bugzilla.redhat.com/1117404

Paulo Andrade wrote there:
> The problem is that the sh_subshell function was saving pointers
> that could change, and when restoring, bad things would happen.
[...]
> The only comment I added:
> /* contents of shp->st.trapcom may change */
> may be a bit misleading, the "bad" save/restore already knows it,
> probably I should have added a better description telling that the
> data is, usually, modified in code like:
>
> tmp = buf[i]; buf[i] = strdup(tmp); free(tmp);
>
> so the shp->st.trapcom needs a "deep copy", as done in the
> patch, to properly save/restore pointers.

src/cmd/ksh93/sh/subshell.c,
src/cmd/ksh93/sh/xec.c:
- sh_subshell(), sh_funscope(): Make *savsig/*savstak into a
  **savsig array. Use strdup(3) to save the data and get known
  pointers that will not change. Free these upon restore.
- Change the comment from the patch as Paulo wished he had done.

src/cmd/ksh93/tests/subshell.sh:
- Test 2500 times. This should trigger the crash most of the time.
This commit is contained in:
Martijn Dekker 2020-09-27 05:58:54 +02:00
parent 045fe6a110
commit 6193c6a3c5
3 changed files with 63 additions and 14 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

29
src/cmd/ksh93/sh/subshell.c
View file

@ -470,11 +470,11 @@ Sfio_t *sh_subshell(Shell_t *shp,Shnode_t *t, volatile int flags, int comsub)
{
struct subshell sub_data;
register struct subshell *sp = &sub_data;
int jmpval,nsig=0,duped=0;
int jmpval,isig,nsig=0,duped=0;
unsigned int savecurenv = shp->curenv;
int savejobpgid = job.curpgid;
int *saveexitval = job.exitval;
char *savsig;
char **savsig;
Sfio_t *iop=0;
struct checkpt buff;
struct sh_scoped savst;
@ -569,10 +569,24 @@ Sfio_t *sh_subshell(Shell_t *shp,Shnode_t *t, volatile int flags, int comsub)
/* save trap table */
shp->st.otrapcom = 0;
shp->st.otrap = savst.trap;
if((nsig=shp->st.trapmax*sizeof(char*))>0 || shp->st.trapcom[0])
if((nsig=shp->st.trapmax)>0 || shp->st.trapcom[0])
{
nsig += sizeof(char*);
memcpy(savsig=malloc(nsig),(char*)&shp->st.trapcom[0],nsig);
++nsig;
savsig = malloc(nsig * sizeof(char*));
/*
* the data is, usually, modified in code like:
* tmp = buf[i]; buf[i] = strdup(tmp); free(tmp);
* so shp->st.trapcom needs a "deep copy" to properly save/restore pointers.
*/
for (isig = 0; isig < nsig; ++isig)
{
if(shp->st.trapcom[isig] == Empty)
savsig[isig] = Empty;
else if(shp->st.trapcom[isig])
savsig[isig] = strdup(shp->st.trapcom[isig]);
else
savsig[isig] = NULL;
}
/* this nonsense needed for $(trap) */
shp->st.otrapcom = (char**)savsig;
}
@ -732,7 +746,10 @@ Sfio_t *sh_subshell(Shell_t *shp,Shnode_t *t, volatile int flags, int comsub)
shp->st.otrap = 0;
if(nsig)
{
memcpy((char*)&shp->st.trapcom[0],savsig,nsig);
for (isig = 0; isig < nsig; ++isig)
if (shp->st.trapcom[isig] && shp->st.trapcom[isig]!=Empty)
free(shp->st.trapcom[isig]);
memcpy((char*)&shp->st.trapcom[0],savsig,nsig*sizeof(char*));
free((void*)savsig);
}
shp->options = sp->options;

34
src/cmd/ksh93/sh/xec.c
View file

@ -3042,10 +3042,10 @@ int sh_funscope(int argn, char *argv[],int(*fun)(void*),void *arg,int execflg)
struct dolnod *argsav=0,*saveargfor;
struct sh_scoped savst, *prevscope = shp->st.self;
struct argnod *envlist=0;
int jmpval;
int isig,jmpval;
volatile int r = 0;
int n;
char *savstak;
char **savsig;
struct funenv *fp = 0;
struct checkpt *buffp = (struct checkpt*)stkalloc(shp->stk,sizeof(struct checkpt));
Namval_t *nspace = shp->namespace;
@ -3091,10 +3091,24 @@ int sh_funscope(int argn, char *argv[],int(*fun)(void*),void *arg,int execflg)
}
shp->st.cmdname = argv[0];
/* save trap table */
if((nsig=shp->st.trapmax*sizeof(char*))>0 || shp->st.trapcom[0])
if((nsig=shp->st.trapmax)>0 || shp->st.trapcom[0])
{
nsig += sizeof(char*);
memcpy(savstak=stakalloc(nsig),(char*)&shp->st.trapcom[0],nsig);
++nsig;
savsig = malloc(nsig * sizeof(char*));
/*
* the data is, usually, modified in code like:
* tmp = buf[i]; buf[i] = strdup(tmp); free(tmp);
* so shp->st.trapcom needs a "deep copy" to properly save/restore pointers.
*/
for (isig = 0; isig < nsig; ++isig)
{
if(shp->st.trapcom[isig] == Empty)
savsig[isig] = Empty;
else if(shp->st.trapcom[isig])
savsig[isig] = strdup(shp->st.trapcom[isig]);
else
savsig[isig] = NULL;
}
}
sh_sigreset(0);
argsav = sh_argnew(shp,argv,&saveargfor);
@ -3158,10 +3172,14 @@ int sh_funscope(int argn, char *argv[],int(*fun)(void*),void *arg,int execflg)
shp->topscope = (Shscope_t*)prevscope;
nv_getval(sh_scoped(shp,IFSNOD));
if(nsig)
memcpy((char*)&shp->st.trapcom[0],savstak,nsig);
{
for (isig = 0; isig < nsig; ++isig)
if (shp->st.trapcom[isig] && shp->st.trapcom[isig]!=Empty)
free(shp->st.trapcom[isig]);
memcpy((char*)&shp->st.trapcom[0],savsig,nsig*sizeof(char*));
free((void*)savsig);
}
shp->trapnote=0;
if(nsig)
stakset(savstak,0);
shp->options = options;
shp->last_root = last_root;
if(jmpval == SH_JMPSUB)

14
src/cmd/ksh93/tests/subshell.sh
View file

@ -847,5 +847,19 @@ actual=${ get_value; }
actual=`get_value`
[[ $actual == "$expect" ]] || err_exit "\`Comsub\` failed to return output (expected '$expect', got '$actual')"
# ======
# https://bugzilla.redhat.com/1117404
cat >$tmp/crash_rhbz1117404.ksh <<-'EOF'
trap "" HUP # trigger part 1: signal ignored in main shell
i=0
for((i=0; i<2500; i++))
do (trap ": foo" HUP) # trigger part 2: any trap (empty or not) on same signal in subshell
done
EOF
got=$( { "$SHELL" "$tmp/crash_rhbz1117404.ksh"; } 2>&1)
((!(e = $?))) || err_exit 'crash while handling subshell trap' \
"(got status $e$( ((e>128)) && print -n / && kill -l "$e"), $(printf %q "$got"))"
# ======
exit $((Errors<125?Errors:125))