Libguestfs - Feb 2023 - [libnbd PATCH v3 10/29] lib/utils: add unit tests for async-signal-safe execvpe()

Don't try to test async-signal-safety, but strive to exercise as many as
possible paths through nbd_internal_execvpe_init() and
nbd_internal_fork_safe_execvpe().

Signed-off-by: Laszlo Ersek <lersek at redhat.com>
---
 lib/test-fork-safe-execvpe.c  | 117 +++++++++
 lib/Makefile.am               |  10 +
 lib/test-fork-safe-execvpe.sh | 270 ++++++++++++++++++++
 .gitignore                    |   1 +
 4 files changed, 398 insertions(+)

diff --git a/lib/test-fork-safe-execvpe.c b/lib/test-fork-safe-execvpe.c
new file mode 100644
index 000000000000..09b91102b613
--- /dev/null
+++ b/lib/test-fork-safe-execvpe.c
@@ -0,0 +1,117 @@
+  /* nbd client library in userspace
+ * Copyright (C) 2013-2023 Red Hat Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+#include <config.h>
+
+#include <errno.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+
+#include "internal.h"
+
+extern char **environ;
+
+/* This is a perror() replacement that makes the error message more
+ * machine-readable, for a select few error numbers. Do not use it as a general
+ * error() replacement, only upon nbd_internal_execvpe_init() and
+ * nbd_internal_fork_safe_execvpe() failure.
+ */
+static void
+xperror (const char *s)
+{
+  const char *err;
+
+  if (s != NULL && *s != '\0')
+    (void)fprintf (stderr, "%s: ", s);
+
+  switch (errno) {
+  case EACCES:
+    err = "EACCES";
+    break;
+  case ELOOP:
+    err = "ELOOP";
+    break;
+  case ENOENT:
+    err = "ENOENT";
+    break;
+  case ENOTDIR:
+    err = "ENOTDIR";
+    break;
+  default:
+    err = strerror (errno);
+  }
+  (void)fprintf (stderr, "%s\n", err);
+}
+
+int
+main (int argc, char **argv)
+{
+  struct execvpe ctx;
+  const char *prog_file;
+  string_vector prog_argv;
+  size_t i;
+
+  if (argc < 3) {
+    fprintf (stderr, "%1$s: usage: %1$s program-to-exec argv0 ...\n",
argv[0]);
+    return EXIT_FAILURE;
+  }
+
+  prog_file = argv[1];
+
+  /* For the argv of the program to execute, we need to drop our argv[0] (= our
+   * own name) and argv[1] (= the program we need to execute), and to tack on a
+   * terminating null pointer. Note that "argc" does not include the
terminating
+   * NULL.
+   */
+  prog_argv = (string_vector)empty_vector;
+  if (string_vector_reserve (&prog_argv, argc - 2 + 1) == -1) {
+    perror ("string_vector_reserve");
+    return EXIT_FAILURE;
+  }
+
+  for (i = 2; i < argc; ++i)
+    (void)string_vector_append (&prog_argv, argv[i]);
+  (void)string_vector_append (&prog_argv, NULL);
+
+  if (nbd_internal_execvpe_init (&ctx, prog_file, prog_argv.len) == -1) {
+    xperror ("nbd_internal_execvpe_init");
+    goto reset_prog_argv;
+  }
+
+  /* Print out the generated candidates. */
+  for (i = 0; i < ctx.pathnames.len; ++i)
+    (void)fprintf (stdout, "%s\n", ctx.pathnames.ptr[i]);
+
+  if (fflush (stdout) == EOF) {
+    perror ("fflush");
+    goto uninit_execvpe;
+  }
+
+  (void)nbd_internal_fork_safe_execvpe (&ctx, &prog_argv, environ);
+  xperror ("nbd_internal_fork_safe_execvpe");
+  /* fall through */
+
+uninit_execvpe:
+  nbd_internal_execvpe_uninit (&ctx);
+
+reset_prog_argv:
+  string_vector_reset (&prog_argv);
+
+  return EXIT_FAILURE;
+}
diff --git a/lib/Makefile.am b/lib/Makefile.am
index 1f6501ceb49a..65a2f49e4baa 100644
--- a/lib/Makefile.am
+++ b/lib/Makefile.am
@@ -33,6 +33,7 @@ EXTRA_DIST = \
 	$(generator_built) \
 	libnbd.syms \
 	test-fork-safe-assert.sh \
+	test-fork-safe-execvpe.sh \
 	$(NULL)
 
 lib_LTLIBRARIES = libnbd.la
@@ -104,10 +105,12 @@ pkgconfig_DATA = libnbd.pc
 
 TESTS = \
 	test-fork-safe-assert.sh \
+	test-fork-safe-execvpe.sh \
 	$(NULL)
 
 check_PROGRAMS = \
 	test-fork-safe-assert \
+	test-fork-safe-execvpe \
 	$(NULL)
 
 test_fork_safe_assert_SOURCES = \
@@ -116,3 +119,10 @@ test_fork_safe_assert_SOURCES = \
 	test-fork-safe-assert.c \
 	utils.c \
 	$(NULL)
+
+test_fork_safe_execvpe_SOURCES = \
+	$(top_srcdir)/common/utils/vector.c \
+	errors.c \
+	test-fork-safe-execvpe.c \
+	utils.c \
+	$(NULL)
diff --git a/lib/test-fork-safe-execvpe.sh b/lib/test-fork-safe-execvpe.sh
new file mode 100755
index 000000000000..ac9c48bb83c6
--- /dev/null
+++ b/lib/test-fork-safe-execvpe.sh
@@ -0,0 +1,270 @@
+#!/usr/bin/env bash
+# nbd client library in userspace
+# Copyright (C) 2013-2023 Red Hat Inc.
+#
+# This library is free software; you can redistribute it and/or
+# modify it under the terms of the GNU Lesser General Public
+# License as published by the Free Software Foundation; either
+# version 2 of the License, or (at your option) any later version.
+#
+# This library is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+# Lesser General Public License for more details.
+#
+# You should have received a copy of the GNU Lesser General Public
+# License along with this library; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+
+set -e
+
+# Determine the absolute pathname of the execvpe helper binary. The
"realpath"
+# utility is not in POSIX, but Linux, FreeBSD and OpenBSD all have it.
+bname=$(basename -- "$0" .sh)
+dname=$(dirname -- "$0")
+execvpe=$(realpath -- "$dname/$bname")
+
+# This is an elaborate way to control the PATH variable around the $execvpe
+# helper binary as narrowly as possible.
+#
+# If $1 is "_", then the $execvpe helper binary is invoked with PATH
unset.
+# Otherwise, the binary is invoked with PATH set to $1.
+#
+# $2 and onward are passed to $execvpe; note that $2 becomes *both*
+# "program-to-exec" for the helper *and* argv[0] for the program
executed by the
+# helper.
+#
+# The command itself (including the PATH setting) is written to "cmd"
(for error
+# reporting purposes only); the standard output and error are saved in
"out" and
+# "err" respectively; the exit status is written to
"status". This function
+# should never fail; if it does, then that's a bug in this unit test
script, or
+# the disk is full etc.
+run()
+{
+    local pathctl=$1
+    local program=$2
+    local exit_status
+
+    shift 1
+
+    if test _ = "$pathctl"; then
+        printf 'unset PATH; %s %s %s\n' "$execvpe"
"$program" "$*" >cmd
+        set +e
+        (
+            unset PATH
+            "$execvpe" "$program" "$@" >out
2>err
+        )
+        exit_status=$?
+        set -e
+    else
+        printf 'PATH=%s %s %s %s\n' "$pathctl"
"$execvpe" "$program" "$*" >cmd
+        set +e
+        PATH=$pathctl "$execvpe" "$program" "$@"
>out 2>err
+        exit_status=$?
+        set -e
+    fi
+    printf '%d\n' $exit_status >status
+}
+
+# After "run" returns, the following three functions can verify the
result.
+#
+# Check if the helper binary failed in nbd_internal_execvpe_init().
+#
+# $1 is the error number (a macro name such as ENOENT) that's expected of
+# nbd_internal_execvpe_init().
+init_fail()
+{
+    local expected_error="nbd_internal_execvpe_init: $1"
+    local cmd=$(< cmd)
+    local err=$(< err)
+    local status=$(< status)
+
+    if test 0 -eq "$status"; then
+        printf "'%s' should have failed\\n" "$cmd"
>&2
+        return 1
+    fi
+    if test x"$err" != x"$expected_error"; then
+        printf "'%s' should have failed with '%s', got
'%s'\\n" \
+               "$cmd" "$expected_error" "$err"
>&2
+        return 1
+    fi
+}
+
+# Check if the helper binary failed in nbd_internal_fork_safe_execvpe().
+#
+# $1 is the output (the list of candidate pathnames) that
+# nbd_internal_execvpe_init() is expected to produce; with inner
<newline>
+# characters replaced with <comma> characters, and the last
<newline> stripped.
+#
+# $2 is the error number (a macro name such as ENOENT) that's expected of
+# nbd_internal_fork_safe_execvpe().
+execve_fail()
+{
+    local expected_output=$1
+    local expected_error="nbd_internal_fork_safe_execvpe: $2"
+    local cmd=$(< cmd)
+    local out=$(< out)
+    local err=$(< err)
+    local status=$(< status)
+
+    if test 0 -eq "$status"; then
+        printf "'%s' should have failed\\n" "$cmd"
>&2
+        return 1
+    fi
+    if test x"$err" != x"$expected_error"; then
+        printf "'%s' should have failed with '%s', got
'%s'\\n" \
+               "$cmd" "$expected_error" "$err"
>&2
+        return 1
+    fi
+    out=${out//$'\n'/,}
+    if test x"$out" != x"$expected_output"; then
+        printf "'%s' should have output '%s', got
'%s'\\n" \
+               "$cmd" "$expected_output" "$out"
>&2
+        return 1
+    fi
+}
+
+# Check if the helper binary and the program executed by it succeeded.
+#
+# $1 is the output (the list of candidate pathnames) that
+# nbd_internal_execvpe_init() is expected to produce, followed by any output
+# expected of the program that's executed by the helper; with inner
<newline>
+# characters replaced with <comma> characters, and the last
<newline> stripped.
+success()
+{
+    local expected_output=$1
+    local cmd=$(< cmd)
+    local out=$(< out)
+    local status=$(< status)
+
+    if test 0 -ne "$status"; then
+        printf "'%s' should have succeeded\\n"
"$cmd" >&2
+        return 1
+    fi
+    out=${out//$'\n'/,}
+    if test x"$out" != x"$expected_output"; then
+        printf "'%s' should have output '%s', got
'%s'\\n" \
+               "$cmd" "$expected_output" "$out"
>&2
+        return 1
+    fi
+}
+
+# Create a temporary directory and change the working directory to it.
+tmpd=$(mktemp -d)
+trap 'rm -r -- "$tmpd"' EXIT
+cd "$tmpd"
+
+# If the "file" parameter of execvpe() is an empty string, then we
must fail --
+# in nbd_internal_execvpe_init() -- regardless of PATH.
+run _  ""; init_fail ENOENT
+run "" ""; init_fail ENOENT
+run .  ""; init_fail ENOENT
+
+# Create subdirectories for triggering non-fatal internal error conditions of
+# execvpe(). (Almost) every subdirectory will contain one entry, called
"f".
+#
+# Create a directory that's empty.
+mkdir empty
+
+# Create a directory with a named pipe (FIFO) in it.
+mkdir fifo
+mkfifo fifo/f
+
+# Create a directory with a non-executable file in it.
+mkdir nxregf
+touch nxregf/f
+
+# Create a symlink loop.
+ln -s symlink symlink
+
+# Create a directory with a (most likely) binary executable in it.
+mkdir bin
+expr_pathname=$(command -p -v expr)
+cp -- "$expr_pathname" bin/f
+
+# Create a directory with an executable shell script that does not contain a
+# shebang (#!). The script will print $0 and $1, and not depend on PATH for it.
+mkdir sh
+printf "command -p printf '%%s %%s\\\\n' \"\$0\"
\"\$1\"\\n" >sh/f
+chmod u+x sh/f
+
+# In the tests below, invoke each "f" such that the "file"
parameter of
+# execvpe() contain a <slash> character.
+#
+# Therefore, PATH does not matter. Set it to the empty string. (Which in this
+# implementation would cause nbd_internal_execvpe_init() to fail with ENOENT,
if
+# the "file" parameter didn't contain a <slash>.)
+run "" empty/f;   execve_fail empty/f   ENOENT
+run "" fifo/f;    execve_fail fifo/f    EACCES
+run "" nxregf/f;  execve_fail nxregf/f  EACCES
+run "" nxregf/f/; execve_fail nxregf/f/ ENOTDIR
+run "" symlink/f; execve_fail symlink/f ELOOP
+
+# This runs "expr 1 + 1".
+run "" bin/f 1 + 1; success bin/f,2
+
+# This triggers the ENOEXEC branch in nbd_internal_fork_safe_execvpe().
+# nbd_internal_fork_safe_execvpe() will first try
+#
+#   execve("sh/f", {"sh/f", "arg1", NULL}, envp)
+#
+# hitting ENOEXEC. Then it will successfully call
+#
+#   execve("/bin/sh", {"sh/f", "sh/f",
"arg1", NULL}, envp)
+#
+# The shell script will get "sh/f" for $0 and "arg1" for
$1, and print those
+# out.
+run "" sh/f arg1; success sh/f,"sh/f arg1"
+
+# In the tests below, the "file" parameter of execvpe() will not
contain a
+# <slash> character.
+#
+# Show that PATH matters that way -- first, trigger an ENOENT in
+# nbd_internal_execvpe_init() by setting PATH to the empty string.
+run "" expr 1 + 1; init_fail ENOENT
+
+# Fall back to confstr(_CS_PATH) in nbd_internal_execvpe_init(), by unsetting
+# PATH. Verify the generated candidates by invoking "getconf PATH"
here, and
+# appending "/expr" to each prefix.
+expected_output=$(
+    path=$(command -p getconf PATH)
+    IFS=:
+    for p in $path; do
+        printf '%s/%s\n' $p expr
+    done
+    command -p expr 1 + 1
+)
+run _ expr 1 + 1; success "${expected_output//$'\n'/,}"
+
+# Continue with tests where the "file" parameter of execvpe() does
not contain a
+# <slash> character, but now set PATH to explicit prefix lists.
+#
+# Show that, if the last candidate fails execve() with an error number that
+# would not be fatal otherwise, we do get that error number.
+run empty:fifo:nxregf:symlink f
+execve_fail empty/f,fifo/f,nxregf/f,symlink/f ELOOP
+
+# Put a single prefix in PATH, such that it lead to a successful execution.
This
+# exercises two things at the same time: (a) that nbd_internal_execvpe_init()
+# produces *one* candidate (i.e., that no <colon> is seen), and (b) that
+# nbd_internal_fork_safe_execvpe() succeeds for the *last* candidate. Repeat
the
+# test with "expr" (called "f" under "bin") and
the shell script (called "f"
+# under "sh", triggering the ENOEXEC branch).
+run bin f 1 + 1; success bin/f,2
+run sh  f arg1;  success sh/f,"sh/f arg1"
+
+# Demonstrate that the order of candidates matters. The first invocation finds
+# "expr" (called "f" under "bin"), the second
invocation finds the shell script
+# under "sh" (triggering the ENOEXEC branch).
+run empty:bin:sh f 1 + 1; success empty/f,bin/f,sh/f,2
+run empty:sh:bin f arg1;  success empty/f,sh/f,bin/f,"sh/f arg1"
+
+# Check the expansion of zero-length prefixes in PATH to ".", plus
the
+# (non-)insertion of the "/" separator.
+run a/:       f;       execve_fail a/f,./f                 ENOENT
+run :a/       f;       execve_fail ./f,a/f                 ENOENT
+run :         f;       execve_fail ./f,./f                 ENOENT
+pushd bin
+run :         f 1 + 1; success     ./f,./f,2
+popd
+run :a/:::b/: f;       execve_fail ./f,a/f,./f,./f,b/f,./f ENOENT
diff --git a/.gitignore b/.gitignore
index cd27a4ed7557..5c42060dc0dd 100644
--- a/.gitignore
+++ b/.gitignore
@@ -126,6 +126,7 @@ Makefile.in
 /lib/states.h
 /lib/test-fork-safe-assert
 /lib/test-fork-safe-assert.err
+/lib/test-fork-safe-execvpe
 /lib/unlocked.h
 /libtool
 /ltmain.sh

For #6, #7, #8, #9, #10:

Reviewed-by: Richard W.M. Jones <rjones at redhat.com>

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-builder quickly builds VMs from scratch
http://libguestfs.org/virt-builder.1.html

On Wed, Feb 15, 2023 at 03:11:39PM +0100, Laszlo Ersek
wrote:
> Don't try to test async-signal-safety, but strive to exercise as many
as
> possible paths through nbd_internal_execvpe_init() and
> nbd_internal_fork_safe_execvpe().
> 
> Signed-off-by: Laszlo Ersek <lersek at redhat.com>
> ---
>  lib/test-fork-safe-execvpe.c  | 117 +++++++++
>  lib/Makefile.am               |  10 +
>  lib/test-fork-safe-execvpe.sh | 270 ++++++++++++++++++++
>  .gitignore                    |   1 +
>  4 files changed, 398 insertions(+)
> 
> diff --git a/lib/test-fork-safe-execvpe.c b/lib/test-fork-safe-execvpe.c
> new file mode 100644
> index 000000000000..09b91102b613
> --- /dev/null
> +++ b/lib/test-fork-safe-execvpe.c
> +
> +/* This is a perror() replacement that makes the error message more
> + * machine-readable, for a select few error numbers. Do not use it as a
general
> + * error() replacement, only upon nbd_internal_execvpe_init() and
> + * nbd_internal_fork_safe_execvpe() failure.
> + */
> +static void
> +xperror (const char *s)
Handy for the .sh counterpart that reads expected output.
> +
> +int
> +main (int argc, char **argv)
> +{
> +  struct execvpe ctx;
> +  const char *prog_file;
> +  string_vector prog_argv;
> +  size_t i;
> +
> +  if (argc < 3) {
> +    fprintf (stderr, "%1$s: usage: %1$s program-to-exec argv0
...\n", argv[0]);
> +    return EXIT_FAILURE;
> +  }
> +
> +  prog_file = argv[1];
> +
> +  /* For the argv of the program to execute, we need to drop our argv[0]
(= our
> +   * own name) and argv[1] (= the program we need to execute), and to tack
on a
> +   * terminating null pointer. Note that "argc" does not include
the terminating
> +   * NULL.
> +   */
> +  prog_argv = (string_vector)empty_vector;
> +  if (string_vector_reserve (&prog_argv, argc - 2 + 1) == -1) {
> +    perror ("string_vector_reserve");
> +    return EXIT_FAILURE;
> +  }
> +
> +  for (i = 2; i < argc; ++i)
> +    (void)string_vector_append (&prog_argv, argv[i]);
> +  (void)string_vector_append (&prog_argv, NULL);
I don't know if I would have used the cast-to-void.  But it doesn't
hurt (this is a unit test and you are documenting that we are
basically ignoring append errors as unlikely), and at this point it's
more effort to rip than out than leave well enough alone.
> +
> +  if (nbd_internal_execvpe_init (&ctx, prog_file, prog_argv.len) ==
-1) {
> +    xperror ("nbd_internal_execvpe_init");
> +    goto reset_prog_argv;
> +  }
> +
> +  /* Print out the generated candidates. */
> +  for (i = 0; i < ctx.pathnames.len; ++i)
> +    (void)fprintf (stdout, "%s\n", ctx.pathnames.ptr[i]);
Peering into the internals.  Naughty for normal code, but perfectly
acceptable for the unit test.
> +
> +  if (fflush (stdout) == EOF) {
> +    perror ("fflush");
> +    goto uninit_execvpe;
> +  }
> +
> +  (void)nbd_internal_fork_safe_execvpe (&ctx, &prog_argv,
environ);
> +  xperror ("nbd_internal_fork_safe_execvpe");
> +  /* fall through */
> +
> +uninit_execvpe:
> +  nbd_internal_execvpe_uninit (&ctx);
> +
> +reset_prog_argv:
> +  string_vector_reset (&prog_argv);
> +
> +  return EXIT_FAILURE;
> +}
> +++ b/lib/test-fork-safe-execvpe.sh
> +
> +set -e
> +
> +# Determine the absolute pathname of the execvpe helper binary. The
"realpath"
> +# utility is not in POSIX, but Linux, FreeBSD and OpenBSD all have it.
not in POSIX yet, but see https://www.austingroupbugs.net/view.php?id=1457
> +bname=$(basename -- "$0" .sh)
> +dname=$(dirname -- "$0")
> +execvpe=$(realpath -- "$dname/$bname")
> +
> +# This is an elaborate way to control the PATH variable around the
$execvpe
> +# helper binary as narrowly as possible.
> +#
> +# If $1 is "_", then the $execvpe helper binary is invoked with
PATH unset.
> +# Otherwise, the binary is invoked with PATH set to $1.
> +#
> +# $2 and onward are passed to $execvpe; note that $2 becomes *both*
> +# "program-to-exec" for the helper *and* argv[0] for the program
executed by the
> +# helper.
> +#
> +# The command itself (including the PATH setting) is written to
"cmd" (for error
> +# reporting purposes only); the standard output and error are saved in
"out" and
> +# "err" respectively; the exit status is written to
"status". This function
> +# should never fail; if it does, then that's a bug in this unit test
script, or
> +# the disk is full etc.
> +run()
Looks nice.
> +
> +# Create a temporary directory and change the working directory to it.
> +tmpd=$(mktemp -d)
> +trap 'rm -r -- "$tmpd"' EXIT
Our testsuite has cleanup_fn in functions.sh to make this sort of
cleanup work more robust (more than just on EXIT).
> +cd "$tmpd"
> +
> +# If the "file" parameter of execvpe() is an empty string, then
we must fail --
> +# in nbd_internal_execvpe_init() -- regardless of PATH.
> +run _  ""; init_fail ENOENT
> +run "" ""; init_fail ENOENT
> +run .  ""; init_fail ENOENT
> +
> +# Create subdirectories for triggering non-fatal internal error conditions
of
> +# execvpe(). (Almost) every subdirectory will contain one entry, called
"f".
> +#
> +# Create a directory that's empty.
> +mkdir empty
> +
> +# Create a directory with a named pipe (FIFO) in it.
> +mkdir fifo
> +mkfifo fifo/f
> +
> +# Create a directory with a non-executable file in it.
> +mkdir nxregf
> +touch nxregf/f
> +
> +# Create a symlink loop.
> +ln -s symlink symlink
There are two places where ELOOP could matter: both in PATH (symlink/f
can't resolve, regardless of the binary name we are looking up) and in
the binary (./symlink or PATH=. symlink).  Looking ahead, I'm seeing
your tests of symlink as a directory name, but not as a binary name.
But I don't know if adding it would increase coverage of your code, so
much as coverage of the underlying execve() call.
> +
> +# Create a directory with a (most likely) binary executable in it.
> +mkdir bin
> +expr_pathname=$(command -p -v expr)
> +cp -- "$expr_pathname" bin/f
> +
> +# Create a directory with an executable shell script that does not contain
a
> +# shebang (#!). The script will print $0 and $1, and not depend on PATH
for it.
> +mkdir sh
> +printf "command -p printf '%%s %%s\\\\n' \"\$0\"
\"\$1\"\\n" >sh/f
> +chmod u+x sh/f
Based on your reactions to my comments on 09/29 about handling empty
PATH= the same as PATH=., and on whether to allow the child process to
chdir() and/or deal with a PATH element or binary name such as "+s"
that could impact /bin/sh's behavior, there may be other cases you
want to add to the testsuite.
> +
> +# In the tests below, invoke each "f" such that the
"file" parameter of
> +# execvpe() contain a <slash> character.
> +#
> +# Therefore, PATH does not matter. Set it to the empty string. (Which in
this
> +# implementation would cause nbd_internal_execvpe_init() to fail with
ENOENT, if
> +# the "file" parameter didn't contain a <slash>.)
> +run "" empty/f;   execve_fail empty/f   ENOENT
> +run "" fifo/f;    execve_fail fifo/f    EACCES
> +run "" nxregf/f;  execve_fail nxregf/f  EACCES
> +run "" nxregf/f/; execve_fail nxregf/f/ ENOTDIR
> +run "" symlink/f; execve_fail symlink/f ELOOP
> +
> +# This runs "expr 1 + 1".
> +run "" bin/f 1 + 1; success bin/f,2
> +
> +# This triggers the ENOEXEC branch in nbd_internal_fork_safe_execvpe().
> +# nbd_internal_fork_safe_execvpe() will first try
> +#
> +#   execve("sh/f", {"sh/f", "arg1", NULL},
envp)
> +#
> +# hitting ENOEXEC. Then it will successfully call
> +#
> +#   execve("/bin/sh", {"sh/f", "sh/f",
"arg1", NULL}, envp)
> +#
> +# The shell script will get "sh/f" for $0 and "arg1"
for $1, and print those
> +# out.
> +run "" sh/f arg1; success sh/f,"sh/f arg1"
> +
> +# In the tests below, the "file" parameter of execvpe() will not
contain a
> +# <slash> character.
> +#
> +# Show that PATH matters that way -- first, trigger an ENOENT in
> +# nbd_internal_execvpe_init() by setting PATH to the empty string.
> +run "" expr 1 + 1; init_fail ENOENT
Another spot that might need tweaking based on your reaction to my
comments on the previous patch.
> +
> +# Fall back to confstr(_CS_PATH) in nbd_internal_execvpe_init(), by
unsetting
> +# PATH. Verify the generated candidates by invoking "getconf
PATH" here, and
> +# appending "/expr" to each prefix.
> +expected_output=$(
> +    path=$(command -p getconf PATH)
We may have to check that getconf works, or skip the test if it
doesn't (POSIX says it should exist, but not all the world is POSIX).
Can be done as a followup if CI shows a failure.
> +    IFS=:
> +    for p in $path; do
> +        printf '%s/%s\n' $p expr
> +    done
> +    command -p expr 1 + 1
> +)
> +run _ expr 1 + 1; success "${expected_output//$'\n'/,}"
> +
> +# Continue with tests where the "file" parameter of execvpe()
does not contain a
> +# <slash> character, but now set PATH to explicit prefix lists.
> +#
> +# Show that, if the last candidate fails execve() with an error number
that
> +# would not be fatal otherwise, we do get that error number.
> +run empty:fifo:nxregf:symlink f
> +execve_fail empty/f,fifo/f,nxregf/f,symlink/f ELOOP
> +
> +# Put a single prefix in PATH, such that it lead to a successful
execution. This
leads
> +# exercises two things at the same time: (a) that
nbd_internal_execvpe_init()
> +# produces *one* candidate (i.e., that no <colon> is seen), and (b)
that
> +# nbd_internal_fork_safe_execvpe() succeeds for the *last* candidate.
Repeat the
> +# test with "expr" (called "f" under "bin")
and the shell script (called "f"
> +# under "sh", triggering the ENOEXEC branch).
> +run bin f 1 + 1; success bin/f,2
> +run sh  f arg1;  success sh/f,"sh/f arg1"
> +
> +# Demonstrate that the order of candidates matters. The first invocation
finds
> +# "expr" (called "f" under "bin"), the
second invocation finds the shell script
> +# under "sh" (triggering the ENOEXEC branch).
> +run empty:bin:sh f 1 + 1; success empty/f,bin/f,sh/f,2
> +run empty:sh:bin f arg1;  success empty/f,sh/f,bin/f,"sh/f arg1"
> +
> +# Check the expansion of zero-length prefixes in PATH to ".",
plus the
> +# (non-)insertion of the "/" separator.
> +run a/:       f;       execve_fail a/f,./f                 ENOENT
> +run :a/       f;       execve_fail ./f,a/f                 ENOENT
> +run :         f;       execve_fail ./f,./f                 ENOENT
> +pushd bin
> +run :         f 1 + 1; success     ./f,./f,2
> +popd
> +run :a/:::b/: f;       execve_fail ./f,a/f,./f,./f,b/f,./f ENOENT
Overall a nice unit test.

Reviewed-by: Eric Blake <eblake at redhat.com>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org