[Libguestfs] [PATCH] Enable new-style -chardev ... guestfwd command line

[Richard W.M. Jones]

On Fri, Sep 18, 2009 at 04:28:58PM +0200, Jim Meyering wrote:
> Richard W.M. Jones wrote:
> > This also changes the name of the "vmchannel" buffer to just "buf",
> > reflecting the fact that it's just used as a temporary buffer, and
> > that the word vmchannel is overloaded.
> ...
> 
> Looks good.
> 
> > -//#define GUESTFWD_ADDR "10.0.2.4"
> > +#define GUESTFWD_ADDR "10.0.2.4"
> >
> >  /* GuestFS handle and connection. */
> >  enum state { CONFIG, LAUNCHING, READY, BUSY, NO_HANDLE };
> > @@ -983,7 +983,7 @@ guestfs__launch (guestfs_h *g)
> >    }
> >
> >    if (r == 0) {			/* Child (qemu). */
> > -    char vmchannel[256];
> > +    char buf[256];
> 
> Is 256 guaranteed to be enough?  I.e., who controls "unixsock"?
> If not, failing snprintf will leave you with a truncated command string.

If $TMPDIR is set, then it would be possible to create a unixsock
string which is arbitrarily long.  It's not a security issue, but it
would result in a truncated string.  Of course TMPDIR would have to be
unreasonably long.

> ...
> > +      snprintf (buf, sizeof buf,
> > +                "socket,id=guestfsvmc,path=%s,server,nowait", unixsock);
> > +
> > +      add_cmdline (g, "-chardev");
> > +      add_cmdline (g, buf);
> > +
> > +      snprintf (buf, sizeof buf,
> > +                "user,vlan=0,net=10.0.2.0/8,"
> > +                "guestfwd=tcp:%s:%d-chardev:guestfsvmc",
> > +                GUESTFWD_ADDR, GUESTFWD_PORT);
> ...
> >        add_cmdline (g, "user,vlan=0,net=10.0.2.0/8");
> 
> Note the two hard-coded net strings, "10.0.2.0/8".
> It'd be nice to factor those out.
> They could even be derived from the value of GUESTFWD_ADDR.

Yes we could factor these out.  If we removed them entirely, qemu
would default to 10.0.2.0/8, but I don't trust the qemu developers not
to change things randomly in future.

I'm still trying to work out if we can remove the need for vmchannel
entirely.  So far I have an implementation which could work, but it
has an enormous, glaring security hole in the middle ...

Rich.

-- 
Richard Jones, Emerging Technologies, Red Hat  http://et.redhat.com/~rjones
New in Fedora 11: Fedora Windows cross-compiler. Compile Windows
programs, test, and build Windows installers. Over 70 libraries supprt'd
http://fedoraproject.org/wiki/MinGW http://www.annexia.org/fedora_mingw