Skip to content

Commit

Permalink
Fix crash on 'exec' after 'unset SHLVL' (re: e5e1d4b)
Browse files Browse the repository at this point in the history
Vincent Mihalkovič (@vmihalko) reports:
> If the user unsets the SHLVL variable and later replaces the
> shell by executing a command, ksh will segfault.
>
> Reproducer
>
>     # unset -v SHLVL
>     # exec bash
>     Segmaentation fault (core dumped)
>
> Reason
>
> The reason for this is that the SHLVL variable is getting
> decremented without any guard making sure it's still in the
> environment, see:
>
> src/cmd/ksh93/bltins/misc.c:
> 145: /* if the main shell is about to be replaced, decrease SHLVL
> 	to cancel out a subsequent increase */
> 146: if(!sh.realsubshell)
> 147:         (*SHLVL->nvalue.ip)--;

This should be fixed so that SHLVL can be unset safely and lose its
special properties like other special variables do.

src/cmd/ksh93/sh/init.c,
src/cmd/ksh93/include/shell.h:
- Move the static 'shlvl' in init.c to the sh state struct, so that
  'sh.shlvl' can be accessed from the entire ksh93 code base.
- Change its type from int to int32_t -- this is more correct as
  nv_getval() uses this type to retrieve the value. In practice,
  this makes no difference because sizeof(int) == sizeof(int32_t),
  but this is not guaranteed by the standards.

src/cmd/ksh93/bltins/misc.c:
- Access sh.shlvl directly instead of dereferencing the SHLVL value
  pointer, as this pointer is invalidated upon 'unset SHLVL'. This
  fixes the bug.

Resolves: #788
  • Loading branch information
McDutchie committed Oct 27, 2024
1 parent 4393998 commit 8d08f66
Show file tree
Hide file tree
Showing 6 changed files with 19 additions and 6 deletions.
6 changes: 6 additions & 0 deletions NEWS
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,12 @@ This documents significant changes in the 1.0 branch of ksh 93u+m.
For full details, see the git log at: https://github.com/ksh93/ksh/tree/1.0
Uppercase BUG_* IDs are shell bug IDs as used by the Modernish shell library.

2024-10-27:

- Fixed a bug that crashed the shell upon attempting to replace the shell
with another command using 'exec' after unsetting the SHLVL variable.
Bug introduced on 2021-05-18.

2024-10-23:

- Fixed a bug in multidimensional indexed array assignments where unquoted
Expand Down
2 changes: 1 addition & 1 deletion src/cmd/ksh93/bltins/misc.c
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ int b_exec(int argc,char *argv[], Shbltin_t *context)
return 1;
/* if the main shell is about to be replaced, decrease SHLVL to cancel out a subsequent increase */
if(!sh.realsubshell)
(*SHLVL->nvalue.ip)--;
sh.shlvl--;
sh_onstate(SH_EXEC);
if(sh.subshell && !sh.subshare)
{
Expand Down
1 change: 1 addition & 0 deletions src/cmd/ksh93/include/shell.h
Original file line number Diff line number Diff line change
Expand Up @@ -271,6 +271,7 @@ struct Shell_s
* Programs using libshell should not rely on them as they may change. */
int subshell; /* set for virtual subshell */
int realsubshell; /* ${.sh.subshell}, actual subshell level (including virtual and forked) */
int32_t shlvl; /* $SHLVL, non-subshell child shell level */
char shcomp; /* set when running shcomp */
unsigned char trapnote; /* set when trap/signal is pending */
struct sh_scoped st; /* scoped information */
Expand Down
2 changes: 1 addition & 1 deletion src/cmd/ksh93/include/version.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@

#define SH_RELEASE_FORK "93u+m" /* only change if you develop a new ksh93 fork */
#define SH_RELEASE_SVER "1.0.11-beta" /* semantic version number: https://semver.org */
#define SH_RELEASE_DATE "2024-10-23" /* must be in this format for $((.sh.version)) */
#define SH_RELEASE_DATE "2024-10-27" /* must be in this format for $((.sh.version)) */
#define SH_RELEASE_CPYR "(c) 2020-2024 Contributors to ksh " SH_RELEASE_FORK

/* Scripts sometimes field-split ${.sh.version}, so don't change amount of whitespace. */
Expand Down
7 changes: 3 additions & 4 deletions src/cmd/ksh93/sh/init.c
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,6 @@ static Init_t *nv_init(void);
#if SHOPT_STATS
static void stat_init(void);
#endif
static int shlvl;
static int rand_shift;

/*
Expand Down Expand Up @@ -1352,7 +1351,7 @@ Shell_t *sh_init(int argc,char *argv[], Shinit_f userinit)
nv_putval(ENVNOD,sfstruse(sh.strbuf),NV_RDONLY);
}
/* increase SHLVL */
shlvl++;
sh.shlvl++;
#if SHOPT_SPAWN
{
/*
Expand Down Expand Up @@ -1672,7 +1671,7 @@ void sh_reinit(void)
else
env_import_attributes(env_init());
/* Increase SHLVL */
shlvl++;
sh.shlvl++;
/* call user init function, if any */
if(sh.userinit)
(*sh.userinit)(&sh, 1);
Expand Down Expand Up @@ -1804,7 +1803,7 @@ static Init_t *nv_init(void)
sh.nvfun.last = (char*)&sh;
sh.nvfun.nofree = 1;
sh.var_base = sh.var_tree = sh_inittree(shtab_variables);
SHLVL->nvalue.ip = &shlvl;
SHLVL->nvalue.lp = &sh.shlvl;
ip->IFS_init.hdr.disc = &IFS_disc;
ip->PATH_init.disc = &RESTRICTED_disc;
ip->PATH_init.nofree = 1;
Expand Down
7 changes: 7 additions & 0 deletions src/cmd/ksh93/tests/variables.sh
Original file line number Diff line number Diff line change
Expand Up @@ -1632,5 +1632,12 @@ trap 'echo $((foo))' EXIT # throw the echo $((foo)) 'unset parameter' error tw
echo $((foo))
EOF
# ======
# exec after unset SHLVL
# https://github.com/ksh93/ksh/issues/788
{ "$SHELL" -c 'unset SHLVL; exec true'; } 2>/dev/null
(((e=$?)==0)) || err_exit "crash after unsetting SHLVL" \
"(expected status 0, got status $e$( ((e>128)) && print -n /SIG && kill -l "$e"))"
# ======
exit $((Errors<125?Errors:125))

0 comments on commit 8d08f66

Please sign in to comment.