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

Reply via email to