[qemu-xen staging] net: detect errors from probing vnet hdr flag for TAP devices

[patchbot@xxxxxxx]

commit e7b347d0bf640adb1c998d317eaf44d2d7cbd973
Author:     Daniel P. Berrange <berrange@xxxxxxxxxx>
AuthorDate: Tue Jul 7 20:45:15 2020 +0200
Commit:     Jason Wang <jasowang@xxxxxxxxxx>
CommitDate: Wed Jul 15 21:00:13 2020 +0800

    net: detect errors from probing vnet hdr flag for TAP devices
    
    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 <dgilbert@xxxxxxxxxx>
    Signed-off-by: Daniel P. Berrange <berrange@xxxxxxxxxx>
    Tested-by: Dr. David Alan Gilbert <dgilbert@xxxxxxxxxx>
    Message-id: 20171027085548.3472-1-berrange@xxxxxxxxxx
    [lv: to simplify, don't check on EINVAL with TUNGETIFF as it exists since 
v2.6.27]
    Signed-off-by: Laurent Vivier <lvivier@xxxxxxxxxx>
    Signed-off-by: Jason Wang <jasowang@xxxxxxxxxx>
---
 net/tap-bsd.c     |  2 +-
 net/tap-linux.c   |  8 +++++---
 net/tap-solaris.c |  2 +-
 net/tap-stub.c    |  2 +-
 net/tap.c         | 25 ++++++++++++++++++++-----
 net/tap_int.h     |  2 +-
 6 files changed, 29 insertions(+), 12 deletions(-)

diff --git a/net/tap-bsd.c b/net/tap-bsd.c
index a5c3707f80..77aaf674b1 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 e0dd442ee3..b0635e9e32 100644
--- a/net/tap-linux.c
+++ b/net/tap-linux.c
@@ -147,13 +147,15 @@ 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;
+        /* TUNGETIFF is available since kernel v2.6.27 */
+        error_setg_errno(errp, errno,
+                         "Unable to query TUNGETIFF on FD %d", fd);
+        return -1;
     }
 
     return ifr.ifr_flags & IFF_VNET_HDR;
diff --git a/net/tap-solaris.c b/net/tap-solaris.c
index d03165c57c..0475a58207 100644
--- a/net/tap-solaris.c
+++ b/net/tap-solaris.c
@@ -207,7 +207,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-stub.c b/net/tap-stub.c
index a9ab8f8293..de525a2e69 100644
--- a/net/tap-stub.c
+++ b/net/tap-stub.c
@@ -37,7 +37,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.c b/net/tap.c
index 32e4813dbc..14dc904fca 100644
--- a/net/tap.c
+++ b/net/tap.c
@@ -598,7 +598,11 @@ int net_init_bridge(const Netdev *netdev, const char *name,
     }
 
     qemu_set_nonblock(fd);
-    vnet_hdr = tap_probe_vnet_hdr(fd);
+    vnet_hdr = tap_probe_vnet_hdr(fd, errp);
+    if (vnet_hdr < 0) {
+        close(fd);
+        return -1;
+    }
     s = net_tap_fd_init(peer, "bridge", name, fd, vnet_hdr);
 
     snprintf(s->nc.info_str, sizeof(s->nc.info_str), "helper=%s,br=%s", helper,
@@ -810,7 +814,11 @@ int net_init_tap(const Netdev *netdev, const char *name,
             return -1;
         }
 
-        vnet_hdr = tap_probe_vnet_hdr(fd);
+        vnet_hdr = tap_probe_vnet_hdr(fd, errp);
+        if (vnet_hdr < 0) {
+            close(fd);
+            return -1;
+        }
 
         net_init_tap_one(tap, peer, "tap", name, NULL,
                          script, downscript,
@@ -862,8 +870,11 @@ int net_init_tap(const Netdev *netdev, const char *name,
             }
 
             if (i == 0) {
-                vnet_hdr = tap_probe_vnet_hdr(fd);
-            } else if (vnet_hdr != tap_probe_vnet_hdr(fd)) {
+                vnet_hdr = tap_probe_vnet_hdr(fd, errp);
+                if (vnet_hdr < 0) {
+                    goto free_fail;
+                }
+            } else if (vnet_hdr != tap_probe_vnet_hdr(fd, NULL)) {
                 error_setg(errp,
                            "vnet_hdr not consistent across given tap fds");
                 ret = -1;
@@ -908,7 +919,11 @@ free_fail:
         }
 
         qemu_set_nonblock(fd);
-        vnet_hdr = tap_probe_vnet_hdr(fd);
+        vnet_hdr = tap_probe_vnet_hdr(fd, errp);
+        if (vnet_hdr < 0) {
+            close(fd);
+            return -1;
+        }
 
         net_init_tap_one(tap, peer, "bridge", name, ifname,
                          script, downscript, vhostfdname,
diff --git a/net/tap_int.h b/net/tap_int.h
index e3194b23f4..225a49ea48 100644
--- a/net/tap_int.h
+++ b/net/tap_int.h
@@ -34,7 +34,7 @@ int tap_open(char *ifname, int ifname_size, int *vnet_hdr,
 ssize_t tap_read_packet(int tapfd, uint8_t *buf, int maxlen);
 
 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);
 int tap_probe_vnet_hdr_len(int fd, int len);
 int tap_probe_has_ufo(int fd);
 void tap_fd_set_offload(int fd, int csum, int tso4, int tso6, int ecn, int 
ufo);
--
generated by git-patchbot for /home/xen/git/qemu-xen.git#staging