ksh: shell crashes when read times out

[ 6:38 PM][ttys001][~/test/bz573936]
[211] mbp13 $ type ksh
ksh is a tracked alias for /usr/local/bin/ksh
[ 6:38 PM][ttys001][~/test/bz573936]
[212] mbp13 $ ksh
[ 6:38 PM][ttys001][~/test/bz573936]
[213] mbp13 $ TMOUT=5
[ 6:38 PM][ttys001][~/test/bz573936]
[214] mbp13 $ read
[ 6:39 PM][ttys001][~/test/bz573936]
[215] mbp13 $                           
shell will timeout in 60 seconds due to inactivity
Memory fault
[ 6:39 PM][ttys001][~/test/bz573936]
[215] mbp13 $

I read https://bugzilla.redhat.com/show_bug.cgi?id=573936 and tested it, didn’t see the behavior the bug talked about, figured it must no longer be a problem.

Then the window I was running ksh in closed.

The read timed out after 5 seconds, and the prompt came back. Then ksh popped the timeout message and the memory fault, and the shell crashed.

I can’t make it happen if I put it in a script and call the script, but it happens repeatably if I do it interactively. If I don’t set a timeout and just let the read sit there, it doesn’t crash.

(macOS 10.15.6, Version AJM 93u+m 2020-07-29)

About this issue

  • Original URL
  • State: closed
  • Created 4 years ago
  • Comments: 49

Commits related to this issue

Most upvoted comments

I’m getting nowhere with trying to reproduce any variant of this crash, even when using your complete prompt setup. I just can’t get it to malfunction at all. There must still be something in your setup that we’re missing.

However, based on your crash backtrace, I came up with something. The backtrace includes comsubst() and sh_subshell(), indicating that the crash occurs while a command substitution is being executed. Job control is disabled in command substitutions, so it doesn’t seem to make sense to call job_unpost() (the function that crashed) while in a command substitution. It’s a bit of a shot in the dark, but I’m curious if the following patch makes any difference on your end…

diff --git a/src/cmd/ksh93/sh/jobs.c b/src/cmd/ksh93/sh/jobs.c
index 3170276..915258f 100644
--- a/src/cmd/ksh93/sh/jobs.c
+++ b/src/cmd/ksh93/sh/jobs.c
@@ -460,7 +460,7 @@ int job_reap(register int sig)
 		nochild = 1;
 	}
 	shp->gd->waitevent = waitevent;
-	if(sh_isoption(SH_NOTIFY) && sh_isstate(SH_TTYWAIT))
+	if(job.jobcontrol && sh_isoption(SH_NOTIFY) && sh_isstate(SH_TTYWAIT))
 	{
 		outfile = sfstderr;
 		job_list(pw,JOB_NFLAG|JOB_NLFLAG);

MOST interesting! If I reset PS1, I can’t make the segfault occur. See here, where first it occurs, then reset PS1, then it doesn’t occur. I have set -b for this test.

[10:13 AM][ttys001][~/.ksh/rc]
[328] mbp13 $ ksh
[10:13 AM][ttys001][~/.ksh/rc]
[329] mbp13 $ TMOUT=5
[10:13 AM][ttys001][~/.ksh/rc]
[330] mbp13 $ read
[10:13 AM][ttys001][~/.ksh/rc]
[331] mbp13 $ 
shell will timeout in 60 seconds due to inactivity
Memory fault
[10:13 AM][ttys001][~/.ksh/rc]
[331] mbp13 $ ksh
[10:14 AM][ttys001][~/.ksh/rc]
[332] mbp13 $ PS1="$ "
$ TMOUT=5
$ read
$ 
shell will timeout in 60 seconds due to inactivity
$ ksh: timed out waiting for input

[10:15 AM][ttys001][~/.ksh/rc]
[335] mbp13 $ 

My usual prompt is:

case $TERM in
    # xterm and variants, set the titlebar
    xterm*) PS1='\033]0;[\u@\h][\S]\007${RED}[\@]${OFF}${BLUE}[\l]${OFF}${YELLOW}[\S]${OFF}${CYAN}\b${OFF}\n${GREEN}[\#]${OFF} \h \$ ' ;;
    # anything else, don't try to set the titlebar
    *) PS1='${RED}[\@]${OFF}${BLUE}[\l]${OFF}${YELLOW}[\S]${OFF}\n\h \$ ' ;;
esac

Which ends up being this after the discipline function gets through with it…

[337] mbp13 $ echo $PS1
${RED}[$(printf '%(%l:%M %p)T')]${OFF}${BLUE}[$(basename "$(tty)")]${OFF}${YELLOW}[$(printf '%s' ${RELATIVE_PWD})]${OFF}${CYAN}$(git branch 2> /dev/null | sed -e '/^[^*]/d' -e 's/* \(.*\)/[\1]/')${OFF} ${GREEN}[!]${OFF} mbp13 $

And here is prompt.ksh.

# dicipline finction for setting PS1.  ksh93u+ includes a slightly different one from dgk
# I use this one because I didn't know about the other one
# https://blog.fpmurphy.com/2016/08/bash-like-customizable-prompt-in-korn-shell.html

function PS1.set
{
    typeset prefix remaining=${.sh.value} var= n= k=
    set -A .sh.lversion ${.sh.version}

    while [[ $remaining ]]
    do
        prefix=${remaining%%'\'*}
        remaining=${remaining#$prefix}
        var+="$prefix"

        case ${remaining:1:1} in
            A)    var+="\$(printf '%(%R)T')";;
            b)    var+="\$(git branch 2> /dev/null | sed -e '/^[^*]/d' -e 's/* \(.*\)/[\1]/')";;
            @)    var+="\$(printf '%(%l:%M %p)T')";;  # was var+="\$(printf '%(%H:%M %p)T')"
            d)    var+="\$(printf '%(%a %b:%d)T')";;
            e)    var+="\$'\e'";;
            h)    var+=$(hostname -s);;
            H)    var+=$(hostname);;
            j)    var+="\$(jobs | wc -l)";;
            l)    var+="\$(basename \"\$(tty)\")";;
            n)    var+=$'\n';;
            r)    var+=$'\r';;
            s)    var+="\$(basename \"\$0\")";;
            S)    var+="\$(printf '%s' \${RELATIVE_PWD})" ;; # added this one to the list
            t)    var+="\$(printf '%(%H:%M:%S)T')";;
            T)    var+="\$(printf '%(%I:%M:%S)T')";;
            u)    var+=$USER;;
            v)    var+="\${.sh.lversion[2]}";;
            V)    var+="\${.sh.lversion[2]} (\${.sh.lversion[1]})";;
            w)    var+="\$(pwd)";;
            W)    var+="\$(basename \"\$(pwd)\")";;
          '#')    var+=!;;
            !)    var+=!;;
          '$')    if (( $(id -u) == 0 ))
                  then
                      var+='#'
                  else
                      var+='$'
                  fi;;
          '\')    var+='\\';;
      '['|']')    ;;
        [0-7])    case ${remaining:1:3} in
                   [0-7][0-7][0-7])   k=4;;
                            [0-7][0-7])   k=3;;
                                     *)   k=2;;
                  esac
                  eval n="\$'"${remaining:0:k}"'"
                  var+=$n
                  remaining=${remaining:k}
                  continue ;;
           "")    ;;
            *)    var+='\'${remaining:0:2};;
        esac
        remaining=${remaining:2}
    done
    .sh.value=$var
}

# RELATIVE_PWD.get is actually stolen from polyglot.sh
# https://github.com/agkozak/polyglot
# Copyright 2017-2020 Alexandros Kozak
#
# Permission is hereby granted, free of charge, to any person obtaining a copy
# of this software and associated documentation files (the "Software"), to deal
# in the Software without restriction, including without limitation the rights
# to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
# copies of the Software, and to permit persons to whom the Software is
# furnished to do so, subject to the following conditions:
#
# The above copyright notice and this permission notice shall be included in all
# copies or substantial portions of the Software.
#
# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
# IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
# AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
# OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
# SOFTWARE.

function RELATIVE_PWD.get {

    typeset POLYGLOT_DIRTRIM_ELEMENTS= POLYGLOT_PWD_MINUS_HOME= POLYGLOT_OLD_IFS= POLYGLOT_ABBREVIATED_PATH=

      POLYGLOT_DIRTRIM_ELEMENTS="${1:-2}"

      # If root has / as $HOME, print /, not ~
      [ "$PWD" = '/' ] && printf '%s' '/' && return
      [ "$PWD" = "$HOME" ] && printf '%s' '~' && return

      case $HOME in
        /) POLYGLOT_PWD_MINUS_HOME="$PWD" ;;            # In case root's $HOME is /
        *) POLYGLOT_PWD_MINUS_HOME="${PWD#$HOME}" ;;
      esac

      if [ "$POLYGLOT_DIRTRIM_ELEMENTS" -eq 0 ]; then
        [ "$HOME" = '/' ] && printf '%s' "$PWD" && return # need to fix this one yet
        case $PWD in
          ${HOME}*) .sh.value="$(printf '~%s' "$POLYGLOT_PWD_MINUS_HOME")" ;;
          *) .sh.value="$(printf '%s' "$PWD")" ;;
        esac
      else
        # Calculate the part of $PWD that will be displayed in the prompt
        POLYGLOT_OLD_IFS="$IFS"
        IFS='/'
        # shellcheck disable=SC2086
        set -- $POLYGLOT_PWD_MINUS_HOME
        shift                                  # Discard empty first field preceding /

        # Discard path elements > $POLYGLOT_PROMPT_DIRTRIM
        while [ $# -gt "$POLYGLOT_DIRTRIM_ELEMENTS" ]; do
          shift
        done

        # Reassemble the remaining path elements with slashes
        while [ $# -ne 0 ]; do
          POLYGLOT_ABBREVIATED_PATH="${POLYGLOT_ABBREVIATED_PATH}/$1"
          shift
        done

        IFS="$POLYGLOT_OLD_IFS"

        # If the working directory has not been abbreviated, display it thus
        if [ "$POLYGLOT_ABBREVIATED_PATH" = "${POLYGLOT_PWD_MINUS_HOME}" ]; then
          if [ "$HOME" = '/' ]; then
            printf '%s' "$PWD"
          else
            case $PWD in
              ${HOME}*) .sh.value=$(printf '~%s' "${POLYGLOT_PWD_MINUS_HOME}") ;;
              *) .sh.value="$(printf '%s' "$PWD")" ;;
            esac
          fi
        # Otherwise include an ellipsis to show that abbreviation has taken place
        else
          if [ "$HOME" = '/' ]; then
            .sh.value="$(printf '...%s' "$POLYGLOT_ABBREVIATED_PATH")"
          else
            case $PWD in
              ${HOME}*) .sh.value="$(printf '~/...%s' "$POLYGLOT_ABBREVIATED_PATH")" ;;
              *) .sh.value="$(printf '...%s' "$POLYGLOT_ABBREVIATED_PATH")" ;;
            esac
          fi
        fi
      fi
}

Results… it hasn’t crashed on 10.15.6, it hasn’t crashed on 10.14.6.

My self-imposed carelessness last night with copying around versions of the source tree did not lead to verifiable testing (hence the now-deleted report of a worse crash). That was entirely on me and I apologize for the noise.

I ran it all day on 10.15.6 and it didn’t crash, either line editing or trying to force the read timeout segfault.

Amusingly, I put it on 10.14.6 and the first thing I got was the background job hang. 😃. Nothing else to report so far.

set -o notify is NOT broken with this one, as outlined by @JohnoKing in https://github.com/ksh93/ksh/commit/e805c7d9b15dab373d23749f6a2c777e8cd5126a#commitcomment-41290508.

I have not been able to crash it with this patch. Has anyone else? This is like one of those things where you visit the doctor and say “it hurts when I do this…” and the only thing the doctor can say is “well, don’t do that?”

I can confirm that with this patch, I no longer get the crash with either my complex PS1 or the minimum reproducer.