On Sun, 16 Oct 2016 17:10:23 +0000 "Laurent Bercot" <ska-skaw...@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,
>From d01f2fe92be9d50d5592786b5aec08cabb41e001 Mon Sep 17 00:00:00 2001 From: Olivier Brunel <j...@jjacky.com> Date: Sun, 16 Oct 2016 20:38:55 +0200 Subject: [PATCH] runblock: Fix segfault Signed-off-by: Olivier Brunel <j...@jjacky.com> --- src/execline/runblock.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/execline/runblock.c b/src/execline/runblock.c index a654023..78a4b59 100644 --- a/src/execline/runblock.c +++ b/src/execline/runblock.c @@ -77,7 +77,7 @@ int main (int argc, char const *const *argv, char const *const *envp) if (flagr) /* put remainder envvars into v */ { - if (++m == sharp) return 0 ; + if (++m > sharp) strerr_dief1x(100, "too few arguments") ; if (!genalloc_ready(char const *, &v, sharp - m + 2)) strerr_diefu1sys(111, "genalloc_ready") ; for (; m <= sharp ; m++) -- 2.10.0