Re: [Libguestfs] [PATCH nbdkit 2/3] server: Disallow -FD for stdin/stdout/stderr.

Eric Blake
2020-06-01T17:20:24Z

On 6/1/20 5:31 AM, Richard W.M. Jones wrote:
>    $ ./nbdkit ssh host=localhost /nosuchfile password=-0 --run
'qemu-img info $nbd'
>    abc
>    fcntl: Bad file descriptor
>
> The reason for this is that we close the file descriptor after reading
> the password.  Closing stdin causes bad stuff to happen.
> ---
>   docs/nbdkit-plugin.pod | 5 +++++
>   server/public.c        | 4 ++--
>   2 files changed, 7 insertions(+), 2 deletions(-)
>

Makes sense.

> diff --git a/docs/nbdkit-plugin.pod b/docs/nbdkit-plugin.pod
> index 612688ab..7b8a5393 100644
> --- a/docs/nbdkit-plugin.pod
> +++ b/docs/nbdkit-plugin.pod
> @@ -1249,6 +1249,11 @@ passed in a file.
>
>   C<password=-> can only be used when stdin is a terminal.
>
> +C<password=-FD> cannot be used with stdin, stdout or stderr
> +(ie. C<-0>, C<-1> or C<-2>).  The reason is that after
reading the
> +password the file descriptor is closed, which causes bad stuff to
> +happen.

Sure, we could always skip closing on those three, but it's just as easy
to not permit it in the first place :)

> +
>   =head2 Safely interacting with stdin and stdout
>
>    int nbdkit_stdio_safe (void);
> diff --git a/server/public.c b/server/public.c
> index dafdfbae..2e36e43a 100644
> --- a/server/public.c
> +++ b/server/public.c
> @@ -433,8 +433,8 @@ nbdkit_read_password (const char *value, char
**password)
>
>       if (nbdkit_parse_int ("password file descriptor",
&value[1], &fd) == -1)
>         return -1;
> -    if (fd == STDIN_FILENO && !nbdkit_stdio_safe ()) {
> -      nbdkit_error ("stdin is not available for reading
password");
> +    if (fd == STDIN_FILENO || fd == STDOUT_FILENO || fd == STDERR_FILENO)
{

Could shorten to if (fd <= STDERR_FILENO) if desired, but I'm fine
either way.

ACK.

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