From 244b96bf459e6d3dd4a1e7d2f47d872deaa78d5c Mon Sep 17 00:00:00 2001 From: Martijn Dekker Date: Sun, 10 Mar 2024 23:34:27 +0000 Subject: [PATCH] Fix crash on redefining array in subshell (re: 92f65cb8) The referenced commit left one test unexecuted because it crashes. Minimal reproducer: typeset -a arr=((a b c) 1) got=$( typeset -a arr=( ( ((a b c)1))) ) The crash occurs when the array is redefined in a subshell. Here are abridged ASan stack traces for the crash, for the use after free, and for when it was freed: ================================================================= ==73147==ERROR: AddressSanitizer: heap-use-after-free [snippage] READ of size 8 at 0x000107403eb0 thread T0 #0 0x104fded40 in nv_search nvdisc.c:1007 #1 0x104fbeb1c in nv_create name.c:860 #2 0x104fb8b9c in nv_open name.c:1440 #3 0x104fb1edc in nv_setlist name.c:309 #4 0x104fb4a30 in nv_setlist name.c:475 #5 0x105055b58 in sh_exec xec.c:1079 #6 0x105045cd4 in sh_subshell subshell.c:654 #7 0x104f92c1c in comsubst macro.c:2266 [snippage] 0x000107403eb0 is located 0 bytes inside of 80-byte region [snippage] freed by thread T0 here: #0 0x105c5ade4 in wrap_free+0x98 (libclang_rt.asan_osx_dynamic.dylib:arm64e+0x3ede4) #1 0x105261da0 in dtclose dtclose.c:52 #2 0x104f178cc in array_putval array.c:671 #3 0x104fd7f4c in nv_putv nvdisc.c:144 #4 0x104fbc5f0 in _nv_unset name.c:2435 #5 0x104fb3250 in nv_setlist name.c:364 #6 0x105055b58 in sh_exec xec.c:1079 #7 0x105045cd4 in sh_subshell subshell.c:654 #8 0x104f92c1c in comsubst macro.c:2266 [snippage] So the crash is caused because array_putval (array.c:671) calls dtclose, freeing ap->table, which is then reused after a recursive nv_setlist call via nv_open() -> nv_create() -> nv_search(). This only happens whwn we're in a virtual subshell. src/cmd/ksh93/sh/array.c: - array_putval(): When redefining an array in a virtual subshell, do not free the old ap->table; it will be needed by the parent shell environment. --- NEWS | 3 +++ src/cmd/ksh93/sh/array.c | 2 +- src/cmd/ksh93/tests/basic.sh | 12 ++++-------- 3 files changed, 8 insertions(+), 9 deletions(-) diff --git a/NEWS b/NEWS index eeaee1ea9d85..1883a449463b 100644 --- a/NEWS +++ b/NEWS @@ -7,6 +7,9 @@ Uppercase BUG_* IDs are shell bug IDs as used by the Modernish shell library. - Fixed a longstanding bug where the default terminal width for typeset -L, -R, or -Z, if not given, was miscalculated for multibyte or control characters. +- Fixed a crash on redefining an array inherited from the parent environment + in a subshell. + 2024-03-07: - Fixed a bug that caused some systems to corrupt the display of multibyte diff --git a/src/cmd/ksh93/sh/array.c b/src/cmd/ksh93/sh/array.c index 156db4994c1e..e7022075e752 100644 --- a/src/cmd/ksh93/sh/array.c +++ b/src/cmd/ksh93/sh/array.c @@ -665,7 +665,7 @@ static void array_putval(Namval_t *np, const char *string, int flags, Namfun_t * { if(is_associative(ap)) (*ap->fun)(np, NULL, NV_AFREE); - else if(ap->table) + else if(ap->table && (!sh.subshell || sh.subshare)) { dtclose(ap->table); ap->table = 0; diff --git a/src/cmd/ksh93/tests/basic.sh b/src/cmd/ksh93/tests/basic.sh index ca635bd5291e..130a8a8663c3 100755 --- a/src/cmd/ksh93/tests/basic.sh +++ b/src/cmd/ksh93/tests/basic.sh @@ -2,7 +2,7 @@ # # # This software is part of the ast package # # Copyright (c) 1982-2012 AT&T Intellectual Property # -# Copyright (c) 2020-2023 Contributors to ksh 93u+m # +# Copyright (c) 2020-2024 Contributors to ksh 93u+m # # and is licensed under the # # Eclipse Public License, Version 2.0 # # # @@ -1003,7 +1003,6 @@ done # ====== # Nested compound assignment misparsed in $(...) or ${ ...; } command substitution # https://github.com/ksh93/ksh/issues/269 -# TODO: one of the tests below crashes when actually executed; test lexing only by using noexec. for testcode in \ ': $( typeset -a arr=((a b c) 1) )' \ ': ${ typeset -a arr=((a b c) 1); }' \ @@ -1018,14 +1017,11 @@ for testcode in \ 'typeset -Ca arr=((a=ah b=beh c=si))' \ ': $( typeset -Ca arr=((a=ah b=beh c=si)) )' \ 'r=${ typeset -Ca arr=((a=ah b=beh c=si)); }' \ - 'set --noexec; : $( typeset -a arr=((a $(( $( typeset -a barr=((a $(( 1 << 2 )) c) 1); echo 1 ) << $( typeset -a bazz=((a $(( 1 << 2 )) c) 1); echo 2 ) )) c) 1) )' \ + ': $( typeset -a arr=((a $(( $( typeset -a barr=((a $(( 1 << 2 )) c) 1); echo 1 ) << $( typeset -a bazz=((a $(( 1 << 2 )) c) 1); echo 2 ) )) c) 1) )' \ 'r=$(typeset -C arr=( (a=ah b=beh c=si) 1 (e f g)));' do - # fork comsub with 'ulimit' on old ksh to avoid a fixed lexer bug crashing the entire test script - got=$(let ".sh.version >= 20211209" || ulimit -c 0 - eval "set +x; $testcode" 2>&1) \ - || err_exit "comsub/arithexp lexing test $(printf %q "$testcode"):" \ - "got status $? and $(printf %q "$got")" + got=$(export testcode; "$SHELL" -c 'v=$(eval "$testcode" 2>&1); e=$?; print -r -- "$v"; exit $e' 2>&1) \ + || { e=$?; err_exit "comsub/arithexp lexing test $(printf %q "$testcode"): got status $e and $(printf %q "$got")"; } done unset testcode