Skip to content

Commit

Permalink
Fix a crash in the shbench fibonacci benchmark
Browse files Browse the repository at this point in the history
If the fibslow function in the shbench[*] fibonacci benchmark
is run with an argument greater than 20, ksh will crash with
a memory fault. This was caused by subshell numbers only being
16-bits long, which isn't enough when running a large number
of subshells. Extending the subshell number length to 64-bit
fixes this issue, but with vmalloc only to a certain extent.
The fibonacci benchmark no longer crashes when run, but the
regression test this commit adds only works with standard malloc.
With ksh93u+ vmalloc the regression test will crash.

src/cmd/ksh93/include/defs.h,
src/cmd/ksh93/include/jobs.h,
src/cmd/ksh93/sh/jobs.c,
src/cmd/ksh93/sh/subshell.c,
- Extend curenv, jobenv, subenv and p_env from short to long.
  This bugfix was backported from ksh93v- with a patch from
  OpenSUSE: https://build.opensuse.org/package/view_file/shells/ksh/ksh93-longenv.dif?expand=1

src/cmd/ksh93/tests/subshell.sh:
- Add a regression test for running a large number of
  subshells, but disable it if vmstate is a builtin (since
  it doesn't work with vmalloc).

[*]: https://github.com/ksh-community/shbench
  • Loading branch information
JohnoKing committed Aug 11, 2020
1 parent f485fe0 commit 34fbf89
Show file tree
Hide file tree
Showing 6 changed files with 32 additions and 7 deletions.
4 changes: 4 additions & 0 deletions NEWS
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,10 @@ Any uppercase BUG_* names are modernish shell bug IDs.
- Fixed a crash that occurred intermittently after running an external
command from a command substitution expanded from the $PS1 shell prompt.

- Partially fixed a crash that could occur after running a large number of
subshells in a script. Due to a bug in vmalloc this only applies when ksh
is compiled with -D_std_malloc.

2020-08-09:

- File name generation (a.k.a. pathname expansion, a.k.a. globbing) now
Expand Down
4 changes: 2 additions & 2 deletions src/cmd/ksh93/include/defs.h
Original file line number Diff line number Diff line change
Expand Up @@ -167,8 +167,8 @@ struct shared
Namval_t *prev_table; /* previous table used in nv_open */ \
Sfio_t *outpool; /* output stream pool */ \
long timeout; /* read timeout */ \
short curenv; /* current subshell number */ \
short jobenv; /* subshell number for jobs */ \
long curenv; /* current subshell number */ \
long jobenv; /* subshell number for jobs */ \
int infd; /* input file descriptor */ \
short nextprompt; /* next prompt is PS<nextprompt> */ \
short poolfiles; \
Expand Down
2 changes: 1 addition & 1 deletion src/cmd/ksh93/include/jobs.h
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ struct process
unsigned short p_exit; /* exit value or signal number */
unsigned short p_exitmin; /* minimum exit value for xargs */
unsigned short p_flag; /* flags - see below */
int p_env; /* subshell environment number */
long p_env; /* subshell environment number */
#ifdef JOBS
off_t p_name; /* history file offset for command */
struct termios p_stty; /* terminal state for job */
Expand Down
2 changes: 1 addition & 1 deletion src/cmd/ksh93/sh/jobs.c
Original file line number Diff line number Diff line change
Expand Up @@ -1647,7 +1647,7 @@ static struct process *job_unpost(register struct process *pwtop,int notify)
register struct process *pw;
/* make sure all processes are done */
#ifdef DEBUG
sfprintf(sfstderr,"ksh: job line %4d: drop pid=%d critical=%d pid=%d env=%d\n",__LINE__,getpid(),job.in_critical,pwtop->p_pid,pwtop->p_env);
sfprintf(sfstderr,"ksh: job line %4d: drop pid=%d critical=%d pid=%d env=%ld\n",__LINE__,getpid(),job.in_critical,pwtop->p_pid,pwtop->p_env);
sfsync(sfstderr);
#endif /* DEBUG */
pwtop = pw = job_byjid((int)pwtop->p_job);
Expand Down
6 changes: 3 additions & 3 deletions src/cmd/ksh93/sh/subshell.c
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ static struct subshell
char pwdclose;
} *subshell_data;

static int subenv;
static long subenv;


/*
Expand Down Expand Up @@ -176,7 +176,7 @@ void sh_subfork(void)
{
register struct subshell *sp = subshell_data;
Shell_t *shp = sp->shp;
int curenv = shp->curenv;
long curenv = shp->curenv;
pid_t pid;
char *trap = shp->st.trapcom[0];
if(trap)
Expand Down Expand Up @@ -456,7 +456,7 @@ 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 savecurenv = shp->curenv;
long savecurenv = shp->curenv;
int savejobpgid = job.curpgid;
int *saveexitval = job.exitval;
char *savsig;
Expand Down
21 changes: 21 additions & 0 deletions src/cmd/ksh93/tests/subshell.sh
Original file line number Diff line number Diff line change
Expand Up @@ -758,5 +758,26 @@ SHELL=$SHELL "$SHELL" -c '
' | awk '/^DEBUG/ { pid[NR] = $2; } END { exit !(pid[1] == pid[2] && pid[2] == pid[3]); }' \
|| err_exit "setting PATH to readonly in subshell triggers an erroneous fork"

# ======
# Ksh shouldn't crash after running a large number of subshells.
# NOTE: This test will always crash with vmalloc, so for now
# it's disabled if vmstate is present.
if ! builtin vmstate 2> /dev/null; then
subshell_crash="$tmp/subshell_crash.sh"
cat >| "$subshell_crash" << EOF
(
for ((n=0; n != 50000; n++)) do
(
function foo {
(true)
}
foo
)
done
)
EOF
"$SHELL" "$subshell_crash" || err_exit 'ksh crashes after running a large number of subshells'
fi

# ======
exit $((Errors<125?Errors:125))

0 comments on commit 34fbf89

Please sign in to comment.