Skip to content

Commit

Permalink
Scope variables to their subshell before running their discipline
Browse files Browse the repository at this point in the history
Currently, running the tilde.sh tests under ASan will fail with a
use after free. Stacktrace below (with irrelevant parts cut out):
test tilde(C.UTF-8) begins at 2024-12-28+20:08:53
=================================================================
==189441==ERROR: AddressSanitizer: heap-use-after-free on address 0x5070000069a8 at pc 0x7b65d030bd77 bp 0x7ffc12db7390 sp 0x7ffc12db7380
READ of size 2 at 0x5070000069a8 thread T0
    #0 0x7b65d030bd76 in simple /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/parse.c:1484
    ksh93#1 0x7b65d030a209 in item /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/parse.c:1374
    ksh93#2 0x7b65d02fc516 in term /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/parse.c:655
    ---CUT 1---

0x5070000069a8 is located 24 bytes inside of 78-byte region [0x507000006990,0x5070000069de)
freed by thread T0 here:
    #0 0x7b65d0d53ef2 in free /usr/src/debug/gcc/gcc/libsanitizer/asan/asan_malloc_linux.cpp:52
    ksh93#1 0x7b65d02bd246 in nv_delete /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/name.c:1329
    ksh93#2 0x7b65d03621d3 in sh_subshell /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/subshell.c:807
    ksh93#3 0x7b65d029902b in comsubst /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/macro.c:2268
    ---CUT 2---

previously allocated by thread T0 here:
    #0 0x7b65d0d555da in calloc /usr/src/debug/gcc/gcc/libsanitizer/asan/asan_malloc_linux.cpp:77
    ksh93#1 0x7b65d01dd012 in sh_calloc /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/init.c:255
    ksh93#2 0x7b65d0135140 in newnode /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/nvdisc.c:823
    ksh93#3 0x7b65d0138da4 in nv_search /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/nvdisc.c:1022
    ksh93#4 0x7b65d02c0552 in nv_open /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/name.c:1486
    ksh93#5 0x7b65d039d356 in sh_exec /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/xec.c:2484
    ---CUT 3---

The crash occurs because the discipline function is assigned
before .sh.tilde is scoped to the currently active virtual subshell.
After this, sh_subshell frees the discipline function by calling
nv_delete upon subshell completion, but because of improper scoping
.sh.tilde in the parent function now has an np->nvfun which points to
freed memory. (As a side note, I'll note that this bug can be reproduced
for any variable assigned a discipline function, not just .sh.tilde.)

src/cmd/ksh93/sh/xec.c: sh_exec():
- Use sh_assignok to scope variables to subshells before assigning
  a new discipline function to them.
  • Loading branch information
JohnoKing committed Dec 29, 2024
1 parent be0cfbf commit 7ad6d58
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 0 deletions.
6 changes: 6 additions & 0 deletions NEWS
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,12 @@ Uppercase BUG_* IDs are shell bug IDs as used by the Modernish shell library.
- [v1.1] The emacs ^Y command now accepts a numeric parameter. For example,
ESC 10 ^Y will now "yank" (paste) the latest deleted text 10 times.

- Fixed a crash that could occur if a discipline function was first
assigned to a variable in a virtual subshell before the variable was
scoped to that subshell, then upon subshell completion another discipline
function of the same type was assigned to that selfsame variable in the
parent shell.

2024-12-25:

- The dirname path-bound built-in now accepts multiple operands.
Expand Down
11 changes: 11 additions & 0 deletions src/cmd/ksh93/sh/xec.c
Original file line number Diff line number Diff line change
Expand Up @@ -2485,7 +2485,18 @@ int sh_exec(const Shnode_t *t, int flags)
if(npv)
{
if(!sh.mktype)
{
if(sh.subshell && !sh.subshare)
{
/*
* When a variable is given a discipline function in
* a subshell, the variable must be scoped to the
* subshell before nvfun is set to the discipline.
*/
sh_assignok(npv, 1);
}
cp = nv_setdisc(npv,cp,np,(Namfun_t*)npv);
}
if(!cp)
{
errormsg(SH_DICT,ERROR_exit(1),e_baddisc,fname);
Expand Down

0 comments on commit 7ad6d58

Please sign in to comment.