- dev - passt

[PATCH] conf, util: Disable IPv6 if explicit IPv6 socket probe fails
by Stefano Brivio 09 Jun '26

09 Jun '26

In https://bugs.passt.top/show_bug.cgi?id=188, I originally reported that if IPv6 is disabled in the kernel (for example via command line parameter ipv6.disable=1, or disabled in build configuration), and we attempt to forward any port, we'll exit right away after failing to set up dual-stack listening sockets. The original instance of that issue is now fixed for pasta by commit 75dcbc300bf0 ("pasta: Warn, disable matching IP version if not supported, in local mode") together with the new implementation of the rule forwarding table, starting from commit b223bec48213 ("fwd, tcp, udp: Set up listening sockets based on forward table"), because we first parse forwarding options, then probe for IPv6 support in the target namespace (and disable IPv6 as a result), and finally bind sockets once we already know that IPv6 support is disabled. But we don't do that when invoked as passt, because we have no target namespace and hence no probing for IPv6 support whatsoever. Add IPv6 to the socket features we test in sock_probe_features(), and, if we fail to create an IPv6 socket for whatever reason (which might include security policies as well), disable IPv6 support altogether, so that we won't attempt to use dual-stack sockets for port forwarding either. Note that the probe comes without any sort of debug message, because at this point we haven't parsed the configuration yet, and we would therefore print that regardless of the selected logging level and other options, including --ipv4-only, which would be rather confusing. I doubt we'll miss this kind of message though, IPv6 support being disabled is anyway obvious from the initial configuration dump. Reported-by: Chi Cuong HA <ChiCuong.HA(a)amadeus.com> Reported-by: Romain Geissler <romain.geissler(a)amadeus.com> Link: https://bugs.passt.top/show_bug.cgi?id=188 Fixes: 4ddd59bc6085 ("conf: Separate local mode for each IP version, don't enable disabled IP version") Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com> --- conf.c | 7 ++++++- passt.h | 2 ++ util.c | 8 ++++++++ 3 files changed, 16 insertions(+), 1 deletion(-) diff --git a/conf.c b/conf.c index 6f86940..cd05adf 100644 --- a/conf.c +++ b/conf.c @@ -1254,7 +1254,9 @@ void conf(struct ctx *c, int argc, char **argv) int name, ret; uid_t uid; gid_t gid; - + + if (c->no_ipv6) + v4_only = true; if (c->mode == MODE_PASTA) c->no_dhcp_dns = c->no_dhcp_dns_search = 1; @@ -1682,6 +1684,9 @@ void conf(struct ctx *c, int argc, char **argv) v6_only = false; break; case '6': + if (c->no_ipv6) + die("IPv6 not available but --ipv6-only given"); + v6_only = true; v4_only = false; break; diff --git a/passt.h b/passt.h index 1726965..c5f51d1 100644 --- a/passt.h +++ b/passt.h @@ -211,6 +211,7 @@ struct ip6_ctx { * @low_wmem: Low probed net.core.wmem_max * @low_rmem: Low probed net.core.rmem_max * @no_bindtodevice: Unprivileged SO_BINDTODEVICE not available + * @no_ipv6: IPv6 sockets not available * @vdev: vhost-user device * @device_state_fd: Device state migration channel * @device_state_result: Device state migration result @@ -296,6 +297,7 @@ struct ctx { int low_wmem; int low_rmem; int no_bindtodevice; + bool no_ipv6; struct vu_dev *vdev; diff --git a/util.c b/util.c index b64c29e..fe0aab9 100644 --- a/util.c +++ b/util.c @@ -334,6 +334,14 @@ void sock_probe_features(struct ctx *c) c->no_bindtodevice = 1; } + /* Check if IPv6 sockets are usable */ + close(s); + s = socket(AF_INET6, SOCK_STREAM | SOCK_CLOEXEC, IPPROTO_TCP); + if (s < 0) { + c->no_ipv6 = true; + return; + } + close(s); } -- 2.43.0

2 2

[PATCH 4/4] pif, util: Move listen(2) call from sock_l4_() to pif_listen()
by David Gibson 09 Jun '26

09 Jun '26

It's a bit odd to have the listen(2) call for TCP listening sockets, down deep in sock_l4_() conditional upon the epoll type passed in. Move it to pif_listen(), which is at least about listening, although it does still need to be conditional on TCP. Signed-off-by: David Gibson <david(a)gibson.dropbear.id.au> --- pif.c | 14 ++++++++++---- util.c | 7 ------- 2 files changed, 10 insertions(+), 11 deletions(-) diff --git a/pif.c b/pif.c index ede2f903..f99f33e5 100644 --- a/pif.c +++ b/pif.c @@ -123,11 +123,17 @@ int pif_listen(const struct ctx *c, uint8_t proto, uint8_t pif, ref.listen.pif = pif; ref.listen.rule = rule; - ret = epoll_add(c->epollfd, EPOLLIN, ref); - if (ret < 0) { - close(ref.fd); - return ret; + if (proto == IPPROTO_TCP && listen(ref.fd, 128) < 0) { + ret = -errno; + goto fail; } + ret = epoll_add(c->epollfd, EPOLLIN, ref); + if (ret < 0) + goto fail; + return ref.fd; +fail: + close(ref.fd); + return ret; } diff --git a/util.c b/util.c index b64c29ed..e22696e3 100644 --- a/util.c +++ b/util.c @@ -169,13 +169,6 @@ static int sock_l4_(const struct ctx *c, enum epoll_type type, } } - if (type == EPOLL_TYPE_TCP_LISTEN && listen(fd, 128) < 0) { - ret = -errno; - warn("TCP socket listen: %s", strerror_(-ret)); - close(fd); - return ret; - } - return fd; } -- 2.54.0

1 0

[PATCH 0/4] Assorted minor socket creation cleanups
by David Gibson 09 Jun '26

09 Jun '26

As discussed on our recent call, I was looking again at bug 167. I discovered it's still fairly fiddly to address this, but while investigating spotted a number of cleanups to make in the vicinity. I think they make sense even without fixing bug 167 (yet), so here they are. David Gibson (4): flow: Correct misleading signature of flowside_sock_l4() Makefile: Remove unused DUAL_STACK_SOCKETS define fwd, pif: Remove duplicated logic between tcp_listen() and udp_listen() pif, util: Move listen(2) call from sock_l4_() to pif_listen() Makefile | 5 ----- flow.c | 11 +++++------ fwd.c | 8 +------- pif.c | 45 ++++++++++++++++++++++++++++++++++++--------- pif.h | 2 +- tcp.c | 38 -------------------------------------- tcp.h | 2 -- udp.c | 38 -------------------------------------- udp.h | 2 -- util.c | 7 ------- 10 files changed, 43 insertions(+), 115 deletions(-) -- 2.54.0

1 3

[PATCH 0/4] RFC: Improvements to flow specific logging
by David Gibson 09 Jun '26

09 Jun '26

While working on podman bug 23739, I found some deficiences in how and when we report various socket errors related to specific flows. Here are some preliminary patches to improve that. I think patches 1..3 are pretty straightforward and safe. 4/4 I think is a good idea in principle - promoting the priority of a number of messages now that we have ratelimiting. However, plumbing 'now' all the places we need it resuls in a *lot* of churn, so we might want to postpone or split it up. Still, current drafted posted for consideration. David Gibson (4): flow: Regularise flow specific logging helpers flow: Include flow details with higher priority log messages flow: Safer errno handling in flowside_connect() callers flow, treewide: Promote priority of selected flow-linked messages flow.c | 107 +++++++------- flow.h | 72 +++++----- icmp.c | 27 ++-- icmp.h | 3 +- migrate.c | 27 ++-- migrate.h | 8 +- passt.c | 8 +- tcp.c | 381 ++++++++++++++++++++++++++++--------------------- tcp.h | 5 +- tcp_buf.c | 78 +++++----- tcp_buf.h | 8 +- tcp_conn.h | 8 +- tcp_internal.h | 27 ++-- tcp_splice.c | 59 +++++--- tcp_splice.h | 3 +- tcp_vu.c | 42 +++--- tcp_vu.h | 6 +- udp.c | 48 ++++--- udp_flow.c | 22 +-- 19 files changed, 535 insertions(+), 404 deletions(-) -- 2.54.0

1 3

[PATCH 1/4] flow: Regularise flow specific logging helpers
by David Gibson 09 Jun '26

09 Jun '26

flow.h has a collection of logging helpers that automatically include information about a specific flow. Which variants are present are a bit ad-hoc, based on what we happened to want use (e.g. there are no LOG_WARNING level versions, at present). There's also a rather awkward and only occasionally used flow_log_details_() helper to print additional log messages with more details of the flow (basically its addresses). It's particularly awkward to try to combine that with ratelimiting. Re-organise this to be based around a flow_log__() internal helper, which has bool parameters to include strerror() / perror information and/or the extra details. Add wrapper macros for all combinations of perror, ratelimiting and DEBUG/WARNING/ERR priorities. Be a little more consistent about parameter order between the various functions / macros / wrappers while we're at it. Signed-off-by: David Gibson <david(a)gibson.dropbear.id.au> --- flow.c | 89 ++++++++++++++++++++++++++++------------------------------ flow.h | 70 +++++++++++++++++++++++---------------------- tcp.c | 4 +-- udp.c | 6 ++-- 4 files changed, 85 insertions(+), 84 deletions(-) diff --git a/flow.c b/flow.c index 565ed2b2..6cf3905a 100644 --- a/flow.c +++ b/flow.c @@ -270,17 +270,20 @@ int flowside_connect(const struct ctx *c, int s, return connect(s, &sa.sa, socklen_inany(&sa)); } -/** flow_log_ - Log flow-related message +/** flow_log__ - Log flow-related message, internal helper * @f: flow the message is related to - * @newline: Append newline at the end of the message, if missing * @pri: Log priority + * @perror: Append strerror(errno) output + * @details: Add lines with flow addresses + * @state: State to display details for (used during state changes) * @fmt: Format string * @...: printf-arguments */ -void flow_log_(const struct flow_common *f, bool newline, int pri, - const char *fmt, ...) +void flow_log__(const struct flow_common *f, int pri, bool perror, bool details, + enum flow_state state, const char *fmt, ...) { const char *type_or_state; + int errno_ = errno; char msg[BUFSIZ]; va_list args; @@ -294,45 +297,41 @@ void flow_log_(const struct flow_common *f, bool newline, int pri, else type_or_state = FLOW_TYPE(f); - logmsg(newline, false, pri, + logmsg(!perror, false, pri, "Flow %u (%s): %s", flow_idx(f), type_or_state, msg); -} -/** flow_log_details_() - Log the details of a flow - * @f: flow to log - * @pri: Log priority - * @state: State to log details according to - * - * Logs the details of the flow: endpoints, interfaces, type etc. - */ -void flow_log_details_(const struct flow_common *f, int pri, - enum flow_state state) -{ - char estr0[INANY_ADDRSTRLEN], fstr0[INANY_ADDRSTRLEN]; - char estr1[INANY_ADDRSTRLEN], fstr1[INANY_ADDRSTRLEN]; - const struct flowside *ini = &f->side[INISIDE]; - const struct flowside *tgt = &f->side[TGTSIDE]; - - if (state >= FLOW_STATE_TGT) - flow_log_(f, true, pri, - "%s [%s]:%hu -> [%s]:%hu => %s [%s]:%hu -> [%s]:%hu", - pif_name(f->pif[INISIDE]), - inany_ntop(&ini->eaddr, estr0, sizeof(estr0)), - ini->eport, - inany_ntop(&ini->oaddr, fstr0, sizeof(fstr0)), - ini->oport, - pif_name(f->pif[TGTSIDE]), - inany_ntop(&tgt->oaddr, fstr1, sizeof(fstr1)), - tgt->oport, - inany_ntop(&tgt->eaddr, estr1, sizeof(estr1)), - tgt->eport); - else if (state >= FLOW_STATE_INI) - flow_log_(f, true, pri, "%s [%s]:%hu -> [%s]:%hu => ?", - pif_name(f->pif[INISIDE]), - inany_ntop(&ini->eaddr, estr0, sizeof(estr0)), - ini->eport, - inany_ntop(&ini->oaddr, fstr0, sizeof(fstr0)), - ini->oport); + if (perror) + logmsg(true, true, pri, ": %s", strerror_(errno_)); + + if (details) { + char estr0[INANY_ADDRSTRLEN], fstr0[INANY_ADDRSTRLEN]; + char estr1[INANY_ADDRSTRLEN], fstr1[INANY_ADDRSTRLEN]; + const struct flowside *ini = &f->side[INISIDE]; + const struct flowside *tgt = &f->side[TGTSIDE]; + + if (state >= FLOW_STATE_TGT) { + flow_log__(f, pri, false, false, state, +"%s [%s]:%hu -> [%s]:%hu => %s [%s]:%hu -> [%s]:%hu", + pif_name(f->pif[INISIDE]), + inany_ntop(&ini->eaddr, estr0, sizeof(estr0)), + ini->eport, + inany_ntop(&ini->oaddr, fstr0, sizeof(fstr0)), + ini->oport, + pif_name(f->pif[TGTSIDE]), + inany_ntop(&tgt->oaddr, fstr1, sizeof(fstr1)), + tgt->oport, + inany_ntop(&tgt->eaddr, estr1, sizeof(estr1)), + tgt->eport); + } else if (state >= FLOW_STATE_INI) { + flow_log__(f, pri, false, false, state, + "%s [%s]:%hu -> [%s]:%hu => ?", + pif_name(f->pif[INISIDE]), + inany_ntop(&ini->eaddr, estr0, sizeof(estr0)), + ini->eport, + inany_ntop(&ini->oaddr, fstr0, sizeof(fstr0)), + ini->oport); + } + } } /** @@ -348,10 +347,9 @@ static void flow_set_state(struct flow_common *f, enum flow_state state) assert(oldstate < FLOW_NUM_STATES); f->state = state; - flow_log_(f, true, LOG_DEBUG, "%s -> %s", flow_state_str[oldstate], + flow_log__(f, LOG_DEBUG, false, true, MAX(state, oldstate), + "%s -> %s", flow_state_str[oldstate], FLOW_STATE(f)); - - flow_log_details_(f, LOG_DEBUG, MAX(state, oldstate)); } /** @@ -552,8 +550,7 @@ norule: /* This shouldn't happen, because if there's no rule for it we should * have no listening socket that would let us get here */ - flow_dbg(flow, "Missing forward rule"); - flow_log_details_(f, LOG_DEBUG, f->state); + flow_log(flow, LOG_DEBUG, false, true, "Missing forward rule"); nofwd: flow_err(flow, "No rules to forward %s %s [%s]:%hu -> [%s]:%hu", diff --git a/flow.h b/flow.h index 6c6a9260..d168a35a 100644 --- a/flow.h +++ b/flow.h @@ -280,42 +280,46 @@ int flow_migrate_source(struct ctx *c, const struct migrate_stage *stage, int flow_migrate_target(struct ctx *c, const struct migrate_stage *stage, int fd); -void flow_log_(const struct flow_common *f, bool newline, int pri, - const char *fmt, ...) - __attribute__((format(printf, 4, 5))); - -#define flow_log(f_, pri, ...) flow_log_(&(f_)->f, true, (pri), __VA_ARGS__) -#define flow_dbg(f, ...) flow_log((f), LOG_DEBUG, __VA_ARGS__) -#define flow_err(f, ...) flow_log((f), LOG_ERR, __VA_ARGS__) - -#define flow_trace(f, ...) \ +void flow_log__(const struct flow_common *f, int pri, bool perror, bool details, + enum flow_state state, const char *fmt, ...); + +#define flow_log_(f_, pri_, perror_, details_, ...) \ + flow_log__((f_), (pri_), (perror_), (details_), (f_)->state, \ + __VA_ARGS__) + +#define flow_log(flow_, pri_, perror_, details_, ...) \ + flow_log_(&(flow_)->f, (pri_), (perror_), (details_), __VA_ARGS__) + +#define flow_dbg(flow_, ...) \ + flow_log((flow_), LOG_DEBUG, false, false, __VA_ARGS__) +#define flow_warn(flow_, ...) \ + flow_log((flow_), LOG_WARNING, false, false, __VA_ARGS__) +#define flow_err(flow_, ...) \ + flow_log((flow_), LOG_ERR, false, false, __VA_ARGS__) +#define flow_trace(flow_, ...) \ do { \ if (log_trace) \ - flow_dbg((f), __VA_ARGS__); \ - } while (0) - -#define flow_log_perror_(f, pri, ...) \ - do { \ - int errno_ = errno; \ - flow_log_((f), false, (pri), __VA_ARGS__); \ - logmsg(true, true, (pri), ": %s", strerror_(errno_)); \ + flow_dbg((flow_), __VA_ARGS__); \ } while (0) -#define flow_dbg_perror(f_, ...) flow_log_perror_(&(f_)->f, LOG_DEBUG, __VA_ARGS__) -#define flow_perror(f_, ...) flow_log_perror_(&(f_)->f, LOG_ERR, __VA_ARGS__) - -void flow_log_details_(const struct flow_common *f, int pri, - enum flow_state state); -#define flow_log_details(f_, pri) \ - flow_log_details_(&((f_)->f), (pri), (f_)->f.state) -#define flow_dbg_details(f_) flow_log_details((f_), LOG_DEBUG) -#define flow_err_details(f_) flow_log_details((f_), LOG_ERR) - -#define flow_dbg_ratelimit(f, now, ...) \ - logmsg_ratelimit(flow_dbg, debug, now, f, __VA_ARGS__) -#define flow_err_ratelimit(f, now, ...) \ - logmsg_ratelimit(flow_err, err, now, f, __VA_ARGS__) -#define flow_perror_ratelimit(f, now, ...) \ - logmsg_ratelimit(flow_perror, err, now, f, __VA_ARGS__) +#define flow_dbg_perror(flow_, ...) \ + flow_log((flow_), LOG_DEBUG, true, false, __VA_ARGS__) +#define flow_warn_perror(flow_, ...) \ + flow_log((flow_), LOG_WARNING, true, false, __VA_ARGS__) +#define flow_perror(flow_, ...) \ + flow_log((flow_), LOG_ERR, true, false, __VA_ARGS__) + +#define flow_dbg_ratelimit(flow_, now_, ...) \ + logmsg_ratelimit(flow_dbg, debug, (now_), (flow_), __VA_ARGS__) +#define flow_dbg_perror_ratelimit(flow_, now_, ...) \ + logmsg_ratelimit(flow_dbg_perror, debug, (now_), (flow_), __VA_ARGS__) +#define flow_warn_ratelimit(flow_, now_, ...) \ + logmsg_ratelimit(flow_warn, warn, (now_), (flow_), __VA_ARGS__) +#define flow_warn_perror_ratelimit(flow_, now_, ...) \ + logmsg_ratelimit(flow_warn_perror, warn, (now_), (flow_), __VA_ARGS__) +#define flow_err_ratelimit(flow_, now_, ...) \ + logmsg_ratelimit(flow_err, err, (now_), (flow_), __VA_ARGS__) +#define flow_perror_ratelimit(flow_, now_, ...) \ + logmsg_ratelimit(flow_perror, err, (now_), (flow_), __VA_ARGS__) #endif /* FLOW_H */ diff --git a/tcp.c b/tcp.c index c4000754..6fba865f 100644 --- a/tcp.c +++ b/tcp.c @@ -1404,8 +1404,8 @@ void tcp_linger0_(const struct flow_common *f, int s) */ if (setsockopt(s, SOL_SOCKET, SO_LINGER, &linger0, sizeof(linger0)) < 0) { - flow_log_perror_(f, LOG_DEBUG, - "SO_LINGER failed, may not send RST to peer"); + flow_log_(f, LOG_DEBUG, true, false, + "SO_LINGER failed, may not send RST to peer"); } } diff --git a/udp.c b/udp.c index c28d6ee2..f29ca3da 100644 --- a/udp.c +++ b/udp.c @@ -943,7 +943,8 @@ void udp_sock_handler(const struct ctx *c, union epoll_ref ref, if (events & EPOLLERR) { if (udp_sock_errs(c, ref.fd, ref.flowside, PIF_NONE, 0) < 0) { - flow_err(uflow, "Unrecoverable error on flow socket"); + flow_log(uflow, LOG_ERR, false, true, + "Unrecoverable error on flow socket"); goto fail; } } @@ -974,7 +975,7 @@ void udp_sock_handler(const struct ctx *c, union epoll_ref ref, udp_buf_sock_to_tap(c, s, n, tosidx); } } else { - flow_err(uflow, + flow_log(uflow, LOG_ERR, false, true, "No support for forwarding UDP from %s to %s", pif_name(pif_at_sidx(ref.flowside)), pif_name(topif)); @@ -984,7 +985,6 @@ void udp_sock_handler(const struct ctx *c, union epoll_ref ref, return; fail: - flow_err_details(uflow); udp_flow_close(c, uflow); } -- 2.54.0

1 0

[PATCH] tap: don't let overheard traffic move addr_seen when serving a fixed address
by EJ Campbell 09 Jun '26

09 Jun '26

The latest source address seen on the tap interface (ip4.addr_seen) is taken to be the guest's and is used as the target for inbound port forwarding. The update accepts any source address. On a point-to-point tap that is always correct. But when the tap sits on a shared L2 segment -- for example when it is bridged together with other hosts -- pasta also sees frames from those other hosts. With a static configuration (-a / address set, DHCP disabled), an overheard frame from another host silently moves addr_seen to that host, and every subsequent inbound forwarded connection is sent there instead of to the guest, so connections are reset or hang. I hit this running pasta with its tap enslaved to a Linux bridge that also carries other traffic: inbound port forwarding to the guest broke as soon as another address was seen on the segment. Restrict the update so that, when DHCP is disabled (no_dhcp), only frames sourced from the configured guest address may move addr_seen. With DHCP enabled -- the default -- behaviour is unchanged, since the guest's address is whatever pasta leases and tracking it is still correct. The same reasoning applies to ip6.addr_seen on a shared segment; I left IPv6 alone for now since I only exercise IPv4 in this setup, and would be glad to extend it the same way if you prefer. Signed-off-by: EJ Campbell <ej.campbell(a)gmail.com> --- tap.c | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/tap.c b/tap.c index 4cba4c7..514379e 100644 --- a/tap.c +++ b/tap.c @@ -770,7 +770,19 @@ resume: continue; } - if (iph->saddr && c->ip4.addr_seen.s_addr != iph->saddr) + /* The latest source address seen on the tap is taken to be the + * guest's, and becomes the target for inbound forwarding. On a + * point-to-point tap that's always correct. But if the tap sits + * on a shared L2 segment (for example bridged together with + * other hosts) and we serve a fixed address (DHCP disabled), a + * frame overheard from another host on that segment must not + * move addr_seen and silently retarget forwarded connections + * away from the guest. When DHCP is disabled, only the + * configured guest address may update addr_seen; with DHCP + * enabled (the default) behaviour is unchanged. + */ + if (iph->saddr && c->ip4.addr_seen.s_addr != iph->saddr && + (!c->no_dhcp || iph->saddr == c->ip4.addr.s_addr)) c->ip4.addr_seen.s_addr = iph->saddr; if (!iov_drop_header(&data, hlen)) base-commit: 4b2823784aab04a70dfc295b16fd6f0592955790 -- 2.53.0

3 2

[PATCH v2] udp: Provide dummy iov in udp_peek_addr() to avoid Coverity warning
by Jon Maloy 09 Jun '26

09 Jun '26

udp_peek_addr() initialises struct msghdr without setting msg_iov, leaving it implicitly NULL. Coverity flags this as FORWARD_NULL, believing recvmsg() will dereference the NULL pointer. In practice, msg_iovlen being zero means the kernel never touches msg_iov, so the warning is a false positive. We now provide a one-byte dummy iov to make msg_iov non-NULL, hence suppressing this warning without changing the function's behaviour. Signed-off-by: Jon Maloy <jmaloy(a)redhat.com> ---- v2: - Make the dummy iov conditional on an ANALYZER macro, so it has zero runtime cost in production builds. - Add a new 'analyzer' Makefile target (similar to 'valgrind') that defines ANALYZER via CPPFLAGS for use with static analysis builds. --- Makefile | 3 +++ udp.c | 11 +++++++++++ 2 files changed, 14 insertions(+) diff --git a/Makefile b/Makefile index 0a0a60b0..4dcf4cd1 100644 --- a/Makefile +++ b/Makefile @@ -122,6 +122,9 @@ passt-repair: $(PASST_REPAIR_SRCS) $(PASST_REPAIR_HEADERS) seccomp_repair.h pesto: BASE_CPPFLAGS += -DPESTO pesto: $(PESTO_SRCS) $(PESTO_HEADERS) seccomp_pesto.h +analyzer: BASE_CPPFLAGS += -DANALYZER +analyzer: all + valgrind: EXTRA_SYSCALLS += rt_sigprocmask rt_sigtimedwait rt_sigaction \ rt_sigreturn getpid gettid kill clock_gettime \ mmap|mmap2 munmap open unlink gettimeofday futex \ diff --git a/udp.c b/udp.c index c28d6ee2..36c8c070 100644 --- a/udp.c +++ b/udp.c @@ -734,9 +734,20 @@ static int udp_peek_addr(int s, union sockaddr_inany *src, { char sastr[SOCKADDR_STRLEN], dstr[INANY_ADDRSTRLEN]; char cmsg[PKTINFO_SPACE]; +#ifdef ANALYZER + char dummy; + struct iovec iov = { + .iov_base = &dummy, + .iov_len = sizeof(dummy), + }; +#endif /* ANALYZER */ struct msghdr msg = { .msg_name = src, .msg_namelen = sizeof(*src), +#ifdef ANALYZER + .msg_iov = &iov, + .msg_iovlen = 1, +#endif /* ANALYZER */ .msg_control = cmsg, .msg_controllen = sizeof(cmsg), }; -- 2.52.0

2 1

[PATCH v2] util, passt: Close daemon-lifetime fds on exit to avoid Coverity warning
by Jon Maloy 09 Jun '26

09 Jun '26

conf_open_files() opens three file descriptors (fd_tap_listen, fd_repair_listen, fd_control_listen) that are held for the entire daemon lifetime. Because no close() call exists for them anywhere, Coverity flags each as INCOMPLETE_DEALLOCATOR. This is clearly a false positive, but we still want to get rid of this warning. We now register the execution context in global area so that passt_exit() can use it to close these descriptors before calling _exit(). All exit paths (signal handler, die(), die_perror()) funnel through passt_exit(), so this covers all cases. Signed-off-by: Jon Maloy <jmaloy(a)redhat.com> --- v2: - Made struct ctx global from the beginning, on suggestion from David Gibson. - Drop pidfile_fd from passt_exit() cleanup, since it can be closed directly after use. Signed-off-by: Jon Maloy <jmaloy(a)redhat.com> --- passt.c | 77 +++++++++++++++++++++++++++++++-------------------------- passt.h | 2 ++ util.c | 7 ++++++ 3 files changed, 51 insertions(+), 35 deletions(-) diff --git a/passt.c b/passt.c index b3f806b9..e75b80b1 100644 --- a/passt.c +++ b/passt.c @@ -62,6 +62,13 @@ char pkt_buf[PKT_BUF_BYTES] __attribute__ ((aligned(PAGE_SIZE))); +struct ctx passt_ctx = { + .pidfile_fd = -1, + .fd_tap = -1, + .pasta_netns_fd = -1, + .device_state_fd = -1, +}; + char *epoll_type_str[] = { [EPOLL_TYPE_TCP] = "connected TCP socket", [EPOLL_TYPE_TCP_SPLICE] = "connected spliced TCP socket", @@ -322,9 +329,9 @@ static void passt_worker(void *opaque, int nfds, struct epoll_event *events) */ int main(int argc, char **argv) { + struct ctx *c = &passt_ctx; struct epoll_event events[NUM_EPOLL_EVENTS]; int nfds, devnull_fd = -1; - struct ctx c = { 0 }; struct rlimit limit; struct timespec now; struct sigaction sa; @@ -336,18 +343,15 @@ int main(int argc, char **argv) isolate_initial(argc, argv); - c.pasta_netns_fd = c.fd_tap = c.pidfile_fd = -1; - c.device_state_fd = -1; - sigemptyset(&sa.sa_mask); sa.sa_flags = 0; sa.sa_handler = exit_handler; sigaction(SIGTERM, &sa, NULL); sigaction(SIGQUIT, &sa, NULL); - c.mode = conf_mode(argc, argv); + c->mode = conf_mode(argc, argv); - if (c.mode == MODE_PASTA) { + if (c->mode == MODE_PASTA) { sa.sa_handler = pasta_child_handler; if (sigaction(SIGCHLD, &sa, NULL)) die_perror("Couldn't install signal handlers"); @@ -358,67 +362,70 @@ int main(int argc, char **argv) madvise(pkt_buf, sizeof(pkt_buf), MADV_HUGEPAGE); - c.epollfd = epoll_create1(EPOLL_CLOEXEC); - if (c.epollfd == -1) + c->epollfd = epoll_create1(EPOLL_CLOEXEC); + if (c->epollfd == -1) die_perror("Failed to create epoll file descriptor"); - flow_epollid_register(EPOLLFD_ID_DEFAULT, c.epollfd); + flow_epollid_register(EPOLLFD_ID_DEFAULT, c->epollfd); if (getrlimit(RLIMIT_NOFILE, &limit)) die_perror("Failed to get maximum value of open files limit"); - c.nofile = limit.rlim_cur = limit.rlim_max; + c->nofile = limit.rlim_cur = limit.rlim_max; if (setrlimit(RLIMIT_NOFILE, &limit)) die_perror("Failed to set current limit for open files"); - sock_probe_features(&c); + sock_probe_features(c); - conf(&c, argc, argv); - trace_init(c.trace); + conf(c, argc, argv); + trace_init(c->trace); - pasta_netns_quit_init(&c); + pasta_netns_quit_init(c); - tap_backend_init(&c); + tap_backend_init(c); - random_init(&c); + random_init(c); if (clock_gettime(CLOCK_MONOTONIC, &now)) die_perror("Failed to get CLOCK_MONOTONIC time"); flow_init(); - fwd_scan_ports_init(&c); + fwd_scan_ports_init(c); - if ((!c.no_udp && udp_init(&c)) || (!c.no_tcp && tcp_init(&c))) + if ((!c->no_udp && udp_init(c)) || (!c->no_tcp && tcp_init(c))) passt_exit(EXIT_FAILURE); - if (fwd_listen_init(&c)) + if (fwd_listen_init(c)) passt_exit(EXIT_FAILURE); - proto_update_l2_buf(c.guest_mac); + proto_update_l2_buf(c->guest_mac); - if (c.ifi4 && !c.no_dhcp) + if (c->ifi4 && !c->no_dhcp) dhcp_init(); - if (c.ifi6 && !c.no_dhcpv6) - dhcpv6_init(&c); + if (c->ifi6 && !c->no_dhcpv6) + dhcpv6_init(c); - pcap_init(&c); + pcap_init(c); - fwd_neigh_table_init(&c); - nl_neigh_notify_init(&c); + fwd_neigh_table_init(c); + nl_neigh_notify_init(c); - if (!c.foreground) { + if (!c->foreground) { if ((devnull_fd = open("/dev/null", O_RDWR | O_CLOEXEC)) < 0) die_perror("Failed to open /dev/null"); } - if (isolate_prefork(&c)) + if (isolate_prefork(c)) die("Failed to sandbox process, exiting"); - if (!c.foreground) { - __daemon(c.pidfile_fd, devnull_fd); + if (!c->foreground) { + __daemon(c->pidfile_fd, devnull_fd); + close(c->pidfile_fd); + c->pidfile_fd = -1; log_stderr = false; } else { - pidfile_write(c.pidfile_fd, getpid()); + pidfile_write(c->pidfile_fd, getpid()); + c->pidfile_fd = -1; } if (pasta_child_pid) { @@ -426,19 +433,19 @@ int main(int argc, char **argv) log_stderr = false; } - isolate_postfork(&c); + isolate_postfork(c); - timer_init(&c, &now); + timer_init(c, &now); loop: /* NOLINTBEGIN(bugprone-branch-clone): intervals can be the same */ /* cppcheck-suppress [duplicateValueTernary, unmatchedSuppression] */ - nfds = epoll_wait(c.epollfd, events, NUM_EPOLL_EVENTS, TIMER_INTERVAL); + nfds = epoll_wait(c->epollfd, events, NUM_EPOLL_EVENTS, TIMER_INTERVAL); /* NOLINTEND(bugprone-branch-clone) */ if (nfds == -1 && errno != EINTR) die_perror("epoll_wait() failed in main loop"); - passt_worker(&c, nfds, events); + passt_worker(c, nfds, events); goto loop; } diff --git a/passt.h b/passt.h index 1726965d..e1905f4c 100644 --- a/passt.h +++ b/passt.h @@ -307,6 +307,8 @@ struct ctx { bool migrate_exit; }; +extern struct ctx passt_ctx; + void proto_update_l2_buf(const unsigned char *eth_d); #endif /* PASST_H */ diff --git a/util.c b/util.c index b64c29ed..ef6ba80e 100644 --- a/util.c +++ b/util.c @@ -1108,6 +1108,13 @@ void abort_with_msg(const char *fmt, ...) */ void passt_exit(int status) { + if (passt_ctx.fd_tap_listen >= 0) + close(passt_ctx.fd_tap_listen); + if (passt_ctx.fd_repair_listen >= 0) + close(passt_ctx.fd_repair_listen); + if (passt_ctx.fd_control_listen >= 0) + close(passt_ctx.fd_control_listen); + /* Make sure we don't leave the pcap file truncated */ if (pcap_fd != -1 && fsync(pcap_fd)) warn_perror("Failed to flush pcap file, it might be truncated"); -- 2.52.0

2 1

[PATCH 0/8] splice() forwarding cleanups
by David Gibson 05 Jun '26

05 Jun '26

As we discussed on an earlier call, while fixing bug 202 I noticed a number of warts in the surrounding splice() forwarding code. Most are just things that are longer or harder to follow than they need to be, but in some cases there may be real (if unlikely to trigger) bugs. Here's a collection of fixes. David Gibson (8): tcp_splice: Remove never-invoked SO_RCVLOWAT logic tcp_splice: Simplify EPOLLRDHUP / eof / FIN handling tcp_splice: Improve EOF exit condition for the loop tcp_splice: Remove goto from forwarding loop tcp_splice: Simplify shutdown(2) handling tcp_splice: Simplify / correct OUT_WAIT flag handling tcp_splice: Remove questionable "optimisation" of pending bytes tracking tcp_splice: Exit forwarding earlier when stalled read side tcp_splice.c | 100 ++++++++++++++++----------------------------------- 1 file changed, 31 insertions(+), 69 deletions(-) -- 2.54.0

2 20

[PATCH] passt, tcp: Inline CALL_PROTO_HANDLER() and merge tcp_timer()
by Laurent Vivier 05 Jun '26

05 Jun '26

Since 260075bde769 ("tcp, udp, fwd: Run all port scanning from a single timer"), CALL_PROTO_HANDLER() has only one user (tcp), so inline it at the call site and remove the macro. Merge tcp_timer() into tcp_defer_handler(), moving the timer interval check there, matching the pattern used by flow_defer_handler() and fwd_scan_ports_timer(). The weak declaration and null check for tcp_defer_handler are also dropped as the function is always defined. Signed-off-by: Laurent Vivier <lvivier(a)redhat.com> --- passt.c | 23 ++--------------------- tcp.c | 22 ++++++++++------------ tcp.h | 3 +-- 3 files changed, 13 insertions(+), 35 deletions(-) diff --git a/passt.c b/passt.c index b6fc12d4ed81..b3f806b9a15e 100644 --- a/passt.c +++ b/passt.c @@ -101,27 +101,8 @@ struct passt_stats { */ static void post_handler(struct ctx *c, const struct timespec *now) { -#define CALL_PROTO_HANDLER(lc, uc) \ - do { \ - extern void \ - lc ## _defer_handler (struct ctx *c) \ - __attribute__ ((weak)); \ - \ - if (!c->no_ ## lc) { \ - if (lc ## _defer_handler) \ - lc ## _defer_handler(c); \ - \ - if (timespec_diff_ms((now), &c->lc.timer_run) \ - >= uc ## _TIMER_INTERVAL) { \ - lc ## _timer(c, now); \ - c->lc.timer_run = *now; \ - } \ - } \ - } while (0) - - /* NOLINTNEXTLINE(bugprone-branch-clone): intervals can be the same */ - CALL_PROTO_HANDLER(tcp, TCP); -#undef CALL_PROTO_HANDLER + if (!c->no_tcp) + tcp_defer_handler(c, now); flow_defer_handler(c, now); fwd_scan_ports_timer(c, now); diff --git a/tcp.c b/tcp.c index 0fb8da05cb65..c400075440c9 100644 --- a/tcp.c +++ b/tcp.c @@ -895,16 +895,6 @@ bool tcp_flow_defer(const struct tcp_tap_conn *conn) return true; } -/** - * tcp_defer_handler() - Handler for TCP deferred tasks - * @c: Execution context - */ -/* cppcheck-suppress [constParameterPointer, unmatchedSuppression] */ -void tcp_defer_handler(struct ctx *c) -{ - tcp_payload_flush(c); -} - /** * tcp_fill_header() - Fill the TCP header fields for a given TCP segment. * @@ -3005,12 +2995,20 @@ static void tcp_inactivity(struct ctx *c, const struct timespec *now) } /** - * tcp_timer() - Periodic tasks: port detection, closed connections, pool refill + * tcp_defer_handler() - Handler for TCP deferred tasks * @c: Execution context * @now: Current timestamp */ -void tcp_timer(struct ctx *c, const struct timespec *now) +/* cppcheck-suppress [constParameterPointer, unmatchedSuppression] */ +void tcp_defer_handler(struct ctx *c, const struct timespec *now) { + tcp_payload_flush(c); + + if (timespec_diff_ms(now, &c->tcp.timer_run) < TCP_TIMER_INTERVAL) + return; + + c->tcp.timer_run = *now; + tcp_sock_refill_init(c); if (c->mode == MODE_PASTA) tcp_splice_refill(c); diff --git a/tcp.h b/tcp.h index 8a0eb930b54b..3262a807e5d4 100644 --- a/tcp.h +++ b/tcp.h @@ -29,8 +29,7 @@ int tcp_tap_handler(const struct ctx *c, uint8_t pif, sa_family_t af, int tcp_listen(const struct ctx *c, uint8_t pif, unsigned rule, const union inany_addr *addr, const char *ifname, in_port_t port); int tcp_init(struct ctx *c); -void tcp_timer(struct ctx *c, const struct timespec *now); -void tcp_defer_handler(struct ctx *c); +void tcp_defer_handler(struct ctx *c, const struct timespec *now); void tcp_update_l2_buf(const unsigned char *eth_d); -- 2.54.0

3 2