Backport the ksh2020 fix for timezone name determination #8
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This fix for `printf '%T' now` on FreeBSD was written by @krader1961. This is from att#591: On FreeBSD calling tzset() does not guarantee the tzname array will be correctly populated. On most systems that works but on FreeBSD you have to call localtime() or a related function (e.g., ctime()). This change also eliminates a potential, very small, memory leak due to the strdup()'ed tznames not being freed. src/lib/libast/tm/tminit.c: - Fix timezone name determination on FreeBSD and a memory leak.
McDutchie
added a commit
that referenced
this pull request
Jun 13, 2022
I didn't trust this back in e3d7bf1 (which disabled it for interactive shells) and I trust it less now. In af6a32d/6b380572, this was also disabled for virtual subshells as it caused program flow corruption there. Now, on macOS 10.14.6, a crash occurs when repeatedly running a command with this optimisation: $ ksh -c 'for((i=0;i<100;i++));do print -n "$i ";(sleep 1&);done' 0 1 2 3 4 5 6 7 Illegal instruction Oddly enough it seems that I can only reproduce this crash on macOS -- not on Linux, OpenBSD, or Solaris. It could be a macOS bug, particularly given the odd message in the stack trace below. I've had enough, though. Out it comes. Things now work fine, the reproducer is fixed on macOS, and it didn't optimise much anyway. The double-fork issue discussed in e3d7bf1 remains. ________ For future reference, here's an lldb debugger session with a stack trace. It crashes on calling calloc() (via sh_calloc(), via sh_newof()) in jobsave_create(). This is not an invalid pointer problem as we're allocating new memory, so it does look like an OS bug. The "BUG IN CLIENT OF LIBPLATFORM" message is interesting. $ lldb -- arch/*/bin/ksh -c 'for((i=0;i<100;i++));do print -n "$i ";(sleep 1&);done' (lldb) target create "arch/darwin.i386-64/bin/ksh" Current executable set to 'arch/darwin.i386-64/bin/ksh' (x86_64). (lldb) settings set -- target.run-args "-c" "for((i=0;i<100;i++));do print -n \"$i \";(sleep 1&);done" (lldb) run error: shell expansion failed (reason: lldb-argdumper exited with error 2). consider launching with 'process launch'. (lldb) process launch Process 35038 launched: '/usr/local/src/ksh93/ksh/arch/darwin.i386-64/bin/ksh' (x86_64) 0 1 2 3 4 5 6 7 8 9 Process 35038 stopped * thread #1, queue = 'com.apple.main-thread', stop reason = EXC_BAD_INSTRUCTION (code=EXC_I386_INVOP, subcode=0x0) frame #0: 0x00007fff70deb1c2 libsystem_platform.dylib`_os_unfair_lock_recursive_abort + 23 libsystem_platform.dylib`_os_unfair_lock_recursive_abort: -> 0x7fff70deb1c2 <+23>: ud2 libsystem_platform.dylib`_os_unfair_lock_unowned_abort: 0x7fff70deb1c4 <+0>: movl %edi, %eax 0x7fff70deb1c6 <+2>: leaq 0x1a8a(%rip), %rcx ; "BUG IN CLIENT OF LIBPLATFORM: Unlock of an os_unfair_lock not owned by current thread" 0x7fff70deb1cd <+9>: movq %rcx, 0x361cb16c(%rip) ; gCRAnnotations + 8 Target 0: (ksh) stopped. (lldb) bt * thread #1, queue = 'com.apple.main-thread', stop reason = EXC_BAD_INSTRUCTION (code=EXC_I386_INVOP, subcode=0x0) * frame #0: 0x00007fff70deb1c2 libsystem_platform.dylib`_os_unfair_lock_recursive_abort + 23 frame #1: 0x00007fff70de7c9a libsystem_platform.dylib`_os_unfair_lock_lock_slow + 239 frame #2: 0x00007fff70daa3bd libsystem_malloc.dylib`tiny_malloc_should_clear + 188 frame #3: 0x00007fff70daa20f libsystem_malloc.dylib`szone_malloc_should_clear + 66 frame #4: 0x00007fff70dab444 libsystem_malloc.dylib`malloc_zone_calloc + 99 frame #5: 0x00007fff70dab3c4 libsystem_malloc.dylib`calloc + 30 frame #6: 0x000000010003fa5d ksh`sh_calloc(nmemb=1, size=16) at init.c:264:13 frame #7: 0x000000010004f8a6 ksh`jobsave_create(pid=35055) at jobs.c:272:8 frame #8: 0x000000010004ed42 ksh`job_reap(sig=20) at jobs.c:363:9 frame #9: 0x000000010004ff6f ksh`job_waitsafe(sig=20) at jobs.c:511:3 frame #10: 0x00007fff70de9b5d libsystem_platform.dylib`_sigtramp + 29 frame #11: 0x00007fff70d39ac4 libsystem_kernel.dylib`__fork + 12 frame #12: 0x00007fff70c57d80 libsystem_c.dylib`fork + 17 frame #13: 0x000000010009590d ksh`sh_exec(t=0x0000000101005d30, flags=4) at xec.c:1883:16 frame #14: 0x0000000100096013 ksh`sh_exec(t=0x0000000101005d30, flags=4) at xec.c:2019:4 frame #15: 0x0000000100096c4f ksh`sh_exec(t=0x0000000101005a40, flags=5) at xec.c:2213:9 frame #16: 0x0000000100096013 ksh`sh_exec(t=0x0000000101005a40, flags=5) at xec.c:2019:4 frame #17: 0x000000010001c23f ksh`exfile(iop=0x0000000100405750, fno=-1) at main.c:603:4 frame #18: 0x000000010001b23c ksh`sh_main(ac=3, av=0x00007ffeefbff4f0, userinit=0x0000000000000000) at main.c:365:2 frame #19: 0x0000000100000776 ksh`main(argc=3, argv=0x00007ffeefbff4f0) at pmain.c:45:9 frame #20: 0x00007fff70bfe3d5 libdyld.dylib`start + 1
McDutchie
added a commit
that referenced
this pull request
Jun 13, 2022
I didn't trust this back in e3d7bf1 (which disabled it for interactive shells) and I trust it less now. In af6a32d/6b380572, this was also disabled for virtual subshells as it caused program flow corruption there. Now, on macOS 10.14.6, a crash occurs when repeatedly running a command with this optimisation: $ ksh -c 'for((i=0;i<100;i++));do print -n "$i ";(sleep 1&);done' 0 1 2 3 4 5 6 7 Illegal instruction Oddly enough it seems that I can only reproduce this crash on macOS -- not on Linux, OpenBSD, or Solaris. It could be a macOS bug, particularly given the odd message in the stack trace below. I've had enough, though. Out it comes. Things now work fine, the reproducer is fixed on macOS, and it didn't optimise much anyway. The double-fork issue discussed in e3d7bf1 remains. ________ For future reference, here's an lldb debugger session with a stack trace. It crashes on calling calloc() (via sh_calloc(), via sh_newof()) in jobsave_create(). This is not an invalid pointer problem as we're allocating new memory, so it does look like an OS bug. The "BUG IN CLIENT OF LIBPLATFORM" message is interesting. $ lldb -- arch/*/bin/ksh -c 'for((i=0;i<100;i++));do print -n "$i ";(sleep 1&);done' (lldb) target create "arch/darwin.i386-64/bin/ksh" Current executable set to 'arch/darwin.i386-64/bin/ksh' (x86_64). (lldb) settings set -- target.run-args "-c" "for((i=0;i<100;i++));do print -n \"$i \";(sleep 1&);done" (lldb) run error: shell expansion failed (reason: lldb-argdumper exited with error 2). consider launching with 'process launch'. (lldb) process launch Process 35038 launched: '/usr/local/src/ksh93/ksh/arch/darwin.i386-64/bin/ksh' (x86_64) 0 1 2 3 4 5 6 7 8 9 Process 35038 stopped * thread #1, queue = 'com.apple.main-thread', stop reason = EXC_BAD_INSTRUCTION (code=EXC_I386_INVOP, subcode=0x0) frame #0: 0x00007fff70deb1c2 libsystem_platform.dylib`_os_unfair_lock_recursive_abort + 23 libsystem_platform.dylib`_os_unfair_lock_recursive_abort: -> 0x7fff70deb1c2 <+23>: ud2 libsystem_platform.dylib`_os_unfair_lock_unowned_abort: 0x7fff70deb1c4 <+0>: movl %edi, %eax 0x7fff70deb1c6 <+2>: leaq 0x1a8a(%rip), %rcx ; "BUG IN CLIENT OF LIBPLATFORM: Unlock of an os_unfair_lock not owned by current thread" 0x7fff70deb1cd <+9>: movq %rcx, 0x361cb16c(%rip) ; gCRAnnotations + 8 Target 0: (ksh) stopped. (lldb) bt * thread #1, queue = 'com.apple.main-thread', stop reason = EXC_BAD_INSTRUCTION (code=EXC_I386_INVOP, subcode=0x0) * frame #0: 0x00007fff70deb1c2 libsystem_platform.dylib`_os_unfair_lock_recursive_abort + 23 frame #1: 0x00007fff70de7c9a libsystem_platform.dylib`_os_unfair_lock_lock_slow + 239 frame #2: 0x00007fff70daa3bd libsystem_malloc.dylib`tiny_malloc_should_clear + 188 frame #3: 0x00007fff70daa20f libsystem_malloc.dylib`szone_malloc_should_clear + 66 frame #4: 0x00007fff70dab444 libsystem_malloc.dylib`malloc_zone_calloc + 99 frame #5: 0x00007fff70dab3c4 libsystem_malloc.dylib`calloc + 30 frame #6: 0x000000010003fa5d ksh`sh_calloc(nmemb=1, size=16) at init.c:264:13 frame #7: 0x000000010004f8a6 ksh`jobsave_create(pid=35055) at jobs.c:272:8 frame #8: 0x000000010004ed42 ksh`job_reap(sig=20) at jobs.c:363:9 frame #9: 0x000000010004ff6f ksh`job_waitsafe(sig=20) at jobs.c:511:3 frame #10: 0x00007fff70de9b5d libsystem_platform.dylib`_sigtramp + 29 frame #11: 0x00007fff70d39ac4 libsystem_kernel.dylib`__fork + 12 frame #12: 0x00007fff70c57d80 libsystem_c.dylib`fork + 17 frame #13: 0x000000010009590d ksh`sh_exec(t=0x0000000101005d30, flags=4) at xec.c:1883:16 frame #14: 0x0000000100096013 ksh`sh_exec(t=0x0000000101005d30, flags=4) at xec.c:2019:4 frame #15: 0x0000000100096c4f ksh`sh_exec(t=0x0000000101005a40, flags=5) at xec.c:2213:9 frame #16: 0x0000000100096013 ksh`sh_exec(t=0x0000000101005a40, flags=5) at xec.c:2019:4 frame #17: 0x000000010001c23f ksh`exfile(iop=0x0000000100405750, fno=-1) at main.c:603:4 frame #18: 0x000000010001b23c ksh`sh_main(ac=3, av=0x00007ffeefbff4f0, userinit=0x0000000000000000) at main.c:365:2 frame #19: 0x0000000100000776 ksh`main(argc=3, argv=0x00007ffeefbff4f0) at pmain.c:45:9 frame #20: 0x00007fff70bfe3d5 libdyld.dylib`start + 1
McDutchie
added a commit
that referenced
this pull request
Jun 14, 2022
I didn't trust this back in e3d7bf1 (which disabled it for interactive shells) and I trust it less now. In af6a32d/6b380572, this was also disabled for virtual subshells as it caused program flow corruption there. Now, on macOS 10.14.6, a crash occurs when repeatedly running a command with this optimisation: $ ksh -c 'for((i=0;i<100;i++));do print -n "$i ";(sleep 1&);done' 0 1 2 3 4 5 6 7 Illegal instruction Oddly enough it seems that I can only reproduce this crash on macOS -- not on Linux, OpenBSD, or Solaris. It could be a macOS bug, particularly given the odd message in the stack trace below. I've had enough, though. Out it comes. Things now work fine, the reproducer is fixed on macOS, and it didn't optimise much anyway. The double-fork issue discussed in e3d7bf1 remains. ________ For future reference, here's an lldb debugger session with a stack trace. It crashes on calling calloc() (via sh_calloc(), via sh_newof()) in jobsave_create(). This is not an invalid pointer problem as we're allocating new memory, so it does look like an OS bug. The "BUG IN CLIENT OF LIBPLATFORM" message is interesting. $ lldb -- arch/*/bin/ksh -c 'for((i=0;i<100;i++));do print -n "$i ";(sleep 1&);done' (lldb) target create "arch/darwin.i386-64/bin/ksh" Current executable set to 'arch/darwin.i386-64/bin/ksh' (x86_64). (lldb) settings set -- target.run-args "-c" "for((i=0;i<100;i++));do print -n \"$i \";(sleep 1&);done" (lldb) run error: shell expansion failed (reason: lldb-argdumper exited with error 2). consider launching with 'process launch'. (lldb) process launch Process 35038 launched: '/usr/local/src/ksh93/ksh/arch/darwin.i386-64/bin/ksh' (x86_64) 0 1 2 3 4 5 6 7 8 9 Process 35038 stopped * thread #1, queue = 'com.apple.main-thread', stop reason = EXC_BAD_INSTRUCTION (code=EXC_I386_INVOP, subcode=0x0) frame #0: 0x00007fff70deb1c2 libsystem_platform.dylib`_os_unfair_lock_recursive_abort + 23 libsystem_platform.dylib`_os_unfair_lock_recursive_abort: -> 0x7fff70deb1c2 <+23>: ud2 libsystem_platform.dylib`_os_unfair_lock_unowned_abort: 0x7fff70deb1c4 <+0>: movl %edi, %eax 0x7fff70deb1c6 <+2>: leaq 0x1a8a(%rip), %rcx ; "BUG IN CLIENT OF LIBPLATFORM: Unlock of an os_unfair_lock not owned by current thread" 0x7fff70deb1cd <+9>: movq %rcx, 0x361cb16c(%rip) ; gCRAnnotations + 8 Target 0: (ksh) stopped. (lldb) bt * thread #1, queue = 'com.apple.main-thread', stop reason = EXC_BAD_INSTRUCTION (code=EXC_I386_INVOP, subcode=0x0) * frame #0: 0x00007fff70deb1c2 libsystem_platform.dylib`_os_unfair_lock_recursive_abort + 23 frame #1: 0x00007fff70de7c9a libsystem_platform.dylib`_os_unfair_lock_lock_slow + 239 frame #2: 0x00007fff70daa3bd libsystem_malloc.dylib`tiny_malloc_should_clear + 188 frame #3: 0x00007fff70daa20f libsystem_malloc.dylib`szone_malloc_should_clear + 66 frame #4: 0x00007fff70dab444 libsystem_malloc.dylib`malloc_zone_calloc + 99 frame #5: 0x00007fff70dab3c4 libsystem_malloc.dylib`calloc + 30 frame #6: 0x000000010003fa5d ksh`sh_calloc(nmemb=1, size=16) at init.c:264:13 frame #7: 0x000000010004f8a6 ksh`jobsave_create(pid=35055) at jobs.c:272:8 frame #8: 0x000000010004ed42 ksh`job_reap(sig=20) at jobs.c:363:9 frame #9: 0x000000010004ff6f ksh`job_waitsafe(sig=20) at jobs.c:511:3 frame #10: 0x00007fff70de9b5d libsystem_platform.dylib`_sigtramp + 29 frame #11: 0x00007fff70d39ac4 libsystem_kernel.dylib`__fork + 12 frame #12: 0x00007fff70c57d80 libsystem_c.dylib`fork + 17 frame #13: 0x000000010009590d ksh`sh_exec(t=0x0000000101005d30, flags=4) at xec.c:1883:16 frame #14: 0x0000000100096013 ksh`sh_exec(t=0x0000000101005d30, flags=4) at xec.c:2019:4 frame #15: 0x0000000100096c4f ksh`sh_exec(t=0x0000000101005a40, flags=5) at xec.c:2213:9 frame #16: 0x0000000100096013 ksh`sh_exec(t=0x0000000101005a40, flags=5) at xec.c:2019:4 frame #17: 0x000000010001c23f ksh`exfile(iop=0x0000000100405750, fno=-1) at main.c:603:4 frame #18: 0x000000010001b23c ksh`sh_main(ac=3, av=0x00007ffeefbff4f0, userinit=0x0000000000000000) at main.c:365:2 frame #19: 0x0000000100000776 ksh`main(argc=3, argv=0x00007ffeefbff4f0) at pmain.c:45:9 frame #20: 0x00007fff70bfe3d5 libdyld.dylib`start + 1
JohnoKing
added a commit
to JohnoKing/ksh
that referenced
this pull request
Aug 19, 2022
The ASan crash in basic.sh when sourcing multiple files is caused by a bug that is similar to the crash fixed in 59a5672. This is the trace for the regression test crash (note that in order to see the trace, the 2>/dev/null redirect must be disabled): ==1899388==ERROR: AddressSanitizer: heap-use-after-free on address 0x6150000005b0 at pc 0x55a5e3f9432a bp 0x7ffeb91ea110 sp 0x7ffeb91ea100 WRITE of size 8 at 0x6150000005b0 thread T0 #0 0x55a5e3f94329 in funct /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/parse.c:967 ksh93#1 0x55a5e3f96f77 in item /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/parse.c:1349 ksh93#2 0x55a5e3f90c9f in term /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/parse.c:642 ksh93#3 0x55a5e3f90ac1 in list /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/parse.c:613 ksh93#4 0x55a5e3f90845 in sh_cmd /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/parse.c:561 ksh93#5 0x55a5e3f909e0 in sh_cmd /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/parse.c:586 ksh93#6 0x55a5e3f8fd5e in sh_parse /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/parse.c:438 ksh93#7 0x55a5e3fc43c1 in sh_eval /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/xec.c:635 ksh93#8 0x55a5e4012172 in b_dot_cmd /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/bltins/misc.c:318 ksh93#9 0x55a5e3fca3cb in sh_exec /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/xec.c:1254 ksh93#10 0x55a5e3fd01d4 in sh_exec /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/xec.c:1932 ksh93#11 0x55a5e3fc4544 in sh_eval /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/xec.c:651 ksh93#12 0x55a5e4012172 in b_dot_cmd /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/bltins/misc.c:318 ksh93#13 0x55a5e3fca3cb in sh_exec /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/xec.c:1254 ksh93#14 0x55a5e3ecc1cd in exfile /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/main.c:604 ksh93#15 0x55a5e3ec9e7f in sh_main /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/main.c:369 ksh93#16 0x55a5e3ec801d in main /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/pmain.c:41 ksh93#17 0x7f637b4db2cf (/usr/lib/libc.so.6+0x232cf) ksh93#18 0x7f637b4db389 in __libc_start_main (/usr/lib/libc.so.6+0x23389) ksh93#19 0x55a5e3ec7f24 in _start ../sysdeps/x86_64/start.S:115 Code in question: https://github.com/ksh93/ksh/blob/8d57369b0cb39074437dd82924b604155e30e1e0/src/cmd/ksh93/sh/parse.c#L963-L968 To avoid any more similar crashes, all of the fixes introduced in 7e317c5 that set slp->slptr to null have been improved with the fix in 59a5672.
McDutchie
pushed a commit
that referenced
this pull request
Aug 19, 2022
The ASan crash in basic.sh when sourcing multiple files is caused by a bug that is similar to the crash fixed in 59a5672. This is the trace for the regression test crash (note that in order to see the trace, the 2>/dev/null redirect must be disabled): ==1899388==ERROR: AddressSanitizer: heap-use-after-free on address 0x6150000005b0 at pc 0x55a5e3f9432a bp 0x7ffeb91ea110 sp 0x7ffeb91ea100 WRITE of size 8 at 0x6150000005b0 thread T0 #0 0x55a5e3f94329 in funct /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/parse.c:967 #1 0x55a5e3f96f77 in item /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/parse.c:1349 #2 0x55a5e3f90c9f in term /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/parse.c:642 #3 0x55a5e3f90ac1 in list /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/parse.c:613 #4 0x55a5e3f90845 in sh_cmd /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/parse.c:561 #5 0x55a5e3f909e0 in sh_cmd /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/parse.c:586 #6 0x55a5e3f8fd5e in sh_parse /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/parse.c:438 #7 0x55a5e3fc43c1 in sh_eval /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/xec.c:635 #8 0x55a5e4012172 in b_dot_cmd /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/bltins/misc.c:318 #9 0x55a5e3fca3cb in sh_exec /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/xec.c:1254 #10 0x55a5e3fd01d4 in sh_exec /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/xec.c:1932 #11 0x55a5e3fc4544 in sh_eval /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/xec.c:651 #12 0x55a5e4012172 in b_dot_cmd /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/bltins/misc.c:318 #13 0x55a5e3fca3cb in sh_exec /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/xec.c:1254 #14 0x55a5e3ecc1cd in exfile /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/main.c:604 #15 0x55a5e3ec9e7f in sh_main /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/main.c:369 #16 0x55a5e3ec801d in main /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/pmain.c:41 #17 0x7f637b4db2cf (/usr/lib/libc.so.6+0x232cf) #18 0x7f637b4db389 in __libc_start_main (/usr/lib/libc.so.6+0x23389) #19 0x55a5e3ec7f24 in _start ../sysdeps/x86_64/start.S:115 Code in question: https://github.com/ksh93/ksh/blob/8d57369b0cb39074437dd82924b604155e30e1e0/src/cmd/ksh93/sh/parse.c#L963-L968 To avoid any more similar crashes, all of the fixes introduced in 7e317c5 that set slp->slptr to null have been improved with the fix in 59a5672.
McDutchie
pushed a commit
that referenced
this pull request
Aug 19, 2022
The ASan crash in basic.sh when sourcing multiple files is caused by a bug that is similar to the crash fixed in f24040e. This is the trace for the regression test crash (note that in order to see the trace, the 2>/dev/null redirect must be disabled): ==1899388==ERROR: AddressSanitizer: heap-use-after-free on address 0x6150000005b0 at pc 0x55a5e3f9432a bp 0x7ffeb91ea110 sp 0x7ffeb91ea100 WRITE of size 8 at 0x6150000005b0 thread T0 #0 0x55a5e3f94329 in funct /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/parse.c:967 #1 0x55a5e3f96f77 in item /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/parse.c:1349 #2 0x55a5e3f90c9f in term /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/parse.c:642 #3 0x55a5e3f90ac1 in list /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/parse.c:613 #4 0x55a5e3f90845 in sh_cmd /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/parse.c:561 #5 0x55a5e3f909e0 in sh_cmd /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/parse.c:586 #6 0x55a5e3f8fd5e in sh_parse /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/parse.c:438 #7 0x55a5e3fc43c1 in sh_eval /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/xec.c:635 #8 0x55a5e4012172 in b_dot_cmd /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/bltins/misc.c:318 #9 0x55a5e3fca3cb in sh_exec /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/xec.c:1254 #10 0x55a5e3fd01d4 in sh_exec /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/xec.c:1932 #11 0x55a5e3fc4544 in sh_eval /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/xec.c:651 #12 0x55a5e4012172 in b_dot_cmd /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/bltins/misc.c:318 #13 0x55a5e3fca3cb in sh_exec /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/xec.c:1254 #14 0x55a5e3ecc1cd in exfile /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/main.c:604 #15 0x55a5e3ec9e7f in sh_main /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/main.c:369 #16 0x55a5e3ec801d in main /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/pmain.c:41 #17 0x7f637b4db2cf (/usr/lib/libc.so.6+0x232cf) #18 0x7f637b4db389 in __libc_start_main (/usr/lib/libc.so.6+0x23389) #19 0x55a5e3ec7f24 in _start ../sysdeps/x86_64/start.S:115 Code in question: https://github.com/ksh93/ksh/blob/8d57369b0cb39074437dd82924b604155e30e1e0/src/cmd/ksh93/sh/parse.c#L963-L968 To avoid any more similar crashes, all of the fixes introduced in 69d37d5 that set slp->slptr to null have been improved with the fix in f24040e.
JohnoKing
added a commit
to JohnoKing/ksh
that referenced
this pull request
Sep 23, 2022
The isaname, isaletter, isadigit, isexp and ismeta macros don't check if
c is a negative value before accessing sh_lexstates. This can result in
ASan crashing because of a buffer overflow in quoting2.sh when running
in a multibyte locale:
test quoting2(C.UTF-8) begins at 2022-09-23+14:03:12
=================================================================
==262224==ERROR: AddressSanitizer: global-buffer-overflow on address 0x557b201a451f at pc 0x557b1fe5e6fc bp 0x7fffcf1ac700 sp 0x7fffcf1ac6f8
READ of size 1 at 0x557b201a451f thread T0
#0 0x557b1fe5e6fb in sh_fmtq /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/string.c:341:5
ksh93#1 0x557b1fe6098c in sh_fmtqf /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/string.c:473:10
ksh93#2 0x557b1ff08dc0 in extend /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/bltins/print.c:998:14
ksh93#3 0x557b2008a56c in sfvprintf /home/johno/GitRepos/KornShell/ksh/src/lib/libast/sfio/sfvprintf.c:531:8
ksh93#4 0x557b2005b7f7 in sfprintf /home/johno/GitRepos/KornShell/ksh/src/lib/libast/sfio/sfprintf.c:31:7
ksh93#5 0x557b1ff04272 in b_print /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/bltins/print.c:343:4
ksh93#6 0x557b1ff04ebf in b_printf /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/bltins/print.c:148:9
ksh93#7 0x557b1fe8d9a7 in sh_exec /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/xec.c:1261:21
ksh93#8 0x557b1fe7a7cf in sh_subshell /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/subshell.c:652:4
ksh93#9 0x557b1fdedc0d in comsubst /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/macro.c:2207:9
ksh93#10 0x557b1fdefc79 in varsub /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/macro.c:1181:3
ksh93#11 0x557b1fde3bef in copyto /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/macro.c:620:21
ksh93#12 0x557b1fde0b07 in sh_mactrim /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/macro.c:169:2
ksh93#13 0x557b1fe05ab6 in nv_setlist /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/name.c:280:9
ksh93#14 0x557b1fe8a7e8 in sh_exec /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/xec.c:1051:7
ksh93#15 0x557b1fe95b85 in sh_exec /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/xec.c:1940:5
ksh93#16 0x557b1fe99ea6 in sh_exec /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/xec.c:2271:10
ksh93#17 0x557b1fd23b04 in exfile /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/main.c:604:4
ksh93#18 0x557b1fd1fe10 in sh_main /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/main.c:369:2
ksh93#19 0x557b1fd1d585 in main /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/pmain.c:41:9
ksh93#20 0x7f55d5b5028f (/usr/lib/libc.so.6+0x2328f) (BuildId: 26c81e7e05ebaf40bac3523b7d76be0cd71fad82)
ksh93#21 0x7f55d5b50349 in __libc_start_main (/usr/lib/libc.so.6+0x23349) (BuildId: 26c81e7e05ebaf40bac3523b7d76be0cd71fad82)
ksh93#22 0x557b1fc158d4 in _start /build/glibc/src/glibc/csu/../sysdeps/x86_64/start.S:115
src/cmd/ksh93/include/lexstates.h:
- Check if c is negative before accessing sh_lexstates. Backported from
ksh2020: att@a7013320.
I'll note that later in ksh2020 these macros became functions:
att@adc589de. I didn't backport that
commit because it requires the C99 bool type to avoid compiler
warnings.
McDutchie
added a commit
that referenced
this pull request
Feb 29, 2024
When ksh executes a script without a #! path (note that the AT&T
team had a real disliking for #! paths), ksh forks and goes through
a quick reinitialisation procedure. This is much faster than
invoking a fully new shell but should have the same effect if it
all works well. Unfortunately it's not worked all that well so far.
Even after recent improvements (see referenced commits) I've been
finding corner case problems.
FYI, running a script without #! basically goes like this:
* in path_spawn(), execve() fails with ENOEXEC because the file is
not a binary executable and does not start with #!
* this triggers 'case ENOEXEC:' which:
* forks ksh
* calls exscript()
* exscript() cleans up & calls siglongjmp(*sh.jmplist,SH_JMPSCRIPT)
* SH_JMPSCRIPT is the highest longjmp value, so *all* the previous
sigsetjmp/sh_pushcontext calls are unwinded in reverse order,
triggering all sorts of cleanup, state restoration, removal of
local scopes, etc.
* eventually, this lands us at the top sigsetjmp in sh_main()
* sh_main() calls sh_reinit(), then resumes as if the shell had
just been started
This commit makes the following interrelated changes for the
correct functioning of this procedure:
1. exscript() now exports the environment into a dedicated Stk_t
buffer and sets environ[] to that.
2. Instead of re-using existing variables, sh_reinit() deletes
everything and reinits all name-value trees from scratch,
then re-imports the environment from environ[].
3. Variable values that were imported from the environment are no
longer treated specially with an NV_IMPORT attribute and the
np->nvenv pointer to their value in environ[], fixing at least
one crash.[*1]
Details of the changes follow:
src/cmd/ksh93/sh/path.c:
- exscript(): Generate a new environ[] by activating a dedicated
AST stack that will not be overwritten before calling
sh_envgen(). This will allow sh_reinit() to delete all variables
and then reimport the environment. The exporting must be done
here, before siglongjmp, otherwise locally scoped exported
variables won't be included (siglongjmp with SH_JMPSCRIPT
triggers cleanup of all scopes).
src/cmd/ksh93/sh/init.c:
- sh_reinit(): Largely rewritten as follows.
- Reset shell options first. This has the beneficial side effect
of unsetting SH_RESTRICTED which interferes with unsetting
certain variables, like PATH.
- Remove workarounds for FPATH, SHLVL and tilde expansion
disciplines; these will not be needed now.
- Properly unset and delete all functions and built-ins. Since we
now unset a function before deleting it, this should now free
up their memory. (See nvdisc.c below for a change allowing
removal of special built-ins.)
- Properly unset all variables (which includes any associated
discipline functions). Incorporate here the needed logic from
sh_envnolocal() in name.c; most of it is unneeded (that
function was previously used to cleanup local variables but has
not been used for that for decades). So sh_envnolocal() is now
unused.
- Delete variables in a separate pass after unsetting variables
and unsetting and deleting functions; this avoids use-after-
free problems as well as possible "no parent" problems with
namespace variables (e.g., .rc.status in our new kshrc.sh).
- After all that, close and free up all function, alias, tracked
alias, type and variable trees.
- Free the contiguous built-in node space and the Init_t init
context (with all the special variable discipline pointers).
- Call nv_init (previously only called from sh_init) to
reinitialise all of the above name-value stuff from scratch.
It's the only way to be sure.
- Re-import the environment as stored by exscript() above.
- env_init():
- Per item 3 above and footnote 1 below, no longer set NV_IMPORT
attribute and no longer point np->nvenv to the item in environ.
- POSIX says, for 'environ': "Any application that directly
modifies the pointers to which the environ variable points has
undefined behavior."[*2] Yet, env_init() is indeed juggling the
environ[] pointers to deal with variables that cannot be
imported because their names are invalid (because they still
need to be saved to be passed on to child processes). Replace
the current approach with one where those env vars get
allocated on the heap, pointed to by sh.save_env and counted by
sh.save_env_n (renamed from sh.nenv). This only needs to be
done once as ksh cannot use or change these variables.
src/cmd/ksh93/sh/name.c:
- sh_envgen(): Update to match env_init() change above.
- pushnam() (called by sh_envgen()): Remove NV_IMPORT attribute
check as per above and never get the value from the nvenv pointer
-- simply always use nv_getval(). As of this change, the
NV_IMPORT attribute is unused. The next commit will remove it
and do related cleanups.
- staknam(): is only called if value!=NULL, so remove that 'if'.
- sh_envnolocal(): Removed.
src/cmd/ksh93/sh/nvdisc.c:
- assign(): Remove a check for the SH_INIT state bit that avoids
freeing functions during sh_reinit(). This works fine now.
- sh_addbuiltin(): Allow sh_reinit() to delete special builtins by
checking for the SH_INIT state bit before throwing an error.
src/cmd/ksh93/sh/nvtree.c:
- outval(): Add a workaround for a use-after-free, introduced by
the changes above, that occurred in the types.sh tests for
#!-less scripts (types.sh:675-722). The use-after-free occurred
here (abridged ASan trace follows; line numbers are current as of
this commit):
==30849==ERROR: AddressSanitizer: heap-use-after-free [...]
#0 in dttree dttree.c:393
#1 in sh_reinit init.c:1637
#2 in sh_main main.c:136
[...]
The pointer was freed in the same loop via nv_delete() in outval:
#0 in wrap_free+0x98 (libclang_rt.asan_osx_dynamic.dylib:[...])
#1 in nv_delete name.c:1318
#2 in outval nvtree.c:731
#3 in genvalue nvtree.c:905
#4 in walk_tree nvtree.c:1042
#5 in put_tree nvtree.c:1108
#6 in nv_putv nvdisc.c:144
#7 in _nv_unset name.c:2437
#8 in sh_reinit init.c:1645
#9 in sh_main main.c:136
[...]
So, what happened was that the nv_delete() call on name.c:1318
(eventually resulting from the _nv_unset call on init.c:1645)
freed the node pointed to by np, so that the next loop iteration
crashed on line 1637 as the dtnext() macro now gets a freed np.
Now, why on earth should _nv_unset() *ever* indirectly call
nv_delete()? That's a question for another day; I suspect it may
be a bug, or it may be needed for compound variables for some
reason. For now, I'm adding a workaround: simply avoid calling
nv_delete() if the SH_INIT state bit is on, indicating
sh_reinit() is in the call stack. This allows the variables unset
loop in sh_reinit() to continue without crashing. sh_reinit()
handles deletion later anyway.
src/cmd/ksh93/sh/main.c:
- sh_main(): remove zeroing of sh.fun_depth and sh.dot_depth; these
are known to be 0, coming from either sh_init() or sh_reinit().
________
[*1] This NV_IMPORT/nvenv usage is a redundant holdout from ancient
ksh code; the imported value is easily available as a normal
shell variable value via nv_getval(). Plus, the nvenv pointer
is overloaded with too many other purposes: so far I've
discovered it's used for pointers to subarrays of arrays
(multidimentional arrays), compound variables, builtins, and
other things.
This mess caused at least one crash in set_instance() (xec.c)
due to incorrectly using that nvenv pointer. The current kshrc
script triggers this. Reproducer:
$ export PS1
$ bin/package use
«0»26:…/src/ksh93/ksh[dev] $ typeset +x PS1
...and crash. That is now fixed.
[*2] https://pubs.opengroup.org/onlinepubs/9699919799/functions/environ.html
McDutchie
added a commit
that referenced
this pull request
Feb 29, 2024
When ksh executes a script without a #! path (note that the AT&T
team had a real disliking for #! paths), ksh forks and goes through
a quick reinitialisation procedure. This is much faster than
invoking a fully new shell but should have the same effect if it
all works well. Unfortunately it's not worked all that well so far.
Even after recent improvements (see referenced commits) I've been
finding corner case problems.
FYI, running a script without #! basically goes like this:
* in path_spawn(), execve() fails with ENOEXEC because the file is
not a binary executable and does not start with #!
* this triggers 'case ENOEXEC:' which:
* forks ksh
* calls exscript()
* exscript() cleans up & calls siglongjmp(*sh.jmplist,SH_JMPSCRIPT)
* SH_JMPSCRIPT is the highest longjmp value, so *all* the previous
sigsetjmp/sh_pushcontext calls are unwinded in reverse order,
triggering all sorts of cleanup, state restoration, removal of
local scopes, etc.
* eventually, this lands us at the top sigsetjmp in sh_main()
* sh_main() calls sh_reinit(), then resumes as if the shell had
just been started
This commit makes the following interrelated changes for the
correct functioning of this procedure:
1. exscript() now exports the environment into a dedicated Stk_t
buffer and sets environ[] to that.
2. Instead of re-using existing variables, sh_reinit() deletes
everything and reinits all name-value trees from scratch,
then re-imports the environment from environ[].
3. Variable values that were imported from the environment are no
longer treated specially with an NV_IMPORT attribute and the
np->nvenv pointer to their value in environ[], fixing at least
one crash.[*1]
Details of the changes follow:
src/cmd/ksh93/sh/path.c:
- exscript(): Generate a new environ[] by activating a dedicated
AST stack that will not be overwritten before calling
sh_envgen(). This will allow sh_reinit() to delete all variables
and then reimport the environment. The exporting must be done
here, before siglongjmp, otherwise locally scoped exported
variables won't be included (siglongjmp with SH_JMPSCRIPT
triggers cleanup of all scopes).
src/cmd/ksh93/sh/init.c:
- sh_reinit(): Largely rewritten as follows.
- Reset shell options first. This has the beneficial side effect
of unsetting SH_RESTRICTED which interferes with unsetting
certain variables, like PATH.
- Remove workarounds for FPATH, SHLVL and tilde expansion
disciplines; these will not be needed now.
- Properly unset and delete all functions and built-ins. Since we
now unset a function before deleting it, this should now free
up their memory. (See nvdisc.c below for a change allowing
removal of special built-ins.)
- Properly unset all variables (which includes any associated
discipline functions). Incorporate here the needed logic from
sh_envnolocal() in name.c; most of it is unneeded (that
function was previously used to cleanup local variables but has
not been used for that for decades). So sh_envnolocal() is now
unused.
- Delete variables in a separate pass after unsetting variables
and unsetting and deleting functions; this avoids use-after-
free problems as well as possible "no parent" problems with
namespace variables (e.g., .rc.status in our new kshrc.sh).
- After all that, close and free up all function, alias, tracked
alias, type and variable trees.
- Free the contiguous built-in node space and the Init_t init
context (with all the special variable discipline pointers).
- Call nv_init (previously only called from sh_init) to
reinitialise all of the above name-value stuff from scratch.
It's the only way to be sure.
- Re-import the environment as stored by exscript() above.
- env_init():
- Per item 3 above and footnote 1 below, no longer set NV_IMPORT
attribute and no longer point np->nvenv to the item in environ.
- POSIX says, for 'environ': "Any application that directly
modifies the pointers to which the environ variable points has
undefined behavior."[*2] Yet, env_init() is indeed juggling the
environ[] pointers to deal with variables that cannot be
imported because their names are invalid (because they still
need to be saved to be passed on to child processes). Replace
the current approach with one where those env vars get
allocated on the heap, pointed to by sh.save_env and counted by
sh.save_env_n (renamed from sh.nenv). This only needs to be
done once as ksh cannot use or change these variables.
src/cmd/ksh93/sh/name.c:
- sh_envgen(): Update to match env_init() change above.
- pushnam() (called by sh_envgen()): Remove NV_IMPORT attribute
check as per above and never get the value from the nvenv pointer
-- simply always use nv_getval(). As of this change, the
NV_IMPORT attribute is unused. The next commit will remove it
and do related cleanups.
- staknam(): is only called if value!=NULL, so remove that 'if'.
- sh_envnolocal(): Removed.
src/cmd/ksh93/sh/nvdisc.c:
- assign(): Remove a check for the SH_INIT state bit that avoids
freeing functions during sh_reinit(). This works fine now.
- sh_addbuiltin(): Allow sh_reinit() to delete special builtins by
checking for the SH_INIT state bit before throwing an error.
src/cmd/ksh93/sh/nvtree.c:
- outval(): Add a workaround for a use-after-free, introduced by
the changes above, that occurred in the types.sh tests for
#!-less scripts (types.sh:675-722). The use-after-free occurred
here (abridged ASan trace follows; line numbers are current as of
this commit):
==30849==ERROR: AddressSanitizer: heap-use-after-free [...]
#0 in dttree dttree.c:393
#1 in sh_reinit init.c:1637
#2 in sh_main main.c:136
[...]
The pointer was freed in the same loop via nv_delete() in outval:
#0 in wrap_free+0x98 (libclang_rt.asan_osx_dynamic.dylib:[...])
#1 in nv_delete name.c:1318
#2 in outval nvtree.c:731
#3 in genvalue nvtree.c:905
#4 in walk_tree nvtree.c:1042
#5 in put_tree nvtree.c:1108
#6 in nv_putv nvdisc.c:144
#7 in _nv_unset name.c:2437
#8 in sh_reinit init.c:1645
#9 in sh_main main.c:136
[...]
So, what happened was that the nv_delete() call on name.c:1318
(eventually resulting from the _nv_unset call on init.c:1645)
freed the node pointed to by np, so that the next loop iteration
crashed on line 1637 as the dtnext() macro now gets a freed np.
Now, why on earth should _nv_unset() *ever* indirectly call
nv_delete()? That's a question for another day; I suspect it may
be a bug, or it may be needed for compound variables for some
reason. For now, I'm adding a workaround: simply avoid calling
nv_delete() if the SH_INIT state bit is on, indicating
sh_reinit() is in the call stack. This allows the variables unset
loop in sh_reinit() to continue without crashing. sh_reinit()
handles deletion later anyway.
src/cmd/ksh93/sh/main.c:
- sh_main(): remove zeroing of sh.fun_depth and sh.dot_depth; these
are known to be 0, coming from either sh_init() or sh_reinit().
________
[*1] This NV_IMPORT/nvenv usage is a redundant holdout from ancient
ksh code; the imported value is easily available as a normal
shell variable value via nv_getval(). Plus, the nvenv pointer
is overloaded with too many other purposes: so far I've
discovered it's used for pointers to subarrays of arrays
(multidimentional arrays), compound variables, builtins, and
other things.
This mess caused at least one crash in set_instance() (xec.c)
due to incorrectly using that nvenv pointer. The current kshrc
script triggers this. Reproducer:
$ export PS1
$ bin/package use
«0»26:…/src/ksh93/ksh[dev] $ typeset +x PS1
...and crash. That is now fixed.
[*2] https://pubs.opengroup.org/onlinepubs/9699919799/functions/environ.html
McDutchie
added a commit
that referenced
this pull request
Mar 11, 2024
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.
McDutchie
added a commit
that referenced
this pull request
Mar 11, 2024
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.
JohnoKing
added a commit
to JohnoKing/ksh
that referenced
this pull request
Dec 27, 2024
Under ASan the builtins.sh script silently exits with exit status 1
and no backtrace. After isolating the bug into a simplified script,
I managed to reproduce it with an actual stacktrace. Reproducer:
#!/bin/ksh
set -o xtrace # Shows how far the script goes before crashing
# (which is inconsistent and system-dependent
# AFAICT; ALL_LIBCMD can affect it).
bltin='/opt/ast/bin/basename
/opt/ast/bin/cat
/opt/ast/bin/cp
/opt/ast/bin/cut
/opt/ast/bin/dirname
/opt/ast/bin/getconf
/opt/ast/bin/ln
/opt/ast/bin/mktemp
/opt/ast/bin/mv' # Feel free to expand this with other commands
# from libcmd if you with.
for i in ${bltin}
do ({ PATH=/opt/ast/bin; "${bltin##*/}" --this-option-does-not-exist; } 2>&1)
done
Stacktrace obtained after much effort:
=================================================================
==116622==ERROR: AddressSanitizer: heap-use-after-free on address 0x502000000e50 at pc 0x72ad1bb52b7f bp 0x7ffc8b5a0cd0 sp 0x7ffc8b5a0478
READ of size 3 at 0x502000000e50 thread T0
#0 0x72ad1bb52b7e in memcpy /usr/src/debug/gcc/gcc/libsanitizer/sanitizer_common/sanitizer_common_interceptors_memintrinsics.inc:115
ksh93#1 0x5bb0a922f3de in synthesize /home/johno/GitRepos/KornShell/ksh/src/lib/libast/port/astconf.c:466
ksh93#2 0x5bb0a92305cd in initialize /home/johno/GitRepos/KornShell/ksh/src/lib/libast/port/astconf.c:591
ksh93#3 0x5bb0a9230fac in format /home/johno/GitRepos/KornShell/ksh/src/lib/libast/port/astconf.c:632
ksh93#4 0x5bb0a923ba4b in astgetconf /home/johno/GitRepos/KornShell/ksh/src/lib/libast/port/astconf.c:1382
ksh93#5 0x5bb0a923d5c5 in astconf /home/johno/GitRepos/KornShell/ksh/src/lib/libast/port/astconf.c:1472
ksh93#6 0x5bb0a924e07b in initconformance /home/johno/GitRepos/KornShell/ksh/src/lib/libast/misc/conformance.c:50
ksh93#7 0x5bb0a924eff2 in conformance /home/johno/GitRepos/KornShell/ksh/src/lib/libast/misc/conformance.c:122
ksh93#8 0x5bb0a9288b8e in b_cp /home/johno/GitRepos/KornShell/ksh/src/lib/libcmd/cp.c:706
<CUT>
src/lib/libast/port/astconf.c:
- If fp->value and value pointed to the same allocated memory
before realloc, avoid memcpy as value now points to freed memory.
- Produce an obvious panic if memory allocation fails.
src/cmd/ksh93/sh/init.c:
- For correctness, prevent memory leaks by freeing memory in sh_realloc
upon failure.
JohnoKing
added a commit
to JohnoKing/ksh
that referenced
this pull request
Dec 27, 2024
This backports a bugfix from ksh93v- 2012-08-24 for an
invalid read that could cause a crash at the first strncmp
in create_type() due to misalignment (I'm not sure why this
crash doesn't occur on the dev branch, but the read was
nonetheless invalid).
Crash trace from ASan:
test types(C.UTF-8) begins at 2024-12-13+00:52:02
AddressSanitizer:DEADLYSIGNAL
=================================================================
==1860280==ERROR: AddressSanitizer: SEGV on unknown address 0x000000001340 (pc 0x76b7ec289110 bp 0x7ffc5cf9fbb0 sp 0x7ffc5cf9f320 T0)
==1860280==The signal is caused by a READ memory access.
#0 0x76b7ec289110 in strncmp /usr/src/debug/gcc/gcc/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:491
ksh93#1 0x76b7ebe8cfb1 in create_type /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/nvtype.c:486
ksh93#2 0x76b7ebf5612f in create_tree /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/nvtree.c:74
ksh93#3 0x76b7ebf431fa in nv_create /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/name.c:1239
ksh93#4 0x76b7ebf45336 in nv_open /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/name.c:1452
ksh93#5 0x76b7ebf3da95 in nv_setlist /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/name.c:608
ksh93#6 0x76b7ebfa4c7d in sh_exec /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/xec.c:1077
ksh93#7 0x76b7ebe4aba4 in b_dot_cmd /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/bltins/misc.c:324
ksh93#8 0x76b7ebfbc0e5 in sh_funct /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/xec.c:3204
ksh93#9 0x76b7ebfbd40e in sh_fun /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/xec.c:3286
ksh93#10 0x76b7ebe95cdf in type_init /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/nvtype.c:1255
ksh93#11 0x76b7ebe964f5 in nv_settype /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/nvtype.c:1341
ksh93#12 0x76b7ebe6e7c9 in setall /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/bltins/typeset.c:893
ksh93#13 0x76b7ebe6b88d in b_typeset /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/bltins/typeset.c:574
ksh93#14 0x76b7ebfa74df in sh_exec /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/xec.c:1270
ksh93#15 0x76b7ebe7d6f7 in exfile /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/main.c:591
ksh93#16 0x76b7ebe7acf6 in sh_main /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/main.c:344
JohnoKing
added a commit
to JohnoKing/ksh
that referenced
this pull request
Dec 29, 2024
Under ASan the builtins.sh script silently exits with exit status 1
and no backtrace. After isolating the bug into a simplified script,
I managed to reproduce it with an actual stacktrace. Reproducer:
set -o xtrace # Shows how far the script goes before crashing
# (which is inconsistent and system-dependent
# AFAICT; ALL_LIBCMD can affect it).
bltin='/opt/ast/bin/basename
/opt/ast/bin/cat
/opt/ast/bin/cp
/opt/ast/bin/cut
/opt/ast/bin/dirname
/opt/ast/bin/getconf
/opt/ast/bin/ln
/opt/ast/bin/mktemp
/opt/ast/bin/mv' # Feel free to expand this with other commands
# from libcmd if you with.
for i in ${bltin}
do ({ PATH=/opt/ast/bin; "${bltin##*/}" --this-option-does-not-exist; } 2>&1)
done
Stacktrace obtained after much effort:
=================================================================
==116622==ERROR: AddressSanitizer: heap-use-after-free on address 0x502000000e50 at pc 0x72ad1bb52b7f bp 0x7ffc8b5a0cd0 sp 0x7ffc8b5a0478
READ of size 3 at 0x502000000e50 thread T0
#0 0x72ad1bb52b7e in memcpy /usr/src/debug/gcc/gcc/libsanitizer/sanitizer_common/sanitizer_common_interceptors_memintrinsics.inc:115
ksh93#1 0x5bb0a922f3de in synthesize /home/johno/GitRepos/KornShell/ksh/src/lib/libast/port/astconf.c:466
ksh93#2 0x5bb0a92305cd in initialize /home/johno/GitRepos/KornShell/ksh/src/lib/libast/port/astconf.c:591
ksh93#3 0x5bb0a9230fac in format /home/johno/GitRepos/KornShell/ksh/src/lib/libast/port/astconf.c:632
ksh93#4 0x5bb0a923ba4b in astgetconf /home/johno/GitRepos/KornShell/ksh/src/lib/libast/port/astconf.c:1382
ksh93#5 0x5bb0a923d5c5 in astconf /home/johno/GitRepos/KornShell/ksh/src/lib/libast/port/astconf.c:1472
ksh93#6 0x5bb0a924e07b in initconformance /home/johno/GitRepos/KornShell/ksh/src/lib/libast/misc/conformance.c:50
ksh93#7 0x5bb0a924eff2 in conformance /home/johno/GitRepos/KornShell/ksh/src/lib/libast/misc/conformance.c:122
ksh93#8 0x5bb0a9288b8e in b_cp /home/johno/GitRepos/KornShell/ksh/src/lib/libcmd/cp.c:706
<CUT>
src/lib/libast/port/astconf.c:
- If fp->value and value pointed to the same allocated memory
before realloc, avoid memcpy as value now points to freed memory.
- Produce an obvious panic if memory allocation fails.
src/cmd/ksh93/sh/init.c:
- For correctness, prevent memory leaks by freeing memory in sh_realloc
upon failure.
JohnoKing
added a commit
to JohnoKing/ksh
that referenced
this pull request
Dec 29, 2024
Under ASan the builtins.sh script silently exits with exit status 1
and no backtrace. After isolating the bug into a simplified script,
I managed to reproduce it with an actual stacktrace. Reproducer:
set -o xtrace # Shows how far the script goes before crashing
# (which is inconsistent and system-dependent
# AFAICT; ALL_LIBCMD can affect it).
bltin='/opt/ast/bin/basename
/opt/ast/bin/cat
/opt/ast/bin/cp
/opt/ast/bin/cut
/opt/ast/bin/dirname
/opt/ast/bin/getconf
/opt/ast/bin/ln
/opt/ast/bin/mktemp
/opt/ast/bin/mv' # Feel free to expand this with other commands
# from libcmd if you with.
for i in ${bltin}
do ({ PATH=/opt/ast/bin; "${bltin##*/}" --this-option-does-not-exist; } 2>&1)
done
Stacktrace obtained after much effort:
=================================================================
==116622==ERROR: AddressSanitizer: heap-use-after-free on address 0x502000000e50 at pc 0x72ad1bb52b7f bp 0x7ffc8b5a0cd0 sp 0x7ffc8b5a0478
READ of size 3 at 0x502000000e50 thread T0
#0 0x72ad1bb52b7e in memcpy /usr/src/debug/gcc/gcc/libsanitizer/sanitizer_common/sanitizer_common_interceptors_memintrinsics.inc:115
ksh93#1 0x5bb0a922f3de in synthesize /home/johno/GitRepos/KornShell/ksh/src/lib/libast/port/astconf.c:466
ksh93#2 0x5bb0a92305cd in initialize /home/johno/GitRepos/KornShell/ksh/src/lib/libast/port/astconf.c:591
ksh93#3 0x5bb0a9230fac in format /home/johno/GitRepos/KornShell/ksh/src/lib/libast/port/astconf.c:632
ksh93#4 0x5bb0a923ba4b in astgetconf /home/johno/GitRepos/KornShell/ksh/src/lib/libast/port/astconf.c:1382
ksh93#5 0x5bb0a923d5c5 in astconf /home/johno/GitRepos/KornShell/ksh/src/lib/libast/port/astconf.c:1472
ksh93#6 0x5bb0a924e07b in initconformance /home/johno/GitRepos/KornShell/ksh/src/lib/libast/misc/conformance.c:50
ksh93#7 0x5bb0a924eff2 in conformance /home/johno/GitRepos/KornShell/ksh/src/lib/libast/misc/conformance.c:122
ksh93#8 0x5bb0a9288b8e in b_cp /home/johno/GitRepos/KornShell/ksh/src/lib/libcmd/cp.c:706
<CUT>
src/lib/libast/port/astconf.c:
- If fp->value and value pointed to the same allocated memory
before realloc, avoid memcpy as value now points to freed memory.
- Produce an obvious panic if memory allocation fails.
src/cmd/ksh93/sh/init.c:
- For correctness, prevent memory leaks by freeing memory in sh_realloc
upon failure.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This fix for
printf '%T' nowon FreeBSD was written by @krader1961. The following is from att#591:While this pull request does fix the initial bug reported in #6, it still doesn't update the outdated
Tm_leap_tleap second array in tmdata.c.