Skip to content

Commit e7b347d

Browse files
berrangejasowang
authored andcommitted
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 <[email protected]> Signed-off-by: Daniel P. Berrange <[email protected]> Tested-by: Dr. David Alan Gilbert <[email protected]> Message-id: [email protected] [lv: to simplify, don't check on EINVAL with TUNGETIFF as it exists since v2.6.27] Signed-off-by: Laurent Vivier <[email protected]> Signed-off-by: Jason Wang <[email protected]>
1 parent 894022e commit e7b347d

File tree

6 files changed

+29
-12
lines changed

6 files changed

+29
-12
lines changed

net/tap-bsd.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -211,7 +211,7 @@ void tap_set_sndbuf(int fd, const NetdevTapOptions *tap, Error **errp)
211211
{
212212
}
213213

214-
int tap_probe_vnet_hdr(int fd)
214+
int tap_probe_vnet_hdr(int fd, Error **errp)
215215
{
216216
return 0;
217217
}

net/tap-linux.c

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -147,13 +147,15 @@ void tap_set_sndbuf(int fd, const NetdevTapOptions *tap, Error **errp)
147147
}
148148
}
149149

150-
int tap_probe_vnet_hdr(int fd)
150+
int tap_probe_vnet_hdr(int fd, Error **errp)
151151
{
152152
struct ifreq ifr;
153153

154154
if (ioctl(fd, TUNGETIFF, &ifr) != 0) {
155-
error_report("TUNGETIFF ioctl() failed: %s", strerror(errno));
156-
return 0;
155+
/* TUNGETIFF is available since kernel v2.6.27 */
156+
error_setg_errno(errp, errno,
157+
"Unable to query TUNGETIFF on FD %d", fd);
158+
return -1;
157159
}
158160

159161
return ifr.ifr_flags & IFF_VNET_HDR;

net/tap-solaris.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -207,7 +207,7 @@ void tap_set_sndbuf(int fd, const NetdevTapOptions *tap, Error **errp)
207207
{
208208
}
209209

210-
int tap_probe_vnet_hdr(int fd)
210+
int tap_probe_vnet_hdr(int fd, Error **errp)
211211
{
212212
return 0;
213213
}

net/tap-stub.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ void tap_set_sndbuf(int fd, const NetdevTapOptions *tap, Error **errp)
3737
{
3838
}
3939

40-
int tap_probe_vnet_hdr(int fd)
40+
int tap_probe_vnet_hdr(int fd, Error **errp)
4141
{
4242
return 0;
4343
}

net/tap.c

Lines changed: 20 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -598,7 +598,11 @@ int net_init_bridge(const Netdev *netdev, const char *name,
598598
}
599599

600600
qemu_set_nonblock(fd);
601-
vnet_hdr = tap_probe_vnet_hdr(fd);
601+
vnet_hdr = tap_probe_vnet_hdr(fd, errp);
602+
if (vnet_hdr < 0) {
603+
close(fd);
604+
return -1;
605+
}
602606
s = net_tap_fd_init(peer, "bridge", name, fd, vnet_hdr);
603607

604608
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,
810814
return -1;
811815
}
812816

813-
vnet_hdr = tap_probe_vnet_hdr(fd);
817+
vnet_hdr = tap_probe_vnet_hdr(fd, errp);
818+
if (vnet_hdr < 0) {
819+
close(fd);
820+
return -1;
821+
}
814822

815823
net_init_tap_one(tap, peer, "tap", name, NULL,
816824
script, downscript,
@@ -862,8 +870,11 @@ int net_init_tap(const Netdev *netdev, const char *name,
862870
}
863871

864872
if (i == 0) {
865-
vnet_hdr = tap_probe_vnet_hdr(fd);
866-
} else if (vnet_hdr != tap_probe_vnet_hdr(fd)) {
873+
vnet_hdr = tap_probe_vnet_hdr(fd, errp);
874+
if (vnet_hdr < 0) {
875+
goto free_fail;
876+
}
877+
} else if (vnet_hdr != tap_probe_vnet_hdr(fd, NULL)) {
867878
error_setg(errp,
868879
"vnet_hdr not consistent across given tap fds");
869880
ret = -1;
@@ -908,7 +919,11 @@ int net_init_tap(const Netdev *netdev, const char *name,
908919
}
909920

910921
qemu_set_nonblock(fd);
911-
vnet_hdr = tap_probe_vnet_hdr(fd);
922+
vnet_hdr = tap_probe_vnet_hdr(fd, errp);
923+
if (vnet_hdr < 0) {
924+
close(fd);
925+
return -1;
926+
}
912927

913928
net_init_tap_one(tap, peer, "bridge", name, ifname,
914929
script, downscript, vhostfdname,

net/tap_int.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ int tap_open(char *ifname, int ifname_size, int *vnet_hdr,
3434
ssize_t tap_read_packet(int tapfd, uint8_t *buf, int maxlen);
3535

3636
void tap_set_sndbuf(int fd, const NetdevTapOptions *tap, Error **errp);
37-
int tap_probe_vnet_hdr(int fd);
37+
int tap_probe_vnet_hdr(int fd, Error **errp);
3838
int tap_probe_vnet_hdr_len(int fd, int len);
3939
int tap_probe_has_ufo(int fd);
4040
void tap_fd_set_offload(int fd, int csum, int tso4, int tso6, int ecn, int ufo);

0 commit comments

Comments
 (0)