bin/165164: [PATCH][bin] xargs incorrect pointer passed to waitchildren function

[Matthew Story]

>Number:         165164
>Category:       bin
>Synopsis:       [PATCH][bin] xargs incorrect pointer passed to waitchildren function
>Confidential:   no
>Severity:       non-critical
>Priority:       low
>Responsible:    freebsd-bugs
>State:          open
>Quarter:        
>Keywords:       
>Date-Required:
>Class:          sw-bug
>Submitter-Id:   current-users
>Arrival-Date:   Wed Feb 15 05:20:05 UTC 2012
>Closed-Date:
>Last-Modified:
>Originator:     Matthew Story
>Release:        9.0
>Organization:
>Environment:
FreeBSD matt9fromouterspace 9.0-RELEASE FreeBSD 9.0-RELEASE #0: Tue Jan  3 07:15:25 UTC 2012     root at obrian.cse.buffalo.edu:/usr/obj/usr/src/sys/GENERIC  i386
>Description:
*argv is manipulated in main to account for Jflag replacement strings (moving argv into av, and leaving *argv a pointer to the first argument following a Jflag replacement string argument || NULL if Jflag not set):

    if (*argv == NULL)
        cnt = strlen(*bxp++ = echo);
    else {
        do {
            if (Jflag && strcmp(*argv, replstr) == 0) {
                char **avj;
                jfound = 1;
                argv++;
                for (avj = argv; *avj; avj++)
                    cnt += strlen(*avj) + 1;
                break;
            }   
            cnt += strlen(*bxp++ = *argv) + 1;
        } while (*++argv != NULL);
    } 

*argv is then passed to parse_input, which passes *av to prerun, which in turn passes *av (locally argv) to waitchildren for diagnostic purposes, while the calls from parse_input itself pass *argv to waitchildren for diagnostic purposes.  This incongruity means that calls directly from parse_input (only in cases after EOF has been seen) take either NULL or the first argument after a Jflag string argument as utility name for diagnostic purposes, leading to output like:

xargs: (null): No such file or directory
>How-To-Repeat:
Found this issue after applying patch from PR 165155, which provides POSIX-compliant diagnostic information on exit 255 and children terminated by signals.  Condition exists, but is masked for the most part by a race on line 568 (WNOHANG wait immediately following vfork, execvp), if you comment out that line:

/* waitchildren(*argv, 0); */

You will reliably see the following behavior:

$ # this will work unless you have a this_does_not_exist in your PATH
$ echo "hi" | ./xargs -P10 -n1 this_does_not_exist
xargs: (null): No such file or directory
$ # this should read: xargs: hi: No such file or directory
$ echo "this_does_not_exist" | ./xargs -J % % sh
xargs: sh: No such file or directory

You can alternalively apply the patch from PR 165155, and the following will always yield the (null) error condition:

$ jot - 1 10 | ./xargs -P10 -n12 sh -c 'sleep 1; exit 255'
xargs: (null): exited with status 255, aborting

This issue should be resolved prior to patching PR 165155, I will make a note in that ticket as well.  Following applying the patch:

$ echo "hi" | ./xargs blah
xargs: blah: No such file or directory
$ echo "hi" | ./xargs -J % % sh   
xargs: hi: No such file or directory

And for the PR 165155 case:

$ jot - 1 10 | ./xargs -P10 -n12 sh -c 'sleep 1; exit 255'
xargs: sh: exited with status 255, aborting
>Fix:
Apply patch.  This program looks like it needs a larger re-factor, but the solution provided in the patch is to send *av to waitchildren instead of *argv (which is a pointer to the first argument following a Jflag replacement string || null) to make the behavior congruent with the argument list passed to prerun (locally argv).

Patch attached with submission follows:

--- a/usr.bin/xargs/xargs.c	2012-02-14 22:54:29.000000000 -0500
+++ b/usr.bin/xargs/xargs.c	2012-02-14 16:46:37.000000000 -0500
@@ -281,7 +281,7 @@
 	case EOF:
 		/* No arguments since last exec. */
 		if (p == bbp) {
-			waitchildren(*argv, 1);
+			waitchildren(*av, 1);
 			exit(rval);
 		}
 		goto arg1;
@@ -368,7 +368,7 @@
 			}
 			prerun(argc, av);
 			if (ch == EOF || foundeof) {
-				waitchildren(*argv, 1);
+				waitchildren(*av, 1);
 				exit(rval);
 			}
 			p = bbp;


>Release-Note:
>Audit-Trail:
>Unformatted: