From: "Daniel P. Berrangé" <berrange@redhat.com>
To: qemu-devel@nongnu.org
Cc: "Jason Wang" <jasowang@redhat.com>,
"Daniel P. Berrangé" <berrange@redhat.com>
Subject: [PATCH 2/2] net/tap: fix error reporting when bridge helper forgets to send an FD
Date: Wed, 5 May 2021 13:12:36 +0100 [thread overview]
Message-ID: <20210505121236.532755-3-berrange@redhat.com> (raw)
In-Reply-To: <20210505121236.532755-1-berrange@redhat.com>
The recv_fd() method returns -1 on error, 0 on end of file, or an FD
number on success.
Technically 0 is also a valid FD number, so the return value is
ambiguous.
The caller doesn't even consider the possibility of 0 meaning end
of file, so just blindly assume it is a valid FD.
IOW if the bridge helper exits with 0 status code and forgets to
send an FD, QEMU will accidentally try to use STDIN as a TAP FD.
Fortunately we get an error shortly thereafter
$ qemu-system-x86_64 -netdev bridge,br=br99,helper=/bin/true,id=ns0
qemu-system-x86_64: -netdev bridge,br=br99,helper=/bin/true,id=ns0:
Unable to query TUNGETIFF on FD 0: Inappropriate ioctl for device
It is better if we correctly diagnose this broken bridge helper
though. To do this we need to return the FD in an output parameter
to remove the ambiguity and then fix the caller to check for the
end of file condition. With this done we now get
$ qemu-system-x86_64 -netdev bridge,br=br99,helper=/bin/true,id=ns0
qemu-system-x86_64: -netdev bridge,br=br99,helper=/bin/true,id=ns0:
bridge helper did not send a file descriptor
Fixes: https://gitlab.com/qemu-project/qemu/-/issues/166
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
net/tap.c | 32 ++++++++++++++++++++++----------
1 file changed, 22 insertions(+), 10 deletions(-)
diff --git a/net/tap.c b/net/tap.c
index 276a9077fc..02046c9fd2 100644
--- a/net/tap.c
+++ b/net/tap.c
@@ -450,9 +450,11 @@ static void launch_script(const char *setup_script, const char *ifname,
}
}
-static int recv_fd(int c)
+/*
+ * Returns: -1 on error, 0 on end of file, 1 if an FD was received
+ */
+static int recv_fd(int c, int *fd)
{
- int fd;
uint8_t msgbuf[CMSG_SPACE(sizeof(fd))];
struct msghdr msg = {
.msg_control = msgbuf,
@@ -476,12 +478,12 @@ static int recv_fd(int c)
msg.msg_iovlen = 1;
len = recvmsg(c, &msg, 0);
- if (len > 0) {
- memcpy(&fd, CMSG_DATA(cmsg), sizeof(fd));
- return fd;
+ if (len <= 0) {
+ return len;
}
- return len;
+ memcpy(fd, CMSG_DATA(cmsg), sizeof(*fd));
+ return 1;
}
static int net_bridge_run_helper(const char *helper, const char *bridge,
@@ -564,14 +566,15 @@ static int net_bridge_run_helper(const char *helper, const char *bridge,
_exit(1);
} else {
- int fd;
+ int ret;
+ int fd = -1;
int saved_errno;
close(sv[1]);
do {
- fd = recv_fd(sv[0]);
- } while (fd == -1 && errno == EINTR);
+ ret = recv_fd(sv[0], &fd);
+ } while (ret == -1 && errno == EINTR);
saved_errno = errno;
close(sv[0]);
@@ -580,7 +583,7 @@ static int net_bridge_run_helper(const char *helper, const char *bridge,
/* loop */
}
sigprocmask(SIG_SETMASK, &oldmask, NULL);
- if (fd < 0) {
+ if (ret < 0) {
error_setg_errno(errp, saved_errno,
"failed to recv file descriptor");
return -1;
@@ -589,6 +592,15 @@ static int net_bridge_run_helper(const char *helper, const char *bridge,
error_setg(errp, "bridge helper failed");
return -1;
}
+
+ /*
+ * ret == 0 means EOF, and if status == 0 then helper
+ * exited cleanly but forgot to send us an FD. Opps...
+ */
+ if (ret == 0) {
+ error_setg(errp, "bridge helper did not send a file descriptor");
+ return -1;
+ }
return fd;
}
}
--
2.31.1
next prev parent reply other threads:[~2021-05-05 12:20 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-05-05 12:12 [PATCH 0/2] net/tap: minor fixes to interaction with the bridge helper Daniel P. Berrangé
2021-05-05 12:12 ` [PATCH 1/2] net/tap: fix FreeBSD portability problem receiving TAP FD Daniel P. Berrangé
2021-05-05 12:12 ` Daniel P. Berrangé [this message]
2021-05-05 12:32 ` [PATCH 2/2] net/tap: fix error reporting when bridge helper forgets to send an FD Philippe Mathieu-Daudé
2021-05-12 15:36 ` Daniel P. Berrangé
2021-05-11 18:53 ` Connor Kuehl
2021-05-12 15:34 ` Daniel P. Berrangé
2021-05-11 18:54 ` [PATCH 0/2] net/tap: minor fixes to interaction with the bridge helper Connor Kuehl
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20210505121236.532755-3-berrange@redhat.com \
--to=berrange@redhat.com \
--cc=jasowang@redhat.com \
--cc=qemu-devel@nongnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.