Compound array regression on OpenBSD with standard malloc

[McDutchie]

When compiling with -D_std_malloc on OpenBSD, the following regression test fails:

    arrays.sh[605]: setting element 1 of array to compound variable failed

The OpenBSD system malloc is very robust and hardened, so this is almost certainly not a problem with that, but a bug in ksh exposed by OpenBSD's hardening. It should be fixed in ksh.

When we fix the regression test to show expected and actual values:

--- a/src/cmd/ksh93/tests/arrays.sh
+++ b/src/cmd/ksh93/tests/arrays.sh
@@ -605,7 +605,8 @@ x=$(
    foo[1]=(x=3)
    typeset -p foo
 ) 2> /dev/null
-[[ $x == "$exp" ]] || err_exit 'setting element 1 of array to compound variable failed'
+[[ $x == "$exp" ]] || err_exit 'setting element 1 of array to compound variable failed' \
+   "(expected $(printf %q "$exp"), got $(printf %q "$x"))"

 #test for cloning a very large index array - can core dump
 (  

then the failure shows as:

    arrays.sh[605]: setting element 1 of array to compound variable failed (expected 'typeset -a foo=((11 22) (x=3))', got $'typeset -A foo=([0]=\'\' [1]=(x=3))')

So it seems the wrong type attribute is somehow assigned to the variable (associative instead of indexed array).

This failure does not occur when compiling with vmalloc.

[hyenias]

Update, I am researching this.

[hyenias]

I booted up my OpenBSD 7.0 box and found that the arrays regression test no longer fails with current head at 4e6d1433a74eaee2bd8fe4df7fada404b81f26a6. I ran the arrays regression test many times without any errors. Only one set of builtins.sh regression tests are failing at the moment:

$ bin/shtests arrays
#### Regression-testing /mnt/tmp/ksh/arch/openbsd.amd64-64/bin/ksh ####
test arrays begins at 2021-12-18+21:18:06
test arrays passed at 2021-12-18+21:18:08 [ 157 tests 0 errors ]
test arrays(C.UTF-8) begins at 2021-12-18+21:18:08
test arrays(C.UTF-8) passed at 2021-12-18+21:18:11 [ 157 tests 0 errors ]
test arrays(shcomp) begins at 2021-12-18+21:18:11
test arrays(shcomp) passed at 2021-12-18+21:18:13 [ 157 tests 0 errors ]
Total errors: 0
$ bin/shtests builtins
#### Regression-testing /mnt/tmp/ksh/arch/openbsd.amd64-64/bin/ksh ####
test builtins begins at 2021-12-18+21:18:17
/mnt/tmp/ksh/src/cmd/ksh93/tests/builtins.sh[1369]: cd: .: [No such file or directory]
    builtins.sh[1371]: cd -P without -e exits with error status if $PWD doesn't exist (expected 0, got 1)
/mnt/tmp/ksh/src/cmd/ksh93/tests/builtins.sh[1372]: cd: .: [No such file or directory]
    builtins.sh[1374]: cd -eP doesn't fail if $PWD doesn't exist (expected 1, got 2)
test builtins failed at 2021-12-18+21:18:26 with exit code 2 [ 263 tests 2 errors ]
test builtins(C.UTF-8) begins at 2021-12-18+21:18:26
/mnt/tmp/ksh/src/cmd/ksh93/tests/builtins.sh[1369]: cd: .: [No such file or directory]
    builtins.sh[1371]: cd -P without -e exits with error status if $PWD doesn't exist (expected 0, got 1)
/mnt/tmp/ksh/src/cmd/ksh93/tests/builtins.sh[1372]: cd: .: [No such file or directory]
    builtins.sh[1374]: cd -eP doesn't fail if $PWD doesn't exist (expected 1, got 2)
test builtins(C.UTF-8) failed at 2021-12-18+21:18:36 with exit code 2 [ 263 tests 2 errors ]
test builtins(shcomp) begins at 2021-12-18+21:18:36
/tmp/ksh93.shtests.31413.6191/shcomp-builtins.ksh[1409]: cd: .: [No such file or directory]
    shcomp-builtins.ksh[1411]: cd -P without -e exits with error status if $PWD doesn't exist (expected 0, got 1)
/tmp/ksh93.shtests.31413.6191/shcomp-builtins.ksh[1412]: cd: .: [No such file or directory]
    shcomp-builtins.ksh[1414]: cd -eP doesn't fail if $PWD doesn't exist (expected 1, got 2)
test builtins(shcomp) failed at 2021-12-18+21:18:46 with exit code 2 [ 263 tests 2 errors ]
Total errors: 6

Testing by hand also works.

$ arch/*/bin/ksh
openbsd:/mnt/tmp/ksh$ exp='typeset -a foo=((11 22) (x=3))'
openbsd:/mnt/tmp/ksh$ typeset -a foo=( ( 11 22 ) ( 44 55 ) )
openbsd:/mnt/tmp/ksh$ typeset -p foo
typeset -a foo=((11 22) (44 55) )
openbsd:/mnt/tmp/ksh$ foo[1]=(x=3)
openbsd:/mnt/tmp/ksh$ typeset -p foo
typeset -a foo=((11 22) (x=3))
openbsd:/mnt/tmp/ksh$

[McDutchie]

Interesting, thanks. I'll have to install an OpenBSD 7.0 VM to do my testing as arrays.sh still fails for me as before on OpenBSD 6.2 and OpenBSD 6.8. If it does work correctly on the latest OpenBSD then we can call it resolved.

We'll have to figure out in a separate issue what the problem with cd -Pe on OpenBSD is.

[JohnoKing]

I think that the regression test for cd -Pe is flawed, as it also fails the same way on illumos. The patch below fixes the test failure on illumos. Could anyone confirm if it fixes the test failure on OpenBSD as well?

diff --git a/src/cmd/ksh93/tests/builtins.sh b/src/cmd/ksh93/tests/builtins.sh
index 7d01e73e..38406b54 100755
--- a/src/cmd/ksh93/tests/builtins.sh
+++ b/src/cmd/ksh93/tests/builtins.sh
@@ -1363,9 +1363,10 @@ fi

 # ======
 # These are regression tests for the cd command's -e and -P flags
-mkdir -p "$tmp/failpwd1"
+mkdir -p "$tmp/failpwd"
+ln -s "$tmp/failpwd" "$tmp/failpwd1"
 cd "$tmp/failpwd1"
-rmdir ../failpwd1
+rm ../failpwd1
 cd -P .
 got=$?; exp=0
 (( got == exp )) || err_exit "cd -P without -e exits with error status if \$PWD doesn't exist (expected $exp, got $got)"

[hyenias]

With @JohnoKing's builtins.sh patch above, all tests now pass on my OpenBSD 7.0 metal box. As with all the changes to the build system since this issue was opened and just to be clear in case I did it wrong, I compiled ksh93u+m/1.1.0-alpha+4e6d1433 2021-12-17 using bin/package make CCFLAGS='-D_std_malloc' then ran bin/shtests various ways.

$ bin/shtests arrays builtins
#### Regression-testing /mnt/tmp/ksh/arch/openbsd.amd64-64/bin/ksh ####
test arrays begins at 2021-12-19+10:02:50
test arrays passed at 2021-12-19+10:02:52 [ 157 tests 0 errors ]
test arrays(C.UTF-8) begins at 2021-12-19+10:02:53
test arrays(C.UTF-8) passed at 2021-12-19+10:02:55 [ 157 tests 0 errors ]
test arrays(shcomp) begins at 2021-12-19+10:02:55
test arrays(shcomp) passed at 2021-12-19+10:02:58 [ 157 tests 0 errors ]
test builtins begins at 2021-12-19+10:02:58
test builtins passed at 2021-12-19+10:03:08 [ 263 tests 0 errors ]
test builtins(C.UTF-8) begins at 2021-12-19+10:03:08
test builtins(C.UTF-8) passed at 2021-12-19+10:03:18 [ 263 tests 0 errors ]
test builtins(shcomp) begins at 2021-12-19+10:03:18
test builtins(shcomp) passed at 2021-12-19+10:03:27 [ 263 tests 0 errors ]
Total errors: 0

[McDutchie]

Great news. :) Looks like those failures weren't a problem on our end, then.

Note that, as of f9364b17098ca05dc6ab886d8906ee55bfb2126c, you no longer need _std_malloc. Instead, if you want vmalloc, you now need _AST_vmalloc.

[hyenias]

It does feel so good to see ksh now run without any test errors on OpenBSD 7.0 x86_64. Can't wait to see a new entry in src/cmd/ksh93/README for it. I will work on getting an ARM version of OpenBSD 7.0 running.

[hyenias]

After successfully installing and setting up my little ARM box, I was unable to install any packages for OpenBSD 7.0 aarch64. Not sure why but since I did not do anything different to install very basic packages as compared to the amd64 box I assume OpenBSD 7.0 aarch64 is not ready prime time yet. :(

[JohnoKing]

The regression test in question does work on OpenBSD 7.0, but it still has a remaining issue. It fails with a heap-use-after-free error when run under ASan (this bug is likely what causes it to fail on older versions of OpenBSD):

$ cat /tmp/bar
alias err_exit=echo
exp='typeset -a foo=((11 22) (x=3))'
x=$(
    typeset -a foo=( ( 11 22 ) ( 44 55 ) )
    foo[1]=(x=3)
    typeset -p foo
) 2> /dev/null
[[ $x == "$exp" ]] || err_exit 'setting element 1 of array to compound variable failed'

$ ASAN_OPTIONS='detect_leaks=0' arch/linux.i386-64/bin/ksh /tmp/bar
=================================================================
==23433==ERROR: AddressSanitizer: heap-use-after-free on address 0x614000000a78 at pc 0x558f8c0d3368 bp 0x7ffeee4748e0 sp 0x7ffeee4748d0
READ of size 8 at 0x614000000a78 thread T0
    #0 0x558f8c0d3367 in nv_setlist /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/name.c:524
    #1 0x558f8c131e6a in sh_exec /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/xec.c:1122
    #2 0x558f8c13bd1d in sh_exec /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/xec.c:2059
    #3 0x558f8c122518 in sh_subshell /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/subshell.c:664
    #4 0x558f8c0ca550 in comsubst /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/macro.c:2188
    #5 0x558f8c0bf6b8 in varsub /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/macro.c:1167
    #6 0x558f8c0bac7b in copyto /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/macro.c:626
    #7 0x558f8c0b5962 in sh_mactrim /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/macro.c:176
    #8 0x558f8c0d0edb in nv_setlist /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/name.c:307
    #9 0x558f8c131e6a in sh_exec /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/xec.c:1122
    #10 0x558f8c029604 in exfile /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/main.c:600
    #11 0x558f8c026dda in sh_main /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/main.c:361
    #12 0x558f8c02504d in main /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/pmain.c:46
    #13 0x7f57b52dbb24 in __libc_start_main (/usr/lib/libc.so.6+0x27b24)
    #14 0x558f8c024f5d in _start (/home/johno/GitRepos/KornShell/ksh/arch/linux.i386-64/bin/ksh+0x5cf5d)

0x614000000a78 is located 56 bytes inside of 400-byte region [0x614000000a40,0x614000000bd0)
freed by thread T0 here:
    #0 0x7f57b5693f19 in __interceptor_free /build/gcc/src/gcc/libsanitizer/asan/asan_malloc_linux.cpp:127
    #1 0x558f8c0514c4 in array_putval /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/array.c:768
    #2 0x558f8c02b7d0 in nv_putv /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/nvdisc.c:147
    #3 0x558f8c0dc427 in nv_putval /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/name.c:1633
    #4 0x558f8c0504c9 in array_putval /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/array.c:631
    #5 0x558f8c02b7d0 in nv_putv /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/nvdisc.c:147
    #6 0x558f8c0e2c0a in _nv_unset /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/name.c:2562
    #7 0x558f8c0d3331 in nv_setlist /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/name.c:523
    #8 0x558f8c131e6a in sh_exec /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/xec.c:1122
    #9 0x558f8c13bd1d in sh_exec /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/xec.c:2059
    #10 0x558f8c122518 in sh_subshell /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/subshell.c:664
    #11 0x558f8c0ca550 in comsubst /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/macro.c:2188
    #12 0x558f8c0bf6b8 in varsub /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/macro.c:1167
    #13 0x558f8c0bac7b in copyto /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/macro.c:626
    #14 0x558f8c0b5962 in sh_mactrim /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/macro.c:176
    #15 0x558f8c0d0edb in nv_setlist /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/name.c:307
    #16 0x558f8c131e6a in sh_exec /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/xec.c:1122
    #17 0x558f8c029604 in exfile /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/main.c:600
    #18 0x558f8c026dda in sh_main /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/main.c:361
    #19 0x558f8c02504d in main /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/pmain.c:46
    #20 0x7f57b52dbb24 in __libc_start_main (/usr/lib/libc.so.6+0x27b24)

previously allocated by thread T0 here:
    #0 0x7f57b5694279 in __interceptor_malloc /build/gcc/src/gcc/libsanitizer/asan/asan_malloc_linux.cpp:145
    #1 0x558f8c07293c in sh_malloc /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/init.c:233
    #2 0x558f8c0519d6 in array_grow /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/array.c:829
    #3 0x558f8c05496a in nv_putsub /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/array.c:1210
    #4 0x558f8c05a35e in nv_setvec /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/array.c:1851
    #5 0x558f8c0d26cc in nv_setlist /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/name.c:439
    #6 0x558f8c0d2e8d in nv_setlist /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/name.c:486
    #7 0x558f8c131e6a in sh_exec /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/xec.c:1122
    #8 0x558f8c13bd1d in sh_exec /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/xec.c:2059
    #9 0x558f8c122518 in sh_subshell /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/subshell.c:664
    #10 0x558f8c0ca550 in comsubst /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/macro.c:2188
    #11 0x558f8c0bf6b8 in varsub /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/macro.c:1167
    #12 0x558f8c0bac7b in copyto /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/macro.c:626
    #13 0x558f8c0b5962 in sh_mactrim /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/macro.c:176
    #14 0x558f8c0d0edb in nv_setlist /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/name.c:307
    #15 0x558f8c131e6a in sh_exec /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/xec.c:1122
    #16 0x558f8c029604 in exfile /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/main.c:600
    #17 0x558f8c026dda in sh_main /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/main.c:361
    #18 0x558f8c02504d in main /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/pmain.c:46
    #19 0x7f57b52dbb24 in __libc_start_main (/usr/lib/libc.so.6+0x27b24)

SUMMARY: AddressSanitizer: heap-use-after-free /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/name.c:524 in nv_setlist
Shadow bytes around the buggy address:
  0x0c287fff80f0: 00 00 00 00 00 00 00 00 00 00 fa fa fa fa fa fa
  0x0c287fff8100: fa fa fa fa fa fa fa fa 00 00 00 00 00 00 00 00
  0x0c287fff8110: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c287fff8120: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c287fff8130: 00 00 00 00 00 00 00 00 00 00 fa fa fa fa fa fa
=>0x0c287fff8140: fa fa fa fa fa fa fa fa fd fd fd fd fd fd fd[fd]
  0x0c287fff8150: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c287fff8160: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c287fff8170: fd fd fd fd fd fd fd fd fd fd fa fa fa fa fa fa
  0x0c287fff8180: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c287fff8190: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07 
  Heap left redzone:       fa
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
  Shadow gap:              cc
==23433==ABORTING

[JohnoKing]

I had forgotten about this issue when I submitted https://github.com/ksh93/ksh/pull/411, but I've found that the bugfix I backported from ksh2020 fixes the ASan failure in the above comment. @McDutchie does the regression test still fail on OpenBSD 6.2/6.8? (edit: The bugfix was reverted in 06462aef02d232506907e9b17d9b8b09eb87c58c. Is this accidental?)

[McDutchie]

(edit: The bugfix was reverted in 06462ae. Is this accidental?)

Whoops. Yes, absolutely, sorry about that. I'll have to do a force-push to remove the history pollution – you'll want to do a git reset --hard 38999a64c before the next git pull. (edit: Done.)

[McDutchie]

And yes, I'm happy to report that, after your fix, the arrays.sh tests now passes on older OpenBSD versions. :-)