1
0
Fork 0
mirror of git://git.code.sf.net/p/cdesktopenv/code synced 2025-03-09 15:50:02 +00:00

Fix 'return' emitting signals; allow arbitrary return values

When a global EXIT trap is set, and a ksh-style function exits with
a status > 256 that could have been the result of a signal, then
the shell incorrectly issues that signal to itself. Depending on
the signal, this causes ksh to terminate itself ungracefully:

    $ cat /tmp/exit267
    trap 'echo OK' EXIT  # This trap triggers the crash
    function foo { return 267; }
    foo
    $ bash /tmp/exit267
    OK
    $ ksh-3aee10d7 /tmp/exit267
    OK
    $ ksh /tmp/exit267
    Memory fault(coredump)

On most systems, status 267 corresponds to SIGSEGV. The reported
memory fault is not real; it results from ksh incorrectly killing
itself with that signal.

The problem is caused by two factors:

1. As of 93u+ 2012-08-01, ksh explicitly allows 'return' to use an
   exit status corresponding to a signal (from 257 to end of signal
   range). The rest of the integer range is trunctated to 8 bits.
   This is contrary to both 'man ksh' and 'return --man' which both
   say it's always truncated to 8 bits. Plus, combined with point 2
   below, this new behaviour is nonsensical, as 'return' has no
   business actually generating signals. However, a couple of
   regression tests now depend on this, as may some scripts.

2. When a ksh-style function does not handle a signal, the signal
   is passed down to the parent environment and ksh does this by
   reissuing the signal to its own process after leaving the
   function scope. However, it does this by checking the exit
   status, which is very bad practice as there is no guarantee
   that an exit status corresponding to a signal was in fact
   produced by a signal, particularly after they changed the
   behaviour of 'return' per 1 above.

This commit fixes both issues. It also takes a proper decision on
allowable 'return' exit status arguments. Since 93u+ was released
nearly a decade ago and some scripts may now rely on being able to
pass certain exit statuses out of the 8-bit range, we should not
disallow this now. But neither should we be half-hearted in
allowing only some arbitrary selection of 9-bit statuses; 'return'
values categorically should have nothing to do with signals, so
this is no basis for limiting them. We're now allowing the full
unsigned integer range, which is usually 32 bits. This is like zsh,
and may create some interesting possibilities for scripts.
Just don't forget that $? will still lose all but its 8 least
significant bits when leaving the current (sub)shell environment.

src/cmd/ksh93/sh/xec.c: sh_funscope():
- Fix passing down unhandled signals from interrupted ksh functions
  (jumpval==SH_JMPFUN) to the parent environment. Do not pay any
  attention to the exit status. Instead, use sh.lastsig (a.k.a.
  shp->lastsig). It is set by sh_fault() in fault.c for just this
  purpose and contains the last signal handled for the current
  command. It is reset in sh_exec() before running any new command.
  So if it contains a signal, that is the one that interrupted the
  ksh function, so it's the correct one to pass down. (Further
  evidence: sh_subshell() was already using this in the same way.)

src/cmd/ksh93/bltins/cflow.c: b_return():
- Allow any signed int return value when invoked as and behaving
  like 'return'.
- Add warning if a passed value is out of int range. Set the exit
  status to 128 in that case; int overflow is undefined behaviour
  in C and we want consistent behaviour across platforms. It should
  be safe enough to check if the long and int values are equal.
- Refactor for clarity.

src/cmd/ksh93/sh/subshell.c: sh_subshell():
- If a function returns with a status out of the 8 bit range in a
  virtual subshell, this status could be passed down to the parent
  shell in full. However, if the subshell forks, then the kernel
  will enforce an 8-bit exit status. That is inconsistent. Scripts
  should not be able to tell the difference between forked and
  non-forked subshells, so artificially enforce that limit here.

Other changed files:
- Documentation updates and copy-edits.
- Update an AT&T functions.sh regress test to allow arbitrary
  integer return values for functions.
- Add regression tests based in part on @JohnoKing's reproducers.
- Rework some vaguely related regression tests to fail gracefully.

Thanks to Johnothan King for the report and the testing.

Fixes: https://github.com/ksh93/ksh/issues/364
This commit is contained in:
Martijn Dekker 2021-12-08 04:14:15 +01:00
parent 7fb814e186
commit b3050769ea
11 changed files with 149 additions and 74 deletions

View file

@ -21,7 +21,7 @@
#define SH_RELEASE_FORK "93u+m" /* only change if you develop a new ksh93 fork */
#define SH_RELEASE_SVER "1.0.0-beta.2" /* semantic version number: https://semver.org */
#define SH_RELEASE_DATE "2021-12-05" /* must be in this format for $((.sh.version)) */
#define SH_RELEASE_DATE "2021-12-08" /* must be in this format for $((.sh.version)) */
#define SH_RELEASE_CPYR "(c) 2020-2021 Contributors to ksh " SH_RELEASE_FORK
/* Scripts sometimes field-split ${.sh.version}, so don't change amount of whitespace. */