Skip to content

Commit

Permalink
Fix off-by-one error, possible crash (re: 6193c6a)
Browse files Browse the repository at this point in the history
The ksh-20120801-trapcom.patch patch contains an off-by-one error,
which was also imported into 93u+m. When saving signals:

https://github.com/ksh93/ksh/blob/ceb77b136fb1f2cb0adf1e197c1b60d5b7c91d96/src/cmd/ksh93/sh/subshell.c#L572-L592
572	if((nsig=shp->st.trapmax)>0 || shp->st.trapcom[0])
573	{
574		++nsig;
575		savsig = malloc(nsig * sizeof(char*));
576		/*
577		 * the data is, usually, modified in code like:
578		 *	tmp = buf[i]; buf[i] = strdup(tmp); free(tmp);
579		 * so shp->st.trapcom needs a "deep copy" to properly save/restore pointers.
580		 */
581		for (isig = 0; isig < nsig; ++isig)
582		{
583			if(shp->st.trapcom[isig] == Empty)
584				savsig[isig] = Empty;
585			else if(shp->st.trapcom[isig])
586				savsig[isig] = strdup(shp->st.trapcom[isig]);
587			else
588				savsig[isig] = NULL;
589		}

On line 574, the number of signals 'nsig' is increased by one. That
increase is permanent, so the 'for' loop on line 581 tries to save
one signal state too many.

The increase was a holdout from the ksh93 code from before the
patch. After the patch, it is not required; it is fine to malloc as
many records as there are trapcom elements to save. So it should
simply be removed. xec.c has the same code to save trap states for
ksh functions, and the same applies.

src/cmd/ksh93/sh/subshell.c: sh_subshell():
- Don't increase nsig.

src/cmd/ksh93/sh/xec.c: sh_funscope():
- Same.

src/cmd/ksh93/tests/signal.sh:
- Add test.
  • Loading branch information
McDutchie committed Sep 28, 2020
1 parent f527706 commit 3aee10d
Show file tree
Hide file tree
Showing 3 changed files with 14 additions and 2 deletions.
1 change: 0 additions & 1 deletion src/cmd/ksh93/sh/subshell.c
Expand Up @@ -571,7 +571,6 @@ Sfio_t *sh_subshell(Shell_t *shp,Shnode_t *t, volatile int flags, int comsub)
shp->st.otrap = savst.trap;
if((nsig=shp->st.trapmax)>0 || shp->st.trapcom[0])
{
++nsig;
savsig = malloc(nsig * sizeof(char*));
/*
* the data is, usually, modified in code like:
Expand Down
1 change: 0 additions & 1 deletion src/cmd/ksh93/sh/xec.c
Expand Up @@ -3096,7 +3096,6 @@ int sh_funscope(int argn, char *argv[],int(*fun)(void*),void *arg,int execflg)
/* save trap table */
if((nsig=shp->st.trapmax)>0 || shp->st.trapcom[0])
{
++nsig;
savsig = malloc(nsig * sizeof(char*));
/*
* the data is, usually, modified in code like:
Expand Down
14 changes: 14 additions & 0 deletions src/cmd/ksh93/tests/signal.sh
Expand Up @@ -480,5 +480,19 @@ then
kill -INFO $$ || err_exit '`kill` cannot send SIGINFO to processes when passed `-INFO`'
fi
# ======
# Due to an off-by-one error, the last signal in 'kill -l' output wasn't treated properly and could crash.
sig=$(kill -l | tail -n 1)
exp="OK: $sig"
got=$(export sig; "$SHELL" -c '
trap "print '\''OK: $sig'\''" "$sig"
(kill -s "$sig" "$$")
trap - "$sig"
' 2>&1)
((!(e = $?))) && [[ $got == "$exp" ]] || err_exit "failed to handle SIG$sig from subshell" \
"(got status $e$( ((e>128)) && print -n / && kill -l "$e"), $(printf %q "$got"))"
# ======
exit $((Errors<125?Errors:125))

0 comments on commit 3aee10d

Please sign in to comment.