Re: [Libguestfs] [PATCH nbdkit 3/5] vddk: Miscellaneous improvements to reexec code.

Eric Blake
2020-06-02T14:22:49Z

On 6/2/20 7:27 AM, Richard W.M. Jones wrote:
> Use an extensible buffer (a vector<char>) when reading
> /proc/self/cmdline.
>
> Tidy up some error messages.
> ---
>   plugins/vddk/reexec.c | 57 ++++++++++++++++++++++++++-----------------
>   1 file changed, 35 insertions(+), 22 deletions(-)
>

> @@ -80,42 +95,40 @@ perform_reexec (const char *env, const char *prepend)
>      * until we get a short read.  This assumes nbdkit did not alter its
>      * original argv[].
>      */
> -  fd = open ("/proc/self/cmdline", O_RDONLY);
> +  fd = open (cmdline_file, O_RDONLY|O_CLOEXEC);
>     if (fd == -1) {
> -    nbdkit_debug ("failure to parse original argv: %m");
> +    nbdkit_debug ("open: %s: %m", cmdline_file);
>       return;
>     }
>
> -  do {
> -    char *p = realloc (buf, buflen * 2);
> +  for (;;) {
>       ssize_t r;
>
> -    if (!p) {
> -      nbdkit_debug ("failure to parse original argv: %m");
> +    if (buffer_reserve (&buf, 512) == -1) {
> +      nbdkit_debug ("realloc: %m");
>         return;
>       }

Pre-existing bug, which you did not fix here.  If we failed here, we are
leaking fd.  You slightly improved the situation by marking the leaked
fd O_CLOEXEC, but that really doesn't matter if we properly fix the code
to close(fd) before any early return, at which point the lifetime of fd
is only during single-threaded execution and O_CLOEXEC doesn't matter.

Rest of the patch looks fine.

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