Re: [v2 PATCH] expand: Add ifsfree to expand to fix a logic error that causes a buffer over-read

[Herbert Xu <herbert@xxxxxxxxxxxxxxxxxxx>]

On Mon, Dec 05, 2022 at 03:24:22PM +0000, Harald van Dijk wrote:
>
> FWIW, the possibility of other sh_error calls being overlooked is why I
> ended up doing it differently myself: I ended up with exverror() calling
> ifsfree(). It is a smaller change than this; it works because there is
> currently no case where preserving IFS regions after an error is wanted
> anyway, and I cannot imagine a future change that would change that. But it
> is possible that my imagination is lacking there.
> 
> Q: If exception != EXERROR, longjmp() is called without first calling
> ifsfree(), leaving IFS regions in place for a higher level to deal with. If
> execution ultimately ends up at exitreset(), which calls ifsfree() anyway,
> that is fine. Is that guaranteed to always be the case?

Yes I've checked all the setjmp spots for this.

Longer time we should do away with the global variables and change
the sh_error paradigm to one of direct error returns.  Then we can
simply handle this in expandarg itself.

We could do it now too of course by just adding a setjmp there.

Cheers,
-- 
Email: Herbert Xu <herbert@xxxxxxxxxxxxxxxxxxx>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt