Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix a crash when running 'unalias r' #133

Merged
merged 4 commits into from
Sep 18, 2020
Merged

Commits on Sep 15, 2020

  1. Fix a memory fault caused by 'unalias r'

    After commit ddaa145, the following set of commands now
    causes ksh to crash:
    
    $ unalias history; unalias r
    Memory fault
    
    When ksh is compiled with -D_std_malloc, the crash always
    occurs when the 'r' alias is removed with 'unalias r',
    although with vmalloc 'unalias history' must be run first
    for the crash to occur. With the native malloc, the crash
    message is also different:
    
    $ unalias history; unalias r
    free(): invalid pointer
    Abort
    
    This crash happens because when an alias is unset, _nv_unset
    removes the NV_NOFREE flag which results in an invalid use
    of free(3) as nv_isattr no longer detects NV_NOFREE afterward.
    The history and r aliases shouldn't be freed from memory by
    nv_delete because those aliases are given the NV_NOFREE attribute.
    
    src/cmd/ksh93/bltins/typeset.c:
    - Save the state of NV_NOFREE for aliases to fix the crash
      caused by 'unalias r'. Also of note, using the return
      value from nv_isattr is incorrect since it's just a
      boolean value. The actual flag passed must be NV_NOFREE
      to prevent bugs (like the crash).
    
    src/cmd/ksh93/tests/alias.sh:
    - Use unalias on both history and r to check for the crash.
      'unalias -a' can't be used to replicate the crash.
    JohnoKing committed Sep 15, 2020
    Configuration menu
    Copy the full SHA
    30482dd View commit details
    Browse the repository at this point in the history

Commits on Sep 18, 2020

  1. More concise fix

    Since aliases don't have virtual subshell tables, no need to
    _nv_unset() them first.
    McDutchie committed Sep 18, 2020
    Configuration menu
    Copy the full SHA
    f6ddacf View commit details
    Browse the repository at this point in the history
  2. Revert "More concise fix"

    Actually, never mind. My version introduces a memory leak.
    Apparently, we must _nv_unset() before calling nv_delete(). Which
    makes sense because nv_delete() only frees the node itself,
    _nv_unset() is what frees the value.
    
    This reverts commit f6ddacf.
    McDutchie committed Sep 18, 2020
    Configuration menu
    Copy the full SHA
    41b750d View commit details
    Browse the repository at this point in the history
  3. Configuration menu
    Copy the full SHA
    db94896 View commit details
    Browse the repository at this point in the history