[libvirt] [PATCH v7 10/19] utils: Implement function to pass a buffer to send via a fd to virCommand

[John Ferlan]

On 7/25/19 2:22 PM, Stefan Berger wrote:
> Implement virCommandSetSendBuffer() that allows the caller to pass a
> file descriptor and buffer to virCommand. virCommand will write the
> buffer into the file descriptor. That file descriptor could be the
> write end of a pipe or one of the file descriptors of a socketpair.
> The other file descriptor should be passed to the launched process to
> read the data from.
> 
> Only implement the function to allocate memory for send buffers
> and to free them later on.
> 
> Signed-off-by: Stefan Berger <stefanb at linux.ibm.com>
> Reviewed-by: Daniel P. Berrangé <berrange at redhat.com>
> ---
>  src/libvirt_private.syms |  1 +
>  src/util/vircommand.c    | 93 ++++++++++++++++++++++++++++++++++++++++
>  src/util/vircommand.h    |  5 +++
>  3 files changed, 99 insertions(+)
> 

[...]

> +/**
> + * virCommandSetSendBuffer
> + * @cmd: the command to modify
> + *
> + * Pass a buffer to virCommand that will be written into the
> + * given file descriptor. The buffer will be freed automatically
> + * and the file descriptor closed.
> + */
> +#if defined(F_SETFL)
> +int
> +virCommandSetSendBuffer(virCommandPtr cmd,
> +                        int fd,
> +                        unsigned char *buffer, size_t buflen)
> +{
> +    size_t i = virCommandGetNumSendBuffers(cmd);

This call would deref @cmd before the following check for !cmd

[1] Was found by Coverity, but see below

> +
> +    if (!cmd || cmd->has_error)
> +        return -1;
> +
> +    if (fcntl(fd, F_SETFL, O_NONBLOCK) < 0) {
> +        virReportSystemError(errno, "%s",
> +                             _("fcntl failed to set O_NONBLOCK"));
> +        cmd->has_error = errno;
> +        return -1;
> +    }
> +
> +    if (VIR_REALLOC_N(cmd->sendBuffers, i + 1) < 0) {
> +        cmd->has_error = ENOMEM;
> +        return -1;
> +    }
> +
> +    cmd->sendBuffers[i].fd = fd;
> +    cmd->sendBuffers[i].buffer = buffer;
> +    cmd->sendBuffers[i].buflen = buflen;
> +    cmd->sendBuffers[i].offset = 0;
> +
> +    cmd->numSendBuffers++;
> +
> +    return 0;
> +}
> +
> +#else /* !defined(F_SETFL) */
> +
> +int
> +virCommandSetSendBuffer(virCommandPtr cmd,
> +                        int fd,
> +                        unsigned char *buffer, size_t buflen)
> +{
> +    if (!cmd || cmd->has_error)
> +        return -1;
> +
> +    cmd->has_error = ENOTSUP;
> +
> +    return -1;
> +}
> +
> +#endif
> +
>  /**
>   * virCommandSetInputBuffer:
>   * @cmd: the command to modify
> @@ -2867,6 +2958,8 @@ virCommandFree(virCommandPtr cmd)
>      VIR_FREE(cmd->appArmorProfile);
>  #endif
>  
> +    virCommandFreeSendBuffers(cmd);
> +
>      VIR_FREE(cmd);
>  }
>  
> diff --git a/src/util/vircommand.h b/src/util/vircommand.h
> index 2a9ee5cdc7..c2abc7b2c3 100644
> --- a/src/util/vircommand.h
> +++ b/src/util/vircommand.h
> @@ -146,6 +146,11 @@ void virCommandAddArgList(virCommandPtr cmd,
>  void virCommandSetWorkingDirectory(virCommandPtr cmd,
>                                     const char *pwd) ATTRIBUTE_NONNULL(2);
>  
> +int virCommandSetSendBuffer(virCommandPtr cmd,
> +                            int fd,
> +                            unsigned char *buffer, size_t buflen)
> +    ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(3);

The ATTRIBUTE_NONNULL(1) causes a Coverity build error when the function
checks !cmd - so either this is removed or the function doesn't check
for !cmd...  If going with the latter, then both halves of the "#if
defined(F_SETFL)" would need to be changed.

John

> +
>  void virCommandSetInputBuffer(virCommandPtr cmd,
>                                const char *inbuf) ATTRIBUTE_NONNULL(2);
>  
>