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

[Richard W.M. Jones]

On Tue, Jun 02, 2020 at 09:22:49AM -0500, Eric Blake wrote:
> 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.

I was wondering if we should define conditions where we want reexec to
be skipped (probably if /proc/self/... files don't exist, since it's
likely that it isn't a "sufficiently Linux-like" platform).  We would
hard fail for all the other errors such as buffer_reserve failing
above.  What do you think?

Note in the next patch I actually did do some hard failing for
operations that shouldn't fail and are hard to recover from.

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