Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Browse files
Browse the repository at this point in the history
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- Loading branch information
