On Mon, Oct 30, 2017 at 03:37:33PM +0800, Jason Wang wrote:
>
>
> On 2017年10月27日 16:55, Daniel P. Berrange wrote:
> > When QEMU sets up a tap based network device backend, it mostly ignores
> > errors
> > reported from various ioctl() calls it makes, assuming the TAP file
> > descriptor
> > is valid. This assumption can easily be violated when the user is passing
> > in a
> > pre-opened file descriptor. At best, the ioctls may fail with a -EBADF, but
> > if
> > the user passes in a bogus FD number that happens to clash with a FD number
> > that
> > QEMU has opened internally for another reason, a wide variety of errnos may
> > result, as the TUNGETIFF ioctl number may map to a completely different
> > command
> > on a different type of file.
> >
> > By ignoring all these errors, QEMU sets up a zombie network backend that
> > will
> > never pass any data. Even worse, when QEMU shuts down, or that network
> > backend
> > is hot-removed, it will close this bogus file descriptor, which could
> > belong to
> > another QEMU device backend.
> >
> > There's no obvious guaranteed reliable way to detect that a FD genuinely is
> > a
> > TAP device, as opposed to a UNIX socket, or pipe, or something else.
> > Checking
> > the errno from probing vnet hdr flag though, does catch the big common
> > cases.
> > ie calling TUNGETIFF will return EBADF for an invalid FD, and ENOTTY when
> > FD is
> > a UNIX socket, or pipe which catches accidental collisions with FDs used for
> > stdio, or monitor socket.
> >
> > Previously the example below where bogus fd 9 collides with the FD used for
> > the
> > chardev saw:
> >
> > $ ./x86_64-softmmu/qemu-system-x86_64 -netdev tap,id=hostnet0,fd=9 \
> > -chardev socket,id=charchannel0,path=/tmp/qga,server,nowait \
> > -monitor stdio -vnc :0
> > qemu-system-x86_64: -netdev tap,id=hostnet0,fd=9: TUNGETIFF ioctl() failed:
> > Inappropriate ioctl for device
> > TUNSETOFFLOAD ioctl() failed: Bad address
> > QEMU 2.9.1 monitor - type 'help' for more information
> > (qemu) Warning: netdev hostnet0 has no peer
> >
> > which gives a running QEMU with a zombie network backend.
> >
> > With this change applied we get an error message and QEMU immediately exits
> > before carrying on and making a bigger disaster:
> >
> > $ ./x86_64-softmmu/qemu-system-x86_64 -netdev tap,id=hostnet0,fd=9 \
> > -chardev socket,id=charchannel0,path=/tmp/qga,server,nowait \
> > -monitor stdio -vnc :0
> > qemu-system-x86_64: -netdev tap,id=hostnet0,vhost=on,fd=9: Unable to query
> > TUNGETIFF on FD 9: Inappropriate ioctl for device
> >
> > Reported-by: Dr. David Alan Gilbert <dgilb...@redhat.com>
> > Signed-off-by: Daniel P. Berrange <berra...@redhat.com>
> > ---
> > net/tap-bsd.c | 2 +-
> > net/tap-linux.c | 12 +++++++++---
> > net/tap-solaris.c | 2 +-
> > net/tap-stub.c | 2 +-
> > net/tap.c | 25 ++++++++++++++++++++-----
> > net/tap_int.h | 2 +-
> > 6 files changed, 33 insertions(+), 12 deletions(-)
> >
> > diff --git a/net/tap-bsd.c b/net/tap-bsd.c
> > index 6c9692263d..4f1d633b08 100644
> > --- a/net/tap-bsd.c
> > +++ b/net/tap-bsd.c
> > @@ -211,7 +211,7 @@ void tap_set_sndbuf(int fd, const NetdevTapOptions
> > *tap, Error **errp)
> > {
> > }
> > -int tap_probe_vnet_hdr(int fd)
> > +int tap_probe_vnet_hdr(int fd, Error **errp)
> > {
> > return 0;
> > }
> > diff --git a/net/tap-linux.c b/net/tap-linux.c
> > index 535b1ddb61..de74928407 100644
> > --- a/net/tap-linux.c
> > +++ b/net/tap-linux.c
> > @@ -147,13 +147,19 @@ void tap_set_sndbuf(int fd, const NetdevTapOptions
> > *tap, Error **errp)
> > }
> > }
> > -int tap_probe_vnet_hdr(int fd)
> > +int tap_probe_vnet_hdr(int fd, Error **errp)
> > {
> > struct ifreq ifr;
> > if (ioctl(fd, TUNGETIFF, &ifr) != 0) {
> > - error_report("TUNGETIFF ioctl() failed: %s", strerror(errno));
> > - return 0;
> > + /* Kernel pre-dates TUNGETIFF support */
> > + if (errno == -EINVAL) {
> > + return 0;
>
> This looks still unsafe, e.g some other device may return -EINVAL too.
>
> Is this better to check stat.st_rdev through fstat()?
Hmm, yes, that might be possible. Let me investigate...
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
