[Libguestfs] [nbdkit PATCH] sh: Don't let child inherit SIGPIPE ignored

[Richard W.M. Jones]

On Sat, Dec 01, 2018 at 04:16:12PM -0600, Eric Blake wrote:
> While nbdkit itself must run with SIGPIPE ignored, many applications
> expect to inherit SIGPIPE in the default state. What's worse, POSIX
> states that a non-interactive shell script cannot use 'trap' to
> undo an inherited SIG_IGN on SIGPIPE.  I have seen several bug
> reports over the years of something that works for a developer but
> fails under a CI environment, where the root cause was the CI
> leaking an ignored SIGPIPE into the file under test.
> 
> Testing this proved to be the more interesting part of the patch.
> 'yes' is probably the easiest way to generate lots of data and
> which behaves differently if SIGPIPE is ignored, but I'm not
> certain if coreutils is always installed on BSD machines.  Also
> fix a missing redirect to stderr in the probe for $tmpdir.
> 
> Signed-off-by: Eric Blake <eblake at redhat.com>
> ---
>  plugins/sh/call.c   |  3 +++
>  tests/test-shell.sh | 14 +++++++++++++-
>  2 files changed, 16 insertions(+), 1 deletion(-)
> 
> diff --git a/plugins/sh/call.c b/plugins/sh/call.c
> index 9b3eca8..42923da 100644
> --- a/plugins/sh/call.c
> +++ b/plugins/sh/call.c
> @@ -121,6 +121,9 @@ call3 (const char *wbuf, size_t wbuflen, /* sent to stdin */
>      close (out_fd[1]);
>      close (err_fd[1]);
> 
> +    /* Restore SIGPIPE back to SIG_DFL, since shell can't undo SIG_IGN */
> +    signal (SIGPIPE, SIG_DFL);
> +
>      /* Set $tmpdir for the script. */
>      setenv ("tmpdir", tmpdir, 1);
> 
> diff --git a/tests/test-shell.sh b/tests/test-shell.sh
> index ef438ec..eabd4fe 100755
> --- a/tests/test-shell.sh
> +++ b/tests/test-shell.sh
> @@ -11,10 +11,22 @@ fi
> 
>  # nbdkit is supposed to set $tmpdir.  If it doesn't, it's an error.
>  if [ ! -d $tmpdir ]; then
> -    echo "\$tmpdir was not set"
> +    echo "\$tmpdir was not set" >&2
>      exit 1
>  fi
> 
> +# Check that SIGPIPE is not ignored unless we want it that way.
> +if (type yes) >/dev/null 2>&1; then
> +    ignored=$(trap "" 13;
> +	      ({ yes; echo $? >&3; } | head -c1) 3>&1 >/dev/null 2>&1)
> +    default=$(trap - 13;
> +	      ({ yes; echo $? >&3; } | head -c1) 3>&1 >/dev/null 2>&1)
> +    if [ $ignored = $default ]; then
> +	echo "SIGPIPE was inherited ignored" >&2
> +	exit 1;
> +    fi
> +fi
> +
>  case "$1" in
>      open)
>          echo handle
> -- 
> 2.17.2

ACK

SIGPIPE leakage is always a nasty problem.  I've noticed this one
before in Python, and we had a similar problem (SIGCHLD leaking)
recently found in collectd.

Thanks,

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-p2v converts physical machines to virtual machines.  Boot with a
live CD or over the network (PXE) and turn machines into KVM guests.
http://libguestfs.org/virt-v2v