From 6a6735ece442298e000de43f5325842a442c263d Mon Sep 17 00:00:00 2001 From: David Gibson Date: Fri, 11 Aug 2023 15:12:22 +1000 Subject: [PATCH] epoll: Always use epoll_ref for the epoll data variable epoll_ref contains a variety of information useful when handling epoll events on our sockets, and we place it in the epoll_event data field returned by epoll. However, for a few other things we use the 'fd' field in the standard union of types for that data field. This actually introduces a bug which is vanishingly unlikely to hit in practice, but very nasty if it ever did: theoretically if we had a very large file descriptor number for fd_tap or fd_tap_listen it could overflow into bits that overlap with the 'proto' field in epoll_ref. With some very bad luck this could mean that we mistakenly think an event on a regular socket is an event on fd_tap or fd_tap_listen. More practically, using different (but overlapping) fields of the epoll_data means we can't unify dispatch for the various different objects in the epoll. Therefore use the same epoll_ref as the data for the tap fds and the netns quit fd, adding new fd type values to describe them. Signed-off-by: David Gibson Signed-off-by: Stefano Brivio --- passt.c | 11 ++++++----- passt.h | 6 +++++- pasta.c | 8 ++++++-- tap.c | 16 ++++++++++++---- 4 files changed, 29 insertions(+), 12 deletions(-) diff --git a/passt.c b/passt.c index ae69478..45e9fbf 100644 --- a/passt.c +++ b/passt.c @@ -60,6 +60,8 @@ char *epoll_type_str[EPOLL_TYPE_MAX + 1] = { [EPOLL_TYPE_UDP] = "UDP socket", [EPOLL_TYPE_ICMP] = "ICMP socket", [EPOLL_TYPE_ICMPV6] = "ICMPv6 socket", + [EPOLL_TYPE_NSQUIT] = "namespace inotify", + [EPOLL_TYPE_TAP] = "tap device", }; /** @@ -328,12 +330,11 @@ loop: for (i = 0; i < nfds; i++) { union epoll_ref ref = *((union epoll_ref *)&events[i].data.u64); - int fd = events[i].data.fd; - if (fd == c.fd_tap || fd == c.fd_tap_listen) - tap_handler(&c, fd, events[i].events, &now); - else if (fd == quit_fd) - pasta_netns_quit_handler(&c, fd); + if (ref.type == EPOLL_TYPE_TAP) + tap_handler(&c, ref.fd, events[i].events, &now); + else if (ref.type == EPOLL_TYPE_NSQUIT) + pasta_netns_quit_handler(&c, quit_fd); else sock_handler(&c, ref, events[i].events, &now); } diff --git a/passt.h b/passt.h index 2110781..8878a11 100644 --- a/passt.h +++ b/passt.h @@ -55,8 +55,12 @@ enum epoll_type { EPOLL_TYPE_ICMP, /* ICMPv6 sockets */ EPOLL_TYPE_ICMPV6, + /* inotify fd watching for end of netns (pasta) */ + EPOLL_TYPE_NSQUIT, + /* tap char device, or qemu socket fd */ + EPOLL_TYPE_TAP, - EPOLL_TYPE_MAX = EPOLL_TYPE_ICMPV6, + EPOLL_TYPE_MAX = EPOLL_TYPE_TAP, }; /** diff --git a/pasta.c b/pasta.c index 1e84680..dbe8e8c 100644 --- a/pasta.c +++ b/pasta.c @@ -365,7 +365,10 @@ void pasta_ns_conf(struct ctx *c) int pasta_netns_quit_init(struct ctx *c) { int flags = O_NONBLOCK | O_CLOEXEC; - struct epoll_event ev = { .events = EPOLLIN }; + union epoll_ref ref = { .type = EPOLL_TYPE_NSQUIT }; + struct epoll_event ev = { + .events = EPOLLIN + }; int inotify_fd; if (c->mode != MODE_PASTA || c->no_netns_quit || !*c->netns_base) @@ -381,7 +384,8 @@ int pasta_netns_quit_init(struct ctx *c) return -1; } - ev.data.fd = inotify_fd; + ref.fd = inotify_fd; + ev.data.u64 = ref.u64; epoll_ctl(c->epollfd, EPOLL_CTL_ADD, inotify_fd, &ev); return inotify_fd; diff --git a/tap.c b/tap.c index d64d2a1..32ddae2 100644 --- a/tap.c +++ b/tap.c @@ -1071,6 +1071,7 @@ restart: static void tap_sock_unix_init(struct ctx *c) { int fd = socket(AF_UNIX, SOCK_STREAM, 0); + union epoll_ref ref = { .type = EPOLL_TYPE_TAP }; struct epoll_event ev = { 0 }; struct sockaddr_un addr = { .sun_family = AF_UNIX, @@ -1123,8 +1124,9 @@ static void tap_sock_unix_init(struct ctx *c) listen(fd, 0); - ev.data.fd = c->fd_tap_listen = fd; + ref.fd = c->fd_tap_listen = fd; ev.events = EPOLLIN | EPOLLET; + ev.data.u64 = ref.u64; epoll_ctl(c->epollfd, EPOLL_CTL_ADD, c->fd_tap_listen, &ev); info("You can now start qemu (>= 7.2, with commit 13c6be96618c):"); @@ -1141,6 +1143,7 @@ static void tap_sock_unix_init(struct ctx *c) */ static void tap_sock_unix_new(struct ctx *c, uint32_t events) { + union epoll_ref ref = { .type = EPOLL_TYPE_TAP }; struct epoll_event ev = { 0 }; int v = INT_MAX / 2; struct ucred ucred; @@ -1180,8 +1183,9 @@ static void tap_sock_unix_new(struct ctx *c, uint32_t events) setsockopt(c->fd_tap, SOL_SOCKET, SO_SNDBUF, &v, sizeof(v))) trace("tap: failed to set SO_SNDBUF to %i", v); - ev.data.fd = c->fd_tap; + ref.fd = c->fd_tap; ev.events = EPOLLIN | EPOLLET | EPOLLRDHUP; + ev.data.u64 = ref.u64; epoll_ctl(c->epollfd, EPOLL_CTL_ADD, c->fd_tap, &ev); } @@ -1226,6 +1230,7 @@ static int tap_ns_tun(void *arg) */ static void tap_sock_tun_init(struct ctx *c) { + union epoll_ref ref = { .type = EPOLL_TYPE_TAP }; struct epoll_event ev = { 0 }; NS_CALL(tap_ns_tun, c); @@ -1234,8 +1239,9 @@ static void tap_sock_tun_init(struct ctx *c) pasta_ns_conf(c); - ev.data.fd = c->fd_tap; + ref.fd = c->fd_tap; ev.events = EPOLLIN | EPOLLRDHUP; + ev.data.u64 = ref.u64; epoll_ctl(c->epollfd, EPOLL_CTL_ADD, c->fd_tap, &ev); } @@ -1257,11 +1263,13 @@ void tap_sock_init(struct ctx *c) } if (c->fd_tap != -1) { /* Passed as --fd */ + union epoll_ref ref = { .type = EPOLL_TYPE_TAP }; struct epoll_event ev = { 0 }; ASSERT(c->one_off); - ev.data.fd = c->fd_tap; + ref.fd = c->fd_tap; ev.events = EPOLLIN | EPOLLET | EPOLLRDHUP; + ev.data.u64 = ref.u64; epoll_ctl(c->epollfd, EPOLL_CTL_ADD, c->fd_tap, &ev); return; }