[Libguestfs] [PATCH nbdkit 3/5] vddk: Miscellaneous improvements to reexec code.
Richard W.M. Jones
rjones at redhat.com
Tue Jun 2 14:33:47 UTC 2020
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
More information about the Libguestfs
mailing list