[libvirt] [PATCH v2 2/2] BSD: implement virNetDevTapCreate() and virNetDevTapDelete()

[Roman Bogorodskiy]

  Eric Blake wrote:

> On 05/25/2013 09:24 AM, Roman Bogorodskiy wrote:
> > Implementation uses SIOCIFCREATE2 and SIOCIFDESTROY ioctls.
> > ---
> >  src/util/virnetdevtap.c | 116 +++++++++++++++++++++++++++++++++++++++++++++++-
> >  1 file changed, 114 insertions(+), 2 deletions(-)
> > 
> 
> > +    /* In case we were given exact interface name (e.g. 'vnetN'),
> > +     * we just rename to it. If we have format string like
> > +     * 'vnet%d', we need to find the first available name that
> > +     * matches this pattern
> > +     */
> > +    if (strstr(*ifname, "%d") != NULL) {
> > +        int i;
> > +        for (i = 0; i <= IF_MAXUNIT; i++) {
> > +            char *newname;
> > +            if (virAsprintf(&newname, *ifname, i) < 0) {
> 
> Are we POSITIVE that this string is sanitized to have exactly one %d, or
> is there a risk that *ifname is user-supplied and can be used to exploit us?

ifname could be supplied by user, but if I understood code correctly,
it's checked that:

 * user supplied ifname doesn't start with auto-generated prefix ('vnet')
 * user supplied ifname doesn't have '%' in it

otherwise we fallback to the auto-generated name (prefix + "%d").

Relevant code pieces:

qemu/qemu_command.c:339
uml/uml_conf.c:114

> > +                virReportOOMError();
> > +                goto cleanup;
> > +            }
> > +
> > +            if (virNetDevExists(newname) == 0) {
> > +                newifname = newname;
> > +                break;
> > +            }
> > +        }
> 
> Memory leak if you go through the loop more than once.  You must free
> newname on each failed iteration.

Will fix.

> The rest of the code compiles fine on my FreeBSD VM, although I'm not
> sure how to test it.  It looks like a fixed version would make sense for
> 1.0.7 (although you may need someone else to step in and commit if I'm
> still on my vacation at the time we hit code freeze).

Well, I test it with domain definition like that:

https://dpaste.de/5jK67/

Networking obviously is not functional, but it's enough to test device
creation.

Roman Bogorodskiy
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20130606/46d60632/attachment-0001.sig>