[ast-developers] Two bugs in libast's stk code

[Michael Schroeder]

Hi David & Co,

I found two bugs in libast's stk handling that lead to ksh crashes
in rare circumstances. They both are in the alias handling code.

1) the pointer stored in the alias list is wrong. It's supposed to
be the old start of the frame, but the new location is used instead.

2) if there already is one alias and the frame does not get moved,
the alias is not copied.

Seems like both bugs were added with the
12-05-18 misc/stk.c: fix access of moved realloc() data
change.

The attached "ksh93-stkalias.dif" patch fixes both bugs. Without
the patch, ksh may do a stkset() call with an unknown address, which
leads to the stk being reset to the beginning.
(This leads to rather hard to debug crashes, so I'm wondering if
simply doing an abort() if the address is non-NULL and unknown would
not be better instead of the reset).

Regarding the alias handling itself: I don't really like the concept
of storing pointers into freed memory. The old address may end up
in another frame later, and then you don't know to what point to
free the stack.
IMHO a better way would be to change the stkset documentation to
require a "frozen" address (i.e. returned by stkfreeze/stkalloc).
So you'd have a single "hasfrozen" flag in the frame instead of the
alias list, which is set to TRUE by stkfreeze/stkalloc.

AFAICT there are only two places in ksh where stkset is called with
a non-frozen address, and both can be fixed easily to use stkfreeze()
instead of stkptr().

(Btw, isn't the "see whether current frame can be extended" check missing
a roundof(, STK_ALIGN)?)

Cheers,
Michael.