From db227904a52c2a8c51d464e8d20e6c34675ab1c6 Mon Sep 17 00:00:00 2001 From: Martijn Dekker Date: Wed, 28 Feb 2024 01:51:47 +0000 Subject: [PATCH] Further robustify sh_reinit() (re: 0af81992, 1e827c92, 2b1deebd) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When ksh executes a script without a #! path (note that the AT&T team had a real disliking for #! paths), ksh forks and goes through a quick reinitialisation procedure. This is much faster than invoking a fully new shell but should have the same effect if it all works well. Unfortunately it's not worked all that well so far. Even after recent improvements (see referenced commits) I've been finding corner case problems. FYI, running a script without #! basically goes like this: * in path_spawn(), execve() fails with ENOEXEC because the file is not a binary executable and does not start with #! * this triggers 'case ENOEXEC:' which: * forks ksh * calls exscript() * exscript() cleans up & calls siglongjmp(*sh.jmplist,SH_JMPSCRIPT) * SH_JMPSCRIPT is the highest longjmp value, so *all* the previous sigsetjmp/sh_pushcontext calls are unwinded in reverse order, triggering all sorts of cleanup, state restoration, removal of local scopes, etc. * eventually, this lands us at the top sigsetjmp in sh_main() * sh_main() calls sh_reinit(), then resumes as if the shell had just been started This commit makes the following interrelated changes for the correct functioning of this procedure: 1. exscript() now exports the environment into a dedicated Stk_t buffer and sets environ[] to that. 2. Instead of re-using existing variables, sh_reinit() deletes everything and reinits all name-value trees from scratch, then re-imports the environment from environ[]. 3. Variable values that were imported from the environment are no longer treated specially with an NV_IMPORT attribute and the np->nvenv pointer to their value in environ[], fixing at least one crash.[*1] Details of the changes follow: src/cmd/ksh93/sh/path.c: - exscript(): Generate a new environ[] by activating a dedicated AST stack that will not be overwritten before calling sh_envgen(). This will allow sh_reinit() to delete all variables and then reimport the environment. The exporting must be done here, before siglongjmp, otherwise locally scoped exported variables won't be included (siglongjmp with SH_JMPSCRIPT triggers cleanup of all scopes). src/cmd/ksh93/sh/init.c: - sh_reinit(): Largely rewritten as follows. - Reset shell options first. This has the beneficial side effect of unsetting SH_RESTRICTED which interferes with unsetting certain variables, like PATH. - Remove workarounds for FPATH, SHLVL and tilde expansion disciplines; these will not be needed now. - Properly unset and delete all functions and built-ins. Since we now unset a function before deleting it, this should now free up their memory. (See nvdisc.c below for a change allowing removal of special built-ins.) - Properly unset all variables (which includes any associated discipline functions). Incorporate here the needed logic from sh_envnolocal() in name.c; most of it is unneeded (that function was previously used to cleanup local variables but has not been used for that for decades). So sh_envnolocal() is now unused. - Delete variables in a separate pass after unsetting variables and unsetting and deleting functions; this avoids use-after- free problems as well as possible "no parent" problems with namespace variables (e.g., .rc.status in our new kshrc.sh). - After all that, close and free up all function, alias, tracked alias, type and variable trees. - Free the contiguous built-in node space and the Init_t init context (with all the special variable discipline pointers). - Call nv_init (previously only called from sh_init) to reinitialise all of the above name-value stuff from scratch. It's the only way to be sure. - Re-import the environment as stored by exscript() above. - env_init(): - Per item 3 above and footnote 1 below, no longer set NV_IMPORT attribute and no longer point np->nvenv to the item in environ. - POSIX says, for 'environ': "Any application that directly modifies the pointers to which the environ variable points has undefined behavior."[*2] Yet, env_init() is indeed juggling the environ[] pointers to deal with variables that cannot be imported because their names are invalid (because they still need to be saved to be passed on to child processes). Replace the current approach with one where those env vars get allocated on the heap, pointed to by sh.save_env and counted by sh.save_env_n (renamed from sh.nenv). This only needs to be done once as ksh cannot use or change these variables. src/cmd/ksh93/sh/name.c: - sh_envgen(): Update to match env_init() change above. - pushnam() (called by sh_envgen()): Remove NV_IMPORT attribute check as per above and never get the value from the nvenv pointer -- simply always use nv_getval(). As of this change, the NV_IMPORT attribute is unused. The next commit will remove it and do related cleanups. - staknam(): is only called if value!=NULL, so remove that 'if'. - sh_envnolocal(): Removed. src/cmd/ksh93/sh/nvdisc.c: - assign(): Remove a check for the SH_INIT state bit that avoids freeing functions during sh_reinit(). This works fine now. - sh_addbuiltin(): Allow sh_reinit() to delete special builtins by checking for the SH_INIT state bit before throwing an error. src/cmd/ksh93/sh/nvtree.c: - outval(): Add a workaround for a use-after-free, introduced by the changes above, that occurred in the types.sh tests for #!-less scripts (types.sh:675-722). The use-after-free occurred here (abridged ASan trace follows; line numbers are current as of this commit): ==30849==ERROR: AddressSanitizer: heap-use-after-free [...] #0 in dttree dttree.c:393 #1 in sh_reinit init.c:1637 #2 in sh_main main.c:136 [...] The pointer was freed in the same loop via nv_delete() in outval: #0 in wrap_free+0x98 (libclang_rt.asan_osx_dynamic.dylib:[...]) #1 in nv_delete name.c:1318 #2 in outval nvtree.c:731 #3 in genvalue nvtree.c:905 #4 in walk_tree nvtree.c:1042 #5 in put_tree nvtree.c:1108 #6 in nv_putv nvdisc.c:144 #7 in _nv_unset name.c:2437 #8 in sh_reinit init.c:1645 #9 in sh_main main.c:136 [...] So, what happened was that the nv_delete() call on name.c:1318 (eventually resulting from the _nv_unset call on init.c:1645) freed the node pointed to by np, so that the next loop iteration crashed on line 1637 as the dtnext() macro now gets a freed np. Now, why on earth should _nv_unset() *ever* indirectly call nv_delete()? That's a question for another day; I suspect it may be a bug, or it may be needed for compound variables for some reason. For now, I'm adding a workaround: simply avoid calling nv_delete() if the SH_INIT state bit is on, indicating sh_reinit() is in the call stack. This allows the variables unset loop in sh_reinit() to continue without crashing. sh_reinit() handles deletion later anyway. src/cmd/ksh93/sh/main.c: - sh_main(): remove zeroing of sh.fun_depth and sh.dot_depth; these are known to be 0, coming from either sh_init() or sh_reinit(). ________ [*1] This NV_IMPORT/nvenv usage is a redundant holdout from ancient ksh code; the imported value is easily available as a normal shell variable value via nv_getval(). Plus, the nvenv pointer is overloaded with too many other purposes: so far I've discovered it's used for pointers to subarrays of arrays (multidimentional arrays), compound variables, builtins, and other things. This mess caused at least one crash in set_instance() (xec.c) due to incorrectly using that nvenv pointer. The current kshrc script triggers this. Reproducer: $ export PS1 $ bin/package use «0»26:…/src/ksh93/ksh[dev] $ typeset +x PS1 ...and crash. That is now fixed. [*2] https://pubs.opengroup.org/onlinepubs/9699919799/functions/environ.html --- NEWS | 8 + src/cmd/ksh93/include/defs.h | 1 - src/cmd/ksh93/include/shell.h | 5 +- src/cmd/ksh93/include/version.h | 2 +- src/cmd/ksh93/sh/init.c | 287 +++++++++++++++---------------- src/cmd/ksh93/sh/main.c | 3 +- src/cmd/ksh93/sh/name.c | 72 +------- src/cmd/ksh93/sh/nvdisc.c | 7 +- src/cmd/ksh93/sh/nvtree.c | 4 +- src/cmd/ksh93/sh/path.c | 25 ++- src/cmd/ksh93/tests/types.sh | 2 +- src/cmd/ksh93/tests/variables.sh | 4 + 12 files changed, 197 insertions(+), 223 deletions(-) diff --git a/NEWS b/NEWS index b94487c39a76..63b9f1cbbce0 100644 --- a/NEWS +++ b/NEWS @@ -2,6 +2,14 @@ This documents significant changes in the dev branch of ksh 93u+m. For full details, see the git log at: https://github.com/ksh93/ksh Uppercase BUG_* IDs are shell bug IDs as used by the Modernish shell library. +2024-02-29: + +- Improved the robustness of the routines handling the execution by ksh of a + script without an initial #! path. More memory is freed up after ksh forks. + +- Fixed a crash that could occur after 'typeset +x var' when var was imported + from the environment and then a discipline function was set for it. + 2024-02-23: - [v1.1] The commands ':', 'true', 'false', 'break' and 'continue' have been diff --git a/src/cmd/ksh93/include/defs.h b/src/cmd/ksh93/include/defs.h index 293ff83671ee..568d6aee7130 100644 --- a/src/cmd/ksh93/include/defs.h +++ b/src/cmd/ksh93/include/defs.h @@ -116,7 +116,6 @@ extern void sh_chktrap(void); extern void sh_deparse(Sfio_t*,const Shnode_t*,int,int); extern int sh_debug(const char*,const char*,const char*,char *const[],int); extern char **sh_envgen(void); -extern void sh_envnolocal(Namval_t*,void*); extern Sfdouble_t sh_arith(const char*); extern void *sh_arithcomp(char*); extern pid_t sh_fork(int,int*); diff --git a/src/cmd/ksh93/include/shell.h b/src/cmd/ksh93/include/shell.h index ef74c6625802..0e805687cf64 100644 --- a/src/cmd/ksh93/include/shell.h +++ b/src/cmd/ksh93/include/shell.h @@ -346,7 +346,8 @@ struct Shell_s int xargmin; int xargmax; int xargexit; - int nenv; + int save_env_n; /* number of saved pointers to environment variables with invalid names */ + char **save_env; /* saved pointers to environment variables with invalid names */ mode_t mask; void *init_context; void *mac_context; @@ -431,7 +432,7 @@ extern Libcomp_t *liblist; extern void sh_subfork(void); extern Shell_t *sh_init(int,char*[],Shinit_f); -extern int sh_reinit(char*[]); +extern void sh_reinit(void); extern int sh_eval(Sfio_t*,int); extern void sh_delay(double,int); extern void *sh_parse(Sfio_t*,int); diff --git a/src/cmd/ksh93/include/version.h b/src/cmd/ksh93/include/version.h index 04433e38f8da..170e00c5b738 100644 --- a/src/cmd/ksh93/include/version.h +++ b/src/cmd/ksh93/include/version.h @@ -18,7 +18,7 @@ #define SH_RELEASE_FORK "93u+m" /* only change if you develop a new ksh93 fork */ #define SH_RELEASE_SVER "1.1.0-alpha" /* semantic version number: https://semver.org */ -#define SH_RELEASE_DATE "2024-02-23" /* must be in this format for $((.sh.version)) */ +#define SH_RELEASE_DATE "2024-02-29" /* 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. */ diff --git a/src/cmd/ksh93/sh/init.c b/src/cmd/ksh93/sh/init.c index a1b5235af9ff..bdac6232b4dd 100644 --- a/src/cmd/ksh93/sh/init.c +++ b/src/cmd/ksh93/sh/init.c @@ -1380,8 +1380,8 @@ Shell_t *sh_init(int argc,char *argv[], Shinit_f userinit) sfprintf(sh.strbuf,"%s/.kshrc",nv_getval(HOME)); nv_putval(ENVNOD,sfstruse(sh.strbuf),NV_RDONLY); } - *SHLVL->nvalue.ip +=1; - nv_offattr(SHLVL,NV_IMPORT); + /* increase SHLVL */ + shlvl++; #if SHOPT_SPAWN { /* @@ -1531,34 +1531,68 @@ Shell_t *sh_init(int argc,char *argv[], Shinit_f userinit) return &sh; } +/* + * Close all the dictionaries on a viewpath + */ +static void freeup_tree(Dt_t *tree) +{ + Dt_t *view; + while(view = dtview(tree, NULL)) + { + dtclose(tree); + tree = view; + } + dtclose(tree); +} + /* * Reinitialize before executing a script without a #! path. - * This is done in a fork of the shell, so "exporting" is done by deleting all non-exported variables. + * This is done in a fork of the shell. */ -int sh_reinit(char *argv[]) +void sh_reinit(void) { Shopt_t opt; Namval_t *np,*npnext; Dt_t *dp; - int nofree; - char *savfpath = NULL; - sh.subshell = sh.realsubshell = sh.comsub = sh.curenv = sh.jobenv = sh.inuse_bits = sh.fn_depth = sh.dot_depth = 0; - sh.envlist = NULL; - sh.last_root = sh.var_tree; + sh_onstate(SH_INIT); + sh_offstate(SH_FORKED); + /* Reset shell options; inherit some */ + memset(&opt,0,sizeof(opt)); + if(sh_isoption(SH_POSIX)) + on_option(&opt,SH_POSIX); +#if SHOPT_ESH + if(sh_isoption(SH_EMACS)) + on_option(&opt,SH_EMACS); + if(sh_isoption(SH_GMACS)) + on_option(&opt,SH_GMACS); +#endif +#if SHOPT_VSH + if(sh_isoption(SH_VI)) + on_option(&opt,SH_VI); +#endif + sh.options = opt; + /* Reset here-document */ if(sh.heredocs) { sfclose(sh.heredocs); sh.heredocs = 0; } - /* Unset tilde expansion disciplines */ - _nv_unset(SH_TILDENOD,NV_RDONLY); - /* save FPATH and treat specially */ - if(nv_isattr(FPATHNOD,NV_EXPORT)) - savfpath = sh_strdup(nv_getval(FPATHNOD)); - _nv_unset(FPATHNOD,NV_RDONLY); - /* Remove non-exported variables, first pass (see sh_envnolocal() in name.c) */ - nv_scan(sh.var_tree,sh_envnolocal,NULL,NV_EXPORT,0); - nv_scan(sh.var_tree,sh_envnolocal,NULL,NV_ARRAY,NV_ARRAY); + /* Reset arguments */ + if(sh.arglist) + sh_argreset(sh.arglist,NULL); + sh.shname = error_info.id = sh_strdup(sh.st.dolv[0]); + /* Reset traps and signals */ + memset(sh.st.trapcom,0,(sh.st.trapmax+1)*sizeof(char*)); + sh_sigreset(0); + sh.st.filename = sh_strdup(sh.lastarg); + nv_delete(NULL, NULL, 0); + job.exitval = 0; + sh.inpipe = sh.outpipe = 0; + job_clear(); + job.in_critical = 0; + /* update $$, $PPID */ + sh.ppid = sh.current_ppid; + sh.pid = sh.current_pid; #if SHOPT_NAMESPACE if(sh.namespace) { @@ -1566,148 +1600,107 @@ int sh_reinit(char *argv[]) if(dp==sh.var_tree) sh.var_tree = dtview(dp,0); _nv_unset(sh.namespace,NV_RDONLY); - sh.namespace = 0; + sh.namespace = NULL; } #endif /* SHOPT_NAMESPACE */ - /* Delete remaining non-exported, non-default variables; remove attributes from exported variables */ - for(np = dtfirst(sh.var_tree); np; np = npnext) - { - if((dp = sh.var_tree)->walk) - dp = dp->walk; - npnext = (Namval_t*)dtnext(sh.var_tree,np); - /* skip default variables (already handled by sh_envnolocal()) */ - if(np >= sh.bltin_nodes && np < &sh.bltin_nodes[nvars]) - continue; - if(nv_isattr(np,NV_EXPORT)) - { - char *cp = NULL; - /* do not export attributes */ - if(nv_isattr(np,NV_INTEGER)) /* any kind of numeric? */ - { - cp = sh_strdup(nv_getval(np)); /* save string value */ - _nv_unset(np,NV_RDONLY); /* free numeric value */ - } - nv_setattr(np,NV_EXPORT); /* turn off everything except export */ - if(cp) - np->nvalue.cp = cp; /* replace by string value */ - } - else - { - nofree = nv_isattr(np,NV_NOFREE); /* note: returns bitmask, not boolean */ - _nv_unset(np,NV_RDONLY); /* also clears NV_NOFREE attr, if any */ - nv_delete(np,dp,nofree); - } - } - /* Delete types */ - for(np = dtfirst(sh.typedict); np; np = npnext) - { - if((dp = sh.typedict)->walk) - dp = dp->walk; - npnext = (Namval_t*)dtnext(sh.typedict,np); - nv_delete(np,dp,0); - } - /* Delete functions/built-ins. Note: fun_tree has a viewpath to bltin_tree */ - for(np = dtfirst(sh.fun_tree); np; np = npnext) - { - if((dp = sh.fun_tree)->walk) - dp = dp->walk; - npnext = (Namval_t*)dtnext(sh.fun_tree,np); - nv_delete(np, dp, NV_NOFREE); - } - while(dp = dtview(sh.fun_tree, NULL)) - { - dtclose(sh.fun_tree); - sh.fun_tree = dp; - } - sh.fun_base = sh.fun_tree; - dtclear(sh.fun_base); - /* Re-init built-ins as per nv_init() */ - free(sh.bltin_cmds); - sh.bltin_tree = sh_inittree((const struct shtable2*)shtab_builtins); - dtview(sh.fun_tree,sh.bltin_tree); /* Delete aliases */ for(np = dtfirst(sh.alias_tree); np; np = npnext) { if((dp = sh.alias_tree)->walk) - dp = dp->walk; + dp = dp->walk; /* the dictionary in which the item was found */ npnext = (Namval_t*)dtnext(sh.alias_tree,np); - _nv_unset(np,nv_isattr(np,NV_NOFREE)); + _nv_unset(np,NV_RDONLY); nv_delete(np,dp,0); } /* Delete hash table entries */ for(np = dtfirst(sh.track_tree); np; np = npnext) { if((dp = sh.track_tree)->walk) - dp = dp->walk; + dp = dp->walk; /* the dictionary in which the item was found */ npnext = (Namval_t*)dtnext(sh.track_tree,np); - nofree = nv_isattr(np,NV_NOFREE); /* note: returns bitmask, not boolean */ - _nv_unset(np,NV_RDONLY); /* also clears NV_NOFREE attr, if any */ - nv_delete(np,dp,nofree); + _nv_unset(np,NV_RDONLY); + nv_delete(np,dp,0); } - while(dp = dtview(sh.track_tree, NULL)) + /* Delete types */ + for(np = dtfirst(sh.typedict); np; np = npnext) { - dtclose(sh.track_tree); - sh.track_tree = dp; + if((dp = sh.typedict)->walk) + dp = dp->walk; /* the dictionary in which the item was found */ + npnext = (Namval_t*)dtnext(sh.typedict,np); + nv_delete(np,dp,0); } -#if SHOPT_STATS - /* Reset statistics */ - free(sh.stats); - stat_init(); -#endif - /* Reset shell options; inherit some */ - memset(&opt,0,sizeof(opt)); - if(sh_isoption(SH_POSIX)) - on_option(&opt,SH_POSIX); -#if SHOPT_ESH - if(sh_isoption(SH_EMACS)) - on_option(&opt,SH_EMACS); - if(sh_isoption(SH_GMACS)) - on_option(&opt,SH_GMACS); -#endif -#if SHOPT_VSH - if(sh_isoption(SH_VI)) - on_option(&opt,SH_VI); -#endif - sh.options = opt; - /* set up new args */ - if(argv) - sh.arglist = sh_argcreate(argv); - if(sh.arglist) - sh_argreset(sh.arglist,NULL); - sh.shname = error_info.id = sh_strdup(sh.st.dolv[0]); - sh_offstate(SH_FORKED); - /* Reset traps and signals */ - memset(sh.st.trapcom,0,(sh.st.trapmax+1)*sizeof(char*)); - sh_sigreset(0); - /* increase SHLVL */ - if(!(SHLVL->nvalue.ip)) + /* Unset all variables (don't delete yet) */ + for(np = dtfirst(sh.var_tree); np; np = npnext) { - shlvl = 0; - SHLVL->nvalue.ip = &shlvl; - nv_onattr(SHLVL,NV_INTEGER|NV_EXPORT|NV_NOFREE); + int nofree; + npnext = (Namval_t*)dtnext(sh.var_tree,np); + if(np==DOTSHNOD || np==L_ARGNOD) /* TODO: unset these without crashing */ + continue; + if(nv_isref(np)) + nv_unref(np); + if(nv_isarray(np)) + nv_putsub(np,NULL,ARRAY_UNDEF); + nofree = nv_isattr(np,NV_NOFREE); /* note: returns bitmask, not boolean */ + _nv_unset(np,NV_RDONLY); /* also clears NV_NOFREE attr, if any */ + nv_setattr(np,nofree); + } + /* Delete functions and built-ins. Note: fun_tree has a viewpath to bltin_tree */ + for(np = dtfirst(sh.fun_tree); np; np = npnext) + { + if((dp = sh.fun_tree)->walk) + dp = dp->walk; /* the dictionary in which the item was found */ + npnext = (Namval_t*)dtnext(sh.fun_tree,np); + if(dp==sh.bltin_tree) + { + if(np->nvalue.bfp) + sh_addbuiltin(nv_name(np), np->nvalue.bfp, pointerof(1)); + } + else if(is_afunction(np)) + { + _nv_unset(np,NV_RDONLY); + nv_delete(np,dp,NV_FUNCTION); + } } - *SHLVL->nvalue.ip +=1; - nv_offattr(SHLVL,NV_IMPORT); - sh.st.filename = sh_strdup(sh.lastarg); - nv_delete(NULL, NULL, 0); - job.exitval = 0; - sh.inpipe = sh.outpipe = 0; - job_clear(); - job.in_critical = 0; - /* update $$, $PPID */ - sh.ppid = sh.current_ppid; - sh.pid = sh.current_pid; - /* restore an exported FPATH */ - if(savfpath) + /* Delete all variables in a separate pass; this avoids 'no parent' errors while + * unsetting variables or discipline functions with dot names like .foo.bar */ + for(np = dtfirst(sh.var_tree); np; np = npnext) { - nv_setattr(FPATHNOD,NV_EXPORT); - nv_putval(FPATHNOD,savfpath,0); - free(savfpath); + if((dp = sh.var_tree)->walk) + dp = dp->walk; /* the dictionary in which the item was found */ + npnext = (Namval_t*)dtnext(sh.var_tree,np); + /* cannot delete default variables */ + if(np >= sh.bltin_nodes && np < &sh.bltin_nodes[nvars]) + continue; + nv_delete(np,dp,nv_isattr(np,NV_NOFREE)); } + /* Reset state for subshells, environment, job control, function calls and file descriptors */ + sh.subshell = sh.realsubshell = sh.comsub = sh.curenv = sh.jobenv = sh.inuse_bits = sh.fn_depth = sh.dot_depth = 0; + sh.envlist = NULL; + sh.last_root = NULL; + /* Free up the dictionary trees themselves */ + freeup_tree(sh.fun_tree); /* includes sh.bltin_tree */ + freeup_tree(sh.alias_tree); + freeup_tree(sh.track_tree); + freeup_tree(sh.typedict); + freeup_tree(sh.var_tree); +#if SHOPT_STATS + free(sh.stats); + sh.stats = NULL; +#endif + /* Re-init variables, functions and built-ins */ + free(sh.bltin_cmds); + free(sh.bltin_nodes); + free(sh.mathnodes); + free(sh.init_context); + sh.init_context = nv_init(); + /* Re-import the environment (re-exported in exscript()) */ + env_init(); + /* Increase SHLVL */ + shlvl++; /* call user init function, if any */ if(sh.userinit) (*sh.userinit)(&sh, 1); - return 1; + sh_offstate(SH_INIT); } /* @@ -2011,26 +2004,28 @@ Dt_t *sh_inittree(const struct shtable2 *name_vals) void env_init(void) { char *cp; - Namval_t *np; char **ep=environ; + int save_env_n = 0; if(ep) { while(cp = *ep++) { if(strncmp(cp,"KSH_VERSION=",12)==0) continue; - else if(np = nv_open(cp,sh.var_tree,(NV_EXPORT|NV_IDENT|NV_ASSIGN|NV_NOFAIL))) - { - nv_onattr(np,NV_IMPORT); - np->nvenv = cp; - } - else /* swap with front */ - { - ep[-1] = environ[sh.nenv]; - environ[sh.nenv++] = cp; + if(!nv_open(cp,sh.var_tree,(NV_EXPORT|NV_IDENT|NV_ASSIGN|NV_NOFAIL)) && !sh.save_env_n) + { /* + * If the shell assignment via nv_open() failed, we cannot import this + * env var (invalid name); save it for sh_envgen() to pass it on to + * child processes. This does not need to be re-done after forking. + */ + save_env_n++; + sh.save_env = sh_realloc(sh.save_env, save_env_n*sizeof(char*)); + sh.save_env[save_env_n-1] = cp; } } } + if(save_env_n) + sh.save_env_n = save_env_n; if(nv_isnull(PWDNOD) || nv_isattr(PWDNOD,NV_TAGGED)) { nv_offattr(PWDNOD,NV_TAGGED); diff --git a/src/cmd/ksh93/sh/main.c b/src/cmd/ksh93/sh/main.c index eb98534eed3a..a650aeeb0d45 100644 --- a/src/cmd/ksh93/sh/main.c +++ b/src/cmd/ksh93/sh/main.c @@ -133,9 +133,8 @@ int sh_main(int ac, char *av[], Shinit_f userinit) if(sigsetjmp(*((sigjmp_buf*)sh.jmpbuffer),0)) { /* begin script execution here */ - sh_reinit(NULL); + sh_reinit(); } - sh.fn_depth = sh.dot_depth = 0; command = error_info.id; path_pwd(); iop = NULL; diff --git a/src/cmd/ksh93/sh/name.c b/src/cmd/ksh93/sh/name.c index f6a005160bea..b372c708f56b 100644 --- a/src/cmd/ksh93/sh/name.c +++ b/src/cmd/ksh93/sh/name.c @@ -1672,8 +1672,6 @@ void nv_putval(Namval_t *np, const char *string, int flags) up = np->nvalue.up; if(up && up->cp==Empty) up->cp = 0; - if(nv_isattr(np,NV_EXPORT)) - nv_offattr(np,NV_IMPORT); if(nv_isattr (np, NV_INTEGER)) { if(nv_isattr(np, NV_DOUBLE) == NV_DOUBLE) @@ -2143,11 +2141,8 @@ static char *staknam(Namval_t *np, char *value) char *p,*q; q = stkalloc(sh.stk,strlen(nv_name(np))+(value?strlen(value):0)+2); p=strcopy(q,nv_name(np)); - if(value) - { - *p++ = '='; - strcpy(p,value); - } + *p++ = '='; + strcpy(p,value); return q; } @@ -2161,9 +2156,7 @@ static void pushnam(Namval_t *np, void *data) if(strchr(np->nvname,'.')) return; ap->tp = 0; - if(nv_isattr(np,NV_IMPORT) && np->nvenv) - *ap->argnam++ = np->nvenv; - else if(value=nv_getval(np)) + if(value=nv_getval(np)) *ap->argnam++ = staknam(np,value); } @@ -2180,11 +2173,13 @@ char **sh_envgen(void) /* L_ARGNOD gets generated automatically as full path name of command */ nv_offattr(L_ARGNOD,NV_EXPORT); namec = nv_scan(sh.var_tree,nullscan,NULL,NV_EXPORT,NV_EXPORT); - namec += sh.nenv; + namec += sh.save_env_n; er = stkalloc(sh.stk,(namec+4)*sizeof(char*)); - data.argnam = (er+=2) + sh.nenv; - if(sh.nenv) - memcpy(er,environ,sh.nenv*sizeof(char*)); + data.argnam = (er+=2) + sh.save_env_n; + /* Pass non-imported env vars to child */ + if(sh.save_env_n) + memcpy(er,sh.save_env,sh.save_env_n*sizeof(char*)); + /* Add exported vars */ nv_scan(sh.var_tree, pushnam,&data,NV_EXPORT, NV_EXPORT); *data.argnam = 0; return er; @@ -2305,53 +2300,6 @@ void sh_scope(struct argnod *envlist, int fun) sh.var_tree = newscope; } -/* - * Remove freeable local space associated with the nvalue field - * of nnod. This includes any strings representing the value(s) of the - * node, as well as its dope vector, if it is an array. - */ -void sh_envnolocal (Namval_t *np, void *data) -{ - char *cp = 0, was_export = nv_isattr(np,NV_EXPORT)!=0; - NOT_USED(data); - if(np==VERSIONNOD && nv_isref(np)) - return; - if(np==L_ARGNOD) - return; - if(np == sh.namespace) - return; - if(nv_isref(np)) - nv_unref(np); - if(nv_isattr(np,NV_EXPORT) && nv_isarray(np)) - { - nv_putsub(np,NULL,0); - if(cp = nv_getval(np)) - cp = sh_strdup(cp); - } - if(nv_isattr(np,NV_EXPORT|NV_NOFREE)) - { - if(nv_isref(np) && np!=VERSIONNOD) - { - nv_offattr(np,NV_NOFREE|NV_REF); - free(np->nvalue.nrp); - np->nvalue.cp = 0; - } - if(!cp) - return; - } - if(nv_isarray(np)) - nv_putsub(np,NULL,ARRAY_UNDEF); - _nv_unset(np,NV_RDONLY); - nv_setattr(np,0); - if(cp) - { - nv_putval(np,cp,0); - free(cp); - } - if(was_export) - nv_onattr(np,NV_EXPORT); -} - static void table_unset(Dt_t *root, int flags, Dt_t *oroot) { Namval_t *np,*nq, *npnext; @@ -2910,8 +2858,6 @@ void nv_newattr (Namval_t *np, unsigned newatts, int size) /* handle attributes that do not change data separately */ n = np->nvflag; trans = !(n&NV_INTEGER) && (n&(NV_LTOU|NV_UTOL)); /* transcode to lower or upper case */ - if(newatts&NV_EXPORT) - nv_offattr(np,NV_IMPORT); if(((n^newatts)&NV_EXPORT)) /* EXPORT attribute has been toggled */ { /* record changes to the environment */ diff --git a/src/cmd/ksh93/sh/nvdisc.c b/src/cmd/ksh93/sh/nvdisc.c index 1e2b62b41f82..66916e1216a3 100644 --- a/src/cmd/ksh93/sh/nvdisc.c +++ b/src/cmd/ksh93/sh/nvdisc.c @@ -328,11 +328,8 @@ static void assign(Namval_t *np,const char* val,int flags,Namfun_t *handle) /* restore everything but the nvlink field */ memcpy(&SH_VALNOD->nvname, &node.nvname, sizeof(node)-sizeof(node.nvlink)); } - else if(sh_isstate(SH_INIT) || np==SH_FUNNAMENOD) - { - /* don't free functions during reinitialization */ + else if(np==SH_FUNNAMENOD) nv_putv(np,val,flags,handle); - } else if(!nq || !isblocked(bp,type)) { Dt_t *root = sh_subfuntree(1); @@ -1144,7 +1141,7 @@ Namval_t *sh_addbuiltin(const char *path, Shbltin_f bltin, void *extra) stkseek(sh.stk,offset); if(extra == (void*)1) { - if(nv_isattr(np,BLT_SPC)) + if(nv_isattr(np,BLT_SPC) && !sh_isstate(SH_INIT)) { /* builtin(1) cannot delete special builtins */ errormsg(SH_DICT,ERROR_exit(1),"cannot delete: %s%s",name,is_spcbuiltin); diff --git a/src/cmd/ksh93/sh/nvtree.c b/src/cmd/ksh93/sh/nvtree.c index 58e185a5d95e..48d69155c50c 100644 --- a/src/cmd/ksh93/sh/nvtree.c +++ b/src/cmd/ksh93/sh/nvtree.c @@ -727,7 +727,9 @@ static void outval(char *name, const char *vname, struct Walk *wp) _nv_unset(np,NV_RDONLY); if(sh.subshell || (wp->flags!=NV_RDONLY) || nv_isattr(np,NV_MINIMAL|NV_NOFREE)) wp->root = 0; - nv_delete(np,wp->root,nv_isattr(np,NV_MINIMAL)?NV_NOFREE:0); + /* Delete the node from the tree and free np, unless we're unsetting variables in sh_reinit() */ + if(!sh_isstate(SH_INIT)) + nv_delete(np,wp->root,nv_isattr(np,NV_MINIMAL)?NV_NOFREE:0); return; } if(isarray==1 && !nq) diff --git a/src/cmd/ksh93/sh/path.c b/src/cmd/ksh93/sh/path.c index 54f665db9ec3..46d45d050b2d 100644 --- a/src/cmd/ksh93/sh/path.c +++ b/src/cmd/ksh93/sh/path.c @@ -1186,7 +1186,10 @@ pid_t path_spawn(const char *opath,char **argv, char **envp, Pathcomp_t *libpath case EISDIR: return -1; case ENOEXEC: - errno = ENOEXEC; + /* + * A script without #! -- it starts here. Summary of events: + * fork; exscript; longjmp back to sh_main; sh_reinit; exfile + */ if(spawn) { if(sh.subshell) @@ -1304,6 +1307,26 @@ static noreturn void exscript(char *path,char *argv[],char **envp) sh_offstate(SH_FORKED); if(sh.sigflag[SIGCHLD]==SH_SIGOFF) sh.sigflag[SIGCHLD] = SH_SIGFAULT; + /* + * Export -x vars to new environment now, before longjmp & removing any local scope. + * Since sh_envgen() puts it all on the stack, create a stack to preserve 'environ'. + */ + { + static Stk_t *envstk; + Stk_t *savstk = sh.stk; + if (envstk) + stkset(envstk, NULL, 0); + else + envstk = stkopen(STK_SMALL); + sh.stk = envstk; + environ = sh_envgen(); + sh.stk = savstk; + stkfreeze(envstk,0); + } + /* + * Longjmp with SH_JMPSCRIPT triggers a chain of longjmps to restore state as appropriate, + * ending up back in sh_main() which then calls sh_reinit() and executes the script. + */ siglongjmp(*sh.jmplist,SH_JMPSCRIPT); UNREACHABLE(); /* silence warning on Haiku */ } diff --git a/src/cmd/ksh93/tests/types.sh b/src/cmd/ksh93/tests/types.sh index 944d51837427..ed8af512d2b3 100755 --- a/src/cmd/ksh93/tests/types.sh +++ b/src/cmd/ksh93/tests/types.sh @@ -673,7 +673,7 @@ exp=': trap: is a special shell builtin' # ====== # Bugs involving scripts without a #! path -# Hashbangless scripts are executed in a reinitialised fork of ksh, which is very bug-prone. +# Hashbangless scripts are executed in a reinitialised fork of ksh. # https://github.com/ksh93/ksh/issues/350 # Some of these fixed bugs don't involve types at all, but the tests need to go somewhere. # Plus, invoking these from an environment with a bunch of types defined is an additional test. diff --git a/src/cmd/ksh93/tests/variables.sh b/src/cmd/ksh93/tests/variables.sh index aaed3943d773..0f314351454b 100755 --- a/src/cmd/ksh93/tests/variables.sh +++ b/src/cmd/ksh93/tests/variables.sh @@ -905,6 +905,10 @@ $SHELL -c "$cmd" 2>/dev/null || err_exit "'$cmd' exit status $?, expected 0" SHLVL=1 level=$($SHELL -c $'$SHELL -c \'print -r "$SHLVL"\'') [[ $level == 3 ]] || err_exit "SHLVL should be 3 not $level" +echo 'print -r "$SHLVL"' >script +chmod +x script +level=$($SHELL -c '$SHELL ./script') +[[ $level == 3 ]] || err_exit "SHLVL should be 3 not $level" [[ $($SHELL -c '{ x=1; : ${x.};print ok;}' 2> /dev/null) == ok ]] || err_exit '${x.} where x is a simple variable causes shell to abort'