On Sun, 16 Oct 2016 17:10:23 +0000
"Laurent Bercot" <ska-skaware_at_skarnet.org> wrote:
> Ew. Shows how much that code is used - in more than two years,
> nobody (including me) has noticed the segfault.
>
> Since it's so widely used, and since diving back into environment
> frame pushing/popping gives me headaches, I have half a mind to just
> remove runblock in the next release (which is a major version bump so
> compatibility isn't guaranteed). I'll try fixing the bug for a little
> more
> time, and if I can't find it, I'll scrap the whole thing.
Had a quick look, and from what i could gather the issue is that in our
case, nothing gets put into v, so by the time pathexec_run() gets
called, v only contains a NULL, and therefore v[0] (well, it's v.s[0]
but hopefully you get what I'm saying) - which is given as first arg -
is indeed, NULL.
So the "file" argument for pathexec_run() is NULL, which ends up in
execvpe() as a call to strchr() with NULL as string/1st arg, and that's
your segfault (in libc, in musl it actually happens in strchrnul).
The attached patch seems to fix it, and also address the other case:
$ ./helper foo bar baz
runblock: fatal: unable to exec bar: No such file or directory
$ ./helper foo bar
runblock: fatal: unable to exec bar: No such file or directory
$ ./helper foo
runblock: fatal: too few arguments
The "return 0" is what made it return w/out even trying to exec into
anything, I'm guessing this was a mistake trying to catch when there
was no rest of command line to exec into?
> I have implemented your -s suggestion in the latest execline git
> head; that one was easy. Please test it and tell me if it works for
> you.
Yep, works great! Thanks!
Cheers,
Received on Sun Oct 16 2016 - 18:59:01 UTC