- dev - passt

[PATCH 0/3] cppcheck-2.19.1 fixes
by David Gibson 14 Jan '26

14 Jan '26

Here are some more fixes for cppcheck warnings that I'm seeing with cppcheck-2.19.1. One looks like it might, at least theoretically be a real bug. I'm not certain if all of these are due to a new cppcheck version, or if some are just due to things merged recently that hadn't previously been checked with a recent-ish version. David Gibson (3): treewide: Fix more pointers which can be const epoll_ctl: Move u64 variant first for safer initialisation igmp: Remove apparently unneeded suppression conf.c | 2 +- dhcpv6.c | 3 ++- epoll_ctl.h | 4 ++-- igmp.c | 1 - vhost_user.c | 2 +- 5 files changed, 6 insertions(+), 6 deletions(-) -- 2.52.0

2 1

[PATCH v2 0/2] Allow listen functions to return fds
by David Gibson 14 Jan '26

14 Jan '26

For future forwarding changes, it makes more sense for the tcp and udp listen functions to return socket fds, rather than just success / failure. Make the change, fixing bug 186 along the way. Link: https://bugs.passt.top/show_bug.cgi?id=186 Changes in v2: * Fix several typos in comments and commit messages * Drop the --no-bindtodevice option, I was convinced it wasn't useful enough * Change from -EACCES to -EAFNOSUPPORT error code David Gibson (2): tcp, udp, conf: Don't silently ignore listens on unsupported IP versions tcp, udp: Make {tcp,udp}_listen() return socket fds conf.c | 24 +++++++++++++++++------- tcp.c | 13 ++++--------- udp.c | 10 ++++------ 3 files changed, 25 insertions(+), 22 deletions(-) -- 2.52.0

2 3

[PATCH] apparmor: Upgrade ABI version to 4.0, explicitly enable user namespace creation
by Stefano Brivio 13 Jan '26

13 Jan '26

In the 3.0 AppArmor ABI version we currently use, user namespace rules are not supported, and, as long as we load confined profiles, those implicitly allow creation of user namespaces. However, ABI version 4.0 introduces rules for user namespaces, and if we don't specify any, we can't create user namespaces, see: https://gitlab.com/apparmor/apparmor/-/wikis/unprivileged_userns_restriction This wouldn't affect us in general, given that we're using the 3.0 ABI, but libvirt's policy uses 4.0 instead, and if our abstractions are used from there, no matter what ABI policy version we declare, rules for user namespace creation now match ABI policy version 4.0. As a result, when libvirtd runs as root, and its profile includes passt's abstraction, cf. commit 66769c2de825 ("apparmor: Workaround for unconfined libvirtd when triggered by unprivileged user"), passt can't detach user namespaces and will fail to start, as reported by Niklas: ERROR internal error: Child process (passt --one-off --socket /run/libvirt/qemu/passt/1-haos-net0.socket --pid /run/libvirt/qemu/passt/1-haos-net0-passt.pid --tcp-ports 8123) unexpected exit status 1: Multiple interfaces with IPv6 routes, picked first UNIX domain socket bound at /run/libvirt/qemu/passt/1-haos-net0.socket Couldn't create user namespace: Permission denied This isn't a problem with libvirtd running as regular user, because in that case, as a workaround, passt currently runs under its own profile, not as a libvirtd subprofile (see commit referenced above). Given that ABI 4.0 has been around for a while, being introduced in July 2023, finally take the step to upgrade to it and explicitly enable user namespace creation. No further changes are needed in the existing policies to match new features introduced in AppArmor 4.0. Reported-by: Niklas Edmundsson <nikke(a)accum.se> Link: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1124801 Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com> --- contrib/apparmor/abstractions/passt | 3 ++- contrib/apparmor/abstractions/pasta | 2 +- contrib/apparmor/usr.bin.passt | 2 +- contrib/apparmor/usr.bin.passt-repair | 2 +- contrib/apparmor/usr.bin.pasta | 2 +- 5 files changed, 6 insertions(+), 5 deletions(-) diff --git a/contrib/apparmor/abstractions/passt b/contrib/apparmor/abstractions/passt index 25b2ea8..0ffadaf 100644 --- a/contrib/apparmor/abstractions/passt +++ b/contrib/apparmor/abstractions/passt @@ -11,7 +11,7 @@ # Copyright (c) 2022 Red Hat GmbH # Author: Stefano Brivio <sbrivio(a)redhat.com> - abi <abi/3.0>, + abi <abi/4.0>, include <abstractions/base> @@ -24,6 +24,7 @@ capability setpcap, capability net_admin, capability sys_ptrace, + userns, / r, # isolate_prefork(), isolation.c mount options=(rw, runbindable) -> /, diff --git a/contrib/apparmor/abstractions/pasta b/contrib/apparmor/abstractions/pasta index 9f73bee..251d4a2 100644 --- a/contrib/apparmor/abstractions/pasta +++ b/contrib/apparmor/abstractions/pasta @@ -11,7 +11,7 @@ # Copyright (c) 2022 Red Hat GmbH # Author: Stefano Brivio <sbrivio(a)redhat.com> - abi <abi/3.0>, + abi <abi/4.0>, include <abstractions/passt> diff --git a/contrib/apparmor/usr.bin.passt b/contrib/apparmor/usr.bin.passt index 62a4514..c123a86 100644 --- a/contrib/apparmor/usr.bin.passt +++ b/contrib/apparmor/usr.bin.passt @@ -11,7 +11,7 @@ # Copyright (c) 2022 Red Hat GmbH # Author: Stefano Brivio <sbrivio(a)redhat.com> -abi <abi/3.0>, +abi <abi/4.0>, include <tunables/global> diff --git a/contrib/apparmor/usr.bin.passt-repair b/contrib/apparmor/usr.bin.passt-repair index 901189d..23ff1ce 100644 --- a/contrib/apparmor/usr.bin.passt-repair +++ b/contrib/apparmor/usr.bin.passt-repair @@ -11,7 +11,7 @@ # Copyright (c) 2025 Red Hat GmbH # Author: Stefano Brivio <sbrivio(a)redhat.com> -abi <abi/3.0>, +abi <abi/4.0>, #include <tunables/global> diff --git a/contrib/apparmor/usr.bin.pasta b/contrib/apparmor/usr.bin.pasta index 2483968..56b5024 100644 --- a/contrib/apparmor/usr.bin.pasta +++ b/contrib/apparmor/usr.bin.pasta @@ -11,7 +11,7 @@ # Copyright (c) 2022 Red Hat GmbH # Author: Stefano Brivio <sbrivio(a)redhat.com> -abi <abi/3.0>, +abi <abi/4.0>, include <tunables/global> -- 2.43.0

2 4

[PATCH 1/3] conf: Introduce --no-bindtodevice option for testing
by David Gibson 13 Jan '26

13 Jan '26

We need to support (as best we can) older kernels which don't allow unprivilieged processes to use the SO_BINDTODEVICE socket option. Fallcaks for that case are controlled by the c->no_bindtodevice variable. Currently testing behaviour of those fallbacks requires setting up a test system with a kernel that doesn't support the option, which is pretty awkward. We can test it almost as well and much more easily by adding a command line option to explicitly disable use of SO_BINDTODEVICE. Like --no-splice this is envisaged as something for developers' and testers' convenience, not a supported option for end users. The man page text reflects that. Signed-off-by: David Gibson <david(a)gibson.dropbear.id.au> --- conf.c | 2 ++ passt.1 | 6 ++++++ 2 files changed, 8 insertions(+) diff --git a/conf.c b/conf.c index ceb9aa55..70ea168c 100644 --- a/conf.c +++ b/conf.c @@ -962,6 +962,7 @@ static void usage(const char *name, FILE *f, int status) " --no-ndp Disable NDP responses\n" " --no-dhcpv6 Disable DHCPv6 server\n" " --no-ra Disable router advertisements\n" + " --no-bindtodevice Disable SO_BINDTODEVICE\n" " --freebind Bind to any address for forwarding\n" " --no-map-gw Don't map gateway address to host\n" " -4, --ipv4-only Enable IPv4 operation only\n" @@ -1454,6 +1455,7 @@ void conf(struct ctx *c, int argc, char **argv) {"no-dhcpv6", no_argument, &c->no_dhcpv6, 1 }, {"no-ndp", no_argument, &c->no_ndp, 1 }, {"no-ra", no_argument, &c->no_ra, 1 }, + {"no-bindtodevice", no_argument, &c->no_bindtodevice, 1}, {"no-splice", no_argument, &c->no_splice, 1 }, {"freebind", no_argument, &c->freebind, 1 }, {"no-map-gw", no_argument, &no_map_gw, 1 }, diff --git a/passt.1 b/passt.1 index db0d6620..4859d9e5 100644 --- a/passt.1 +++ b/passt.1 @@ -348,6 +348,12 @@ namespace will be silently dropped. Disable Router Advertisements. Router Solicitations coming from guest or target namespace will be ignored. +.TP +.BR \-\-no-bindtodevice +Development/testing option, do not use. Disables use of +SO_BINDTODEVICE socket option. Implicitly enabled on older kernels +which don't permit unprivileged use of SO_BINDTODEVICE. + .TP .BR \-\-freebind Allow any binding address to be specified for \fB-t\fR and \fB-u\fR -- 2.52.0

2 5

[PATCH 2/3] epoll_ctl: Move u64 variant first for safer initialisation
by David Gibson 13 Jan '26

13 Jan '26

cppcheck-2.19.1 pointed out that an initialiser like: union epoll_ref foo = { 0 }; doesn't necessarily zero the whole union, because the first variant listed may not be the full length. I don't think this is actually broken in practice, but I'm not 100% certain about that. In any case, make it more clearly correct, and stop cppcheck complaining by moving the @u64 variant to be the first one. Signed-off-by: David Gibson <david(a)gibson.dropbear.id.au> --- epoll_ctl.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/epoll_ctl.h b/epoll_ctl.h index 3f802e76..2c103cde 100644 --- a/epoll_ctl.h +++ b/epoll_ctl.h @@ -17,6 +17,7 @@ /** * union epoll_ref - Breakdown of reference for epoll fd bookkeeping + * @u64: Opaque reference for epoll_ctl() and epoll_wait() * @type: Type of fd (tells us what to do with events) * @fd: File descriptor number (implies < 2^24 total descriptors) * @flow: Index of the flow this fd is linked to @@ -25,9 +26,9 @@ * @data: Data handled by protocol handlers * @nsdir_fd: netns dirfd for fallback timer checking if namespace is gone * @queue: vhost-user queue index for this fd - * @u64: Opaque reference for epoll_ctl() and epoll_wait() */ union epoll_ref { + uint64_t u64; struct { enum epoll_type type:8; int32_t fd:FD_REF_BITS; @@ -40,7 +41,6 @@ union epoll_ref { int queue; }; }; - uint64_t u64; }; static_assert(sizeof(union epoll_ref) <= sizeof(union epoll_data), "epoll_ref must have same size as epoll_data"); -- 2.52.0

1 0

[PATCH 1/3] treewide: Fix more pointers which can be const
by David Gibson 13 Jan '26

13 Jan '26

Here are some more pointers which can be const, as reported by cppcheck 2.19.1. Signed-off-by: David Gibson <david(a)gibson.dropbear.id.au> --- conf.c | 2 +- dhcpv6.c | 3 ++- vhost_user.c | 2 +- 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/conf.c b/conf.c index b1fc4b9f..a242fda1 100644 --- a/conf.c +++ b/conf.c @@ -1224,7 +1224,7 @@ dns6: * * Return: 0 on success, negative error code on failure */ -static int conf_runas(char *opt, unsigned int *uid, unsigned int *gid) +static int conf_runas(const char *opt, unsigned int *uid, unsigned int *gid) { const char *uopt, *gopt = NULL; char *sep = strchr(opt, ':'); diff --git a/dhcpv6.c b/dhcpv6.c index d94be23a..97c04e2c 100644 --- a/dhcpv6.c +++ b/dhcpv6.c @@ -320,7 +320,7 @@ static bool dhcpv6_opt(struct iov_tail *data, uint16_t type) static bool dhcpv6_ia_notonlink(struct iov_tail *data, struct in6_addr *la) { - int ia_types[2] = { OPT_IA_NA, OPT_IA_TA }, *ia_type; + int ia_types[2] = { OPT_IA_NA, OPT_IA_TA }; struct opt_ia_addr opt_addr_storage; const struct opt_ia_addr *opt_addr; struct iov_tail current, ia_base; @@ -330,6 +330,7 @@ static bool dhcpv6_ia_notonlink(struct iov_tail *data, struct in6_addr req_addr; struct opt_hdr h_storage; const struct opt_hdr *h; + const int *ia_type; foreach(ia_type, ia_types) { current = *data; diff --git a/vhost_user.c b/vhost_user.c index 1e320bff..9fe12411 100644 --- a/vhost_user.c +++ b/vhost_user.c @@ -442,7 +442,7 @@ static bool vu_set_mem_table_exec(struct vu_dev *vdev, for (i = 0; i < vdev->memory.nregions; i++) { struct vhost_user_memory_region *msg_region = &memory->regions[i]; struct vu_dev_region *dev_region = &vdev->memory.regions[i]; - void *mmap_addr; + const void *mmap_addr; debug("vhost-user region %d", i); debug(" guest_phys_addr: 0x%016"PRIx64, -- 2.52.0

1 0

[PATCH 2/3] tcp, udp, conf: Don't silently ignore listens on unsupported IP versions
by David Gibson 13 Jan '26

13 Jan '26

Currently, it's possible to explicitly ask for forwarding from an IPv4 address, while disabling IPv4: $ pasta -t 192.0.2.1/12345 -6 or vice versa: $ pasta -t 2001:db8::1/12345 -4 Currently, the impossible to implement forwarding option will be silently ignored. That's potentially confusing since in a complex setup, it might not be obvious why the requested forward isn't taking effect. Specifically, it's ignored at a fairly low level: tcp_listen() and udp_listen() ignore it and return 0. Those run kind of late to give a good error message. Change the low-level functions to return -EACCES (chosen because that's what the kernel will return if you request IPv6 when it's disabled by sysctl). Most callers of {tcp,udp}_listen() ignore the return code, so this is a no-op for them. In the remaining caller, conf_ports_range_except() check for the case explicitly, and provide a meaningful error message. Of itself, this bug is insignificant, but this is a roadblock to having {tcp,udp}_listen() return socket fds, which in turn is a roadblock to my flexible forwarding work. So, might as well fix it. Link: https://bugs.passt.top/show_bug.cgi?id=186 Signed-off-by: David Gibson <david(a)gibson.dropbear.id.au> --- conf.c | 10 ++++++++++ tcp.c | 6 ++---- udp.c | 6 ++---- 3 files changed, 14 insertions(+), 8 deletions(-) diff --git a/conf.c b/conf.c index 70ea168c..cc3c20a9 100644 --- a/conf.c +++ b/conf.c @@ -162,6 +162,16 @@ static void conf_ports_range_except(const struct ctx *c, char optname, optname, optarg); } + if (addr) { + if (!c->ifi4 && inany_v4(addr)) { + die("IPv4 is disabled, can't use -%c %s", + optname, optarg); + } else if (!c->ifi6 && !inany_v4(addr)) { + die("IPv6 is disabled, can't use -%c %s", + optname, optarg); + } + } + for (i = first; i <= last; i++) { if (bitmap_isset(exclude, i)) continue; diff --git a/tcp.c b/tcp.c index e7fa85f3..67007c05 100644 --- a/tcp.c +++ b/tcp.c @@ -2700,16 +2700,14 @@ int tcp_listen(const struct ctx *c, uint8_t pif, /* Restrict to v6 only */ addr = &inany_any6; else if (inany_v4(addr)) - /* Nothing to do */ - return 0; + return -EACCES; } if (!c->ifi6) { if (!addr) /* Restrict to v4 only */ addr = &inany_any4; else if (!inany_v4(addr)) - /* Nothing to do */ - return 0; + return -EACCES; } if (pif == PIF_HOST) { diff --git a/udp.c b/udp.c index eda55c39..8cfa1e1f 100644 --- a/udp.c +++ b/udp.c @@ -1162,16 +1162,14 @@ int udp_listen(const struct ctx *c, uint8_t pif, /* Restrict to v6 only */ addr = &inany_any6; else if (inany_v4(addr)) - /* Nothing to do */ - return 0; + return -EACCES; } if (!c->ifi6) { if (!addr) /* Restrict to v4 only */ addr = &inany_any4; else if (!inany_v4(addr)) - /* Nothing to do */ - return 0; + return -EACCES; } s = pif_sock_l4(c, EPOLL_TYPE_UDP_LISTEN, pif, -- 2.52.0

2 4

[PATCH] treewide: Fix places where we incorrectly indented with spaces
by David Gibson 12 Jan '26

12 Jan '26

Had a moment to address this trivial issue. The passt convention is to indent code with tabs (K&R / kernel style). However there were a handful of places where we used spaces instead. Fix them. Link: https://bugs.passt.top/show_bug.cgi?id=135 Reported-by: Xun Gu <xugu(a)redhat.com> Signed-off-by: David Gibson <david(a)gibson.dropbear.id.au> --- doc/platform-requirements/listen-vs-repair.c | 6 +++--- flow.h | 4 ++-- iov.h | 4 ++-- udp_internal.h | 2 +- vhost_user.c | 6 +++--- virtio.c | 8 ++++---- vu_common.h | 2 +- 7 files changed, 16 insertions(+), 16 deletions(-) diff --git a/doc/platform-requirements/listen-vs-repair.c b/doc/platform-requirements/listen-vs-repair.c index d31fe3f7..e21d1688 100644 --- a/doc/platform-requirements/listen-vs-repair.c +++ b/doc/platform-requirements/listen-vs-repair.c @@ -58,9 +58,9 @@ static void net_sandbox(void) .nlh.nlmsg_len = sizeof(req), .nlh.nlmsg_seq = 1, .ifm.ifi_family = AF_UNSPEC, - .ifm.ifi_index = 1, - .ifm.ifi_flags = IFF_UP, - .ifm.ifi_change = IFF_UP, + .ifm.ifi_index = 1, + .ifm.ifi_flags = IFF_UP, + .ifm.ifi_change = IFF_UP, }; int nl; diff --git a/flow.h b/flow.h index b43b0b1d..8ff565d9 100644 --- a/flow.h +++ b/flow.h @@ -99,7 +99,7 @@ static_assert(FLOW_NUM_STATES <= (1 << FLOW_STATE_BITS), extern const char *flow_state_str[]; #define FLOW_STATE(f) \ - ((f)->state < FLOW_NUM_STATES ? flow_state_str[(f)->state] : "?") + ((f)->state < FLOW_NUM_STATES ? flow_state_str[(f)->state] : "?") /** * enum flow_type - Different types of packet flows we track @@ -126,7 +126,7 @@ static_assert(FLOW_NUM_TYPES <= (1 << FLOW_TYPE_BITS), extern const char *flow_type_str[]; #define FLOW_TYPE(f) \ - ((f)->type < FLOW_NUM_TYPES ? flow_type_str[(f)->type] : "?") + ((f)->type < FLOW_NUM_TYPES ? flow_type_str[(f)->type] : "?") extern const uint8_t flow_proto[]; #define FLOW_PROTO(f) \ diff --git a/iov.h b/iov.h index ba1fda5d..d2184bfd 100644 --- a/iov.h +++ b/iov.h @@ -24,9 +24,9 @@ size_t iov_skip_bytes(const struct iovec *iov, size_t n, size_t skip, size_t *offset); size_t iov_from_buf(const struct iovec *iov, size_t iov_cnt, - size_t offset, const void *buf, size_t bytes); + size_t offset, const void *buf, size_t bytes); size_t iov_to_buf(const struct iovec *iov, size_t iov_cnt, - size_t offset, void *buf, size_t bytes); + size_t offset, void *buf, size_t bytes); size_t iov_size(const struct iovec *iov, size_t iov_cnt); /* diff --git a/udp_internal.h b/udp_internal.h index 96d11cff..f7d09bb6 100644 --- a/udp_internal.h +++ b/udp_internal.h @@ -26,7 +26,7 @@ size_t udp_update_hdr4(struct iphdr *ip4h, struct udp_payload_t *bp, const struct flowside *toside, size_t dlen, bool no_udp_csum); size_t udp_update_hdr6(struct ipv6hdr *ip6h, struct udp_payload_t *bp, - const struct flowside *toside, size_t dlen, + const struct flowside *toside, size_t dlen, bool no_udp_csum); void udp_sock_fwd(const struct ctx *c, int s, uint8_t frompif, in_port_t port, const struct timespec *now); diff --git a/vhost_user.c b/vhost_user.c index f9a5646c..1e320bff 100644 --- a/vhost_user.c +++ b/vhost_user.c @@ -652,9 +652,9 @@ static bool vu_set_vring_addr_exec(struct vu_dev *vdev, struct vhost_user_msg *vmsg) { /* We need to copy the payload to vhost_vring_addr structure - * to access index because address of vmsg->payload.addr - * can be unaligned as it is packed. - */ + * to access index because address of vmsg->payload.addr + * can be unaligned as it is packed. + */ struct vhost_vring_addr addr = vmsg->payload.addr; struct vu_virtq *vq = &vdev->vq[addr.index]; diff --git a/virtio.c b/virtio.c index bd388c2d..447137ee 100644 --- a/virtio.c +++ b/virtio.c @@ -164,8 +164,8 @@ static inline uint16_t vring_avail_ring(const struct vu_virtq *vq, int i) */ static inline uint16_t *virtq_used_event(const struct vu_virtq *vq) { - /* For backwards compat, used event index is at *end* of avail ring. */ - return &vq->vring.avail->ring[vq->vring.num]; + /* For backwards compat, used event index is at *end* of avail ring. */ + return &vq->vring.avail->ring[vq->vring.num]; } /** @@ -356,8 +356,8 @@ void vu_queue_notify(const struct vu_dev *dev, struct vu_virtq *vq) */ static inline uint16_t *virtq_avail_event(const struct vu_virtq *vq) { - /* For backwards compat, avail event index is at *end* of used ring. */ - return (uint16_t *)&vq->vring.used->ring[vq->vring.num]; + /* For backwards compat, avail event index is at *end* of used ring. */ + return (uint16_t *)&vq->vring.used->ring[vq->vring.num]; } /** diff --git a/vu_common.h b/vu_common.h index 27fe7e0b..b5112c1e 100644 --- a/vu_common.h +++ b/vu_common.h @@ -51,7 +51,7 @@ int vu_collect(const struct vu_dev *vdev, struct vu_virtq *vq, size_t *collected); void vu_set_vnethdr(const struct vu_dev *vdev, struct virtio_net_hdr_mrg_rxbuf *vnethdr, - int num_buffers); + int num_buffers); void vu_flush(const struct vu_dev *vdev, struct vu_virtq *vq, struct vu_virtq_element *elem, int elem_cnt); void vu_kick_cb(struct vu_dev *vdev, union epoll_ref ref, -- 2.52.0

2 1

[PATCH] tcp: Remove some no longer used includes
by David Gibson 12 Jan '26

12 Jan '26

Found a moment to fix this trivial bug that's been sitting around for a few months. tcp.c no longer relies on siphash.h or pcap.h so we can remove the #includes. Link: https://bugs.passt.top/show_bug.cgi?id=136 Reported-by: Xun Gu <xugu(a)redhat.com> Signed-off-by: David Gibson <david(a)gibson.dropbear.id.au> --- tcp.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/tcp.c b/tcp.c index e7fa85f3..8a924799 100644 --- a/tcp.c +++ b/tcp.c @@ -301,8 +301,6 @@ #include "ip.h" #include "passt.h" #include "tap.h" -#include "siphash.h" -#include "pcap.h" #include "tcp_splice.h" #include "log.h" #include "inany.h" -- 2.52.0

2 1

[PATCH 0/2] Small cleanups to splice forwarding logic
by David Gibson 12 Jan '26

12 Jan '26

While working on more flexible forwarding stuff, noticed a couple of warts here. David Gibson (2): fwd: Remove now-unnecessary handling of unspecified oaddr from splice fwd: Minor cleanup to fwd_nat_from_splice() fwd.c | 25 ++++++++----------------- 1 file changed, 8 insertions(+), 17 deletions(-) -- 2.52.0

2 2